Skip to content

Commit 8427b8e

Browse files
johnhaley81johnhaley81
committed
Merge pull request nodegit#944 from srajko/throttle-crash-fix
Throttle callbacks (again) + crash fix
2 parents f645360 + b0089b0 commit 8427b8e

File tree

11 files changed

+266
-72
lines changed

11 files changed

+266
-72
lines changed

‎generate/input/callbacks.json‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,8 @@
9696
"type": "int",
9797
"noResults": 1,
9898
"success": 0,
99-
"error": -1
99+
"error": -1,
100+
"throttle": 100
100101
}
101102
},
102103
"git_checkout_perfdata_cb": {
@@ -207,7 +208,8 @@
207208
"type": "int",
208209
"noResults": 1,
209210
"success": 0,
210-
"error": -1
211+
"error": -1,
212+
"throttle": 100
211213
}
212214
},
213215
"git_diff_hunk_cb": {
@@ -560,7 +562,8 @@
560562
"type": "int",
561563
"noResults":0,
562564
"success": 0,
563-
"error": -1
565+
"error": -1,
566+
"throttle": 100
564567
}
565568
},
566569
"git_stash_cb": {
@@ -670,7 +673,8 @@
670673
"type": "int",
671674
"noResults": 0,
672675
"success": 0,
673-
"error": -1
676+
"error": -1,
677+
"throttle": 100
674678
}
675679
},
676680
"git_transport_cb": {

‎generate/templates/manual/include/async_baton.h‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44
#include <uv.h>
55
#include <nan.h>
66

7+
#include "lock_master.h"
8+
#include "functions/sleep_for_ms.h"
9+
710
// Base class for Batons used for callbacks (for example,
811
// JS functions passed as callback parameters,
912
// or field properties of configuration objects whose values are callbacks)
@@ -13,4 +16,33 @@ struct AsyncBaton {
1316
bool done;
1417
};
1518

19+
template<typename ResultT>
20+
struct AsyncBatonWithResult : public AsyncBaton {
21+
ResultT result;
22+
ResultT defaultResult; // result returned if the callback doesn't return anything valid
23+
24+
AsyncBatonWithResult(const ResultT &defaultResult)
25+
: defaultResult(defaultResult) {
26+
}
27+
28+
ResultT ExecuteAsync(uv_async_cb asyncCallback) {
29+
result = 0;
30+
req.data = this;
31+
done = false;
32+
33+
uv_async_init(uv_default_loop(), &req, asyncCallback);
34+
{
35+
LockMaster::TemporaryUnlock temporaryUnlock;
36+
37+
uv_async_send(&req);
38+
39+
while(!done) {
40+
sleep_for_ms(1);
41+
}
42+
}
43+
44+
return result;
45+
}
46+
};
47+
1648
#endif

‎generate/templates/manual/include/callback_wrapper.h‎

Lines changed: 49 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,60 @@
11
#ifndef CALLBACK_WRAPPER_H
22
#define CALLBACK_WRAPPER_H
33

4-
#include <v8.h>
5-
#include <node.h>
6-
7-
#include "nan.h"
4+
#include <nan.h>
5+
#include <uv.h>
86

97
using namespace v8;
108
using namespace node;
119

12-
struct CallbackWrapper {
10+
class CallbackWrapper {
1311
Nan::Callback* jsCallback;
14-
void * payload;
12+
13+
// throttling data, used for callbacks that need to be throttled
14+
int throttle; // in milliseconds - if > 0, calls to the JS callback will be throttled
15+
uint64_t lastCallTime;
16+
17+
public:
18+
CallbackWrapper() {
19+
jsCallback = NULL;
20+
lastCallTime = 0;
21+
throttle = 0;
22+
}
23+
24+
~CallbackWrapper() {
25+
SetCallback(NULL);
26+
}
27+
28+
bool HasCallback() {
29+
return jsCallback != NULL;
30+
}
31+
32+
Nan::Callback* GetCallback() {
33+
return jsCallback;
34+
}
35+
36+
void SetCallback(Nan::Callback* callback, int throttle = 0) {
37+
if(jsCallback) {
38+
delete jsCallback;
39+
}
40+
jsCallback = callback;
41+
this->throttle = throttle;
42+
}
43+
44+
bool WillBeThrottled() {
45+
if(!throttle) {
46+
return false;
47+
}
48+
// throttle if needed
49+
uint64_t now = uv_hrtime();
50+
if(lastCallTime > 0 && now < lastCallTime + throttle * 1000000) {
51+
// throttled
52+
return true;
53+
} else {
54+
lastCallTime = now;
55+
return false;
56+
}
57+
}
1558
};
1659

1760
#endif

‎generate/templates/manual/include/lock_master.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
#ifndef LOCK_MASTER_H
22
#define LOCK_MASTER_H
33

4+
#include <git2.h>
5+
46
class LockMasterImpl;
57

68
class LockMaster {

‎generate/templates/partials/callback_helpers.cc‎

Lines changed: 7 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -6,28 +6,14 @@
66
{{ arg.cType }} {{ arg.name}}{% if not arg.lastArg %},{% endif %}
77
{% endeach %}
88
) {
9-
{{ cppFunctionName }}_{{ cbFunction.name|titleCase }}Baton* baton = new {{ cppFunctionName }}_{{ cbFunction.name|titleCase }}Baton();
9+
{{ cppFunctionName }}_{{ cbFunction.name|titleCase }}Baton* baton =
10+
new {{ cppFunctionName }}_{{ cbFunction.name|titleCase }}Baton({{ cbFunction.return.noResults }});
1011

1112
{% each cbFunction.args|argsInfo as arg %}
1213
baton->{{ arg.name }} = {{ arg.name }};
1314
{% endeach %}
1415

15-
baton->result = 0;
16-
baton->req.data = baton;
17-
baton->done = false;
18-
19-
uv_async_init(uv_default_loop(), &baton->req, (uv_async_cb) {{ cppFunctionName }}_{{ cbFunction.name }}_async);
20-
{
21-
LockMaster::TemporaryUnlock temporaryUnlock;
22-
23-
uv_async_send(&baton->req);
24-
25-
while(!baton->done) {
26-
sleep_for_ms(1);
27-
}
28-
}
29-
30-
return baton->result;
16+
return baton->ExecuteAsync((uv_async_cb) {{ cppFunctionName }}_{{ cbFunction.name }}_async);
3117
}
3218

3319
void {{ cppClassName }}::{{ cppFunctionName }}_{{ cbFunction.name }}_async(uv_async_t* req, int status) {
@@ -93,12 +79,12 @@ void {{ cppClassName }}::{{ cppFunctionName }}_{{ cbFunction.name }}_async(uv_as
9379
baton->result = (int)result->ToNumber()->Value();
9480
}
9581
else {
96-
baton->result = {{ cbFunction.return.noResults }};
82+
baton->result = baton->defaultResult;
9783
}
9884
{% endif %}
9985
}
10086
else {
101-
baton->result = {{ cbFunction.return.noResults }};
87+
baton->result = baton->defaultResult;
10288
}
10389
{% endeach %}
10490

@@ -127,12 +113,12 @@ void {{ cppClassName }}::{{ cppFunctionName }}_{{ cbFunction.name }}_promiseComp
127113
baton->result = (int)result->ToNumber()->Value();
128114
}
129115
else {
130-
baton->result = {{ cbFunction.return.noResults }};
116+
baton->result = baton->defaultResult;
131117
}
132118
{% endif %}
133119
}
134120
else {
135-
baton->result = {{ cbFunction.return.noResults }};
121+
baton->result = baton->defaultResult;
136122
}
137123
{% endeach %}
138124
}

‎generate/templates/partials/field_accessors.cc‎

Lines changed: 45 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@
1111
info.GetReturnValue().Set(Nan::New(wrapper->{{ field.name }}));
1212

1313
{% elsif field.isCallbackFunction %}
14-
if (wrapper->{{field.name}} != NULL) {
15-
info.GetReturnValue().Set(wrapper->{{ field.name }}->GetFunction());
14+
if (wrapper->{{field.name}}.HasCallback()) {
15+
info.GetReturnValue().Set(wrapper->{{ field.name }}.GetCallback()->GetFunction());
1616
} else {
1717
info.GetReturnValue().SetUndefined();
1818
}
@@ -31,7 +31,6 @@
3131
}
3232

3333
NAN_SETTER({{ cppClassName }}::Set{{ field.cppFunctionName }}) {
34-
3534
{{ cppClassName }} *wrapper = Nan::ObjectWrap::Unwrap<{{ cppClassName }}>(info.This());
3635

3736
{% if field.isEnum %}
@@ -47,16 +46,35 @@
4746
wrapper->raw->{{ field.name }} = {% if not field.cType | isPointer %}*{% endif %}{% if field.cppClassName == 'GitStrarray' %}StrArrayConverter::Convert({{ field.name }}->ToObject()){% else %}Nan::ObjectWrap::Unwrap<{{ field.cppClassName }}>({{ field.name }}->ToObject())->GetValue(){% endif %};
4847

4948
{% elsif field.isCallbackFunction %}
50-
if (wrapper->{{ field.name }} != NULL) {
51-
delete wrapper->{{ field.name }};
52-
}
49+
Nan::Callback *callback = NULL;
50+
int throttle = {%if field.return.throttle %}{{ field.return.throttle }}{%else%}0{%endif%};
5351

5452
if (value->IsFunction()) {
53+
callback = new Nan::Callback(value.As<Function>());
54+
} else if (value->IsObject()) {
55+
Local<Object> object = value.As<Object>();
56+
Local<String> callbackKey;
57+
Nan::MaybeLocal<Value> maybeObjectCallback = Nan::Get(object, Nan::New("callback").ToLocalChecked());
58+
if (!maybeObjectCallback.IsEmpty()) {
59+
Local<Value> objectCallback = maybeObjectCallback.ToLocalChecked();
60+
if (objectCallback->IsFunction()) {
61+
callback = new Nan::Callback(objectCallback.As<Function>());
62+
Nan::MaybeLocal<Value> maybeObjectThrottle = Nan::Get(object, Nan::New("throttle").ToLocalChecked());
63+
if(!maybeObjectThrottle.IsEmpty()) {
64+
Local<Value> objectThrottle = maybeObjectThrottle.ToLocalChecked();
65+
if (objectThrottle->IsNumber()) {
66+
throttle = (int)objectThrottle.As<Number>()->Value();
67+
}
68+
}
69+
}
70+
}
71+
}
72+
if (callback) {
5573
if (!wrapper->raw->{{ field.name }}) {
5674
wrapper->raw->{{ field.name }} = ({{ field.cType }}){{ field.name }}_cppCallback;
5775
}
5876

59-
wrapper->{{ field.name }} = new Nan::Callback(value.As<Function>());
77+
wrapper->{{ field.name }}.SetCallback(callback, throttle);
6078
}
6179

6280
{% elsif field.payloadFor %}
@@ -82,46 +100,42 @@
82100
}
83101

84102
{% if field.isCallbackFunction %}
103+
{{ cppClassName }}* {{ cppClassName }}::{{ field.name }}_getInstanceFromBaton({{ field.name|titleCase }}Baton* baton) {
104+
return static_cast<{{ cppClassName }}*>(baton->{% each field.args|argsInfo as arg %}
105+
{% if arg.payload == true %}{{arg.name}}{% elsif arg.lastArg %}{{arg.name}}{% endif %}
106+
{% endeach %});
107+
}
108+
85109
{{ field.return.type }} {{ cppClassName }}::{{ field.name }}_cppCallback (
86110
{% each field.args|argsInfo as arg %}
87111
{{ arg.cType }} {{ arg.name}}{% if not arg.lastArg %},{% endif %}
88112
{% endeach %}
89113
) {
90-
{{ field.name|titleCase }}Baton* baton = new {{ field.name|titleCase }}Baton();
114+
{{ field.name|titleCase }}Baton* baton =
115+
new {{ field.name|titleCase }}Baton({{ field.return.noResults }});
91116

92117
{% each field.args|argsInfo as arg %}
93118
baton->{{ arg.name }} = {{ arg.name }};
94119
{% endeach %}
95120

96-
baton->result = 0;
97-
baton->req.data = baton;
98-
baton->done = false;
99-
100-
uv_async_init(uv_default_loop(), &baton->req, (uv_async_cb) {{ field.name }}_async);
101-
{
102-
LockMaster::TemporaryUnlock temporaryUnlock;
103-
104-
uv_async_send(&baton->req);
121+
{{ cppClassName }}* instance = {{ field.name }}_getInstanceFromBaton(baton);
105122

106-
while(!baton->done) {
107-
sleep_for_ms(1);
108-
}
123+
if (instance->{{ field.name }}.WillBeThrottled()) {
124+
return baton->defaultResult;
109125
}
110126

111-
return baton->result;
127+
return baton->ExecuteAsync((uv_async_cb) {{ field.name }}_async);
112128
}
113129

114130
void {{ cppClassName }}::{{ field.name }}_async(uv_async_t* req, int status) {
115131
Nan::HandleScope scope;
116132

117133
{{ field.name|titleCase }}Baton* baton = static_cast<{{ field.name|titleCase }}Baton*>(req->data);
118-
{{ cppClassName }}* instance = static_cast<{{ cppClassName }}*>(baton->{% each field.args|argsInfo as arg %}
119-
{% if arg.payload == true %}{{arg.name}}{% elsif arg.lastArg %}{{arg.name}}{% endif %}
120-
{% endeach %});
134+
{{ cppClassName }}* instance = {{ field.name }}_getInstanceFromBaton(baton);
121135

122-
if (instance->{{ field.name }}->IsEmpty()) {
136+
if (instance->{{ field.name }}.GetCallback()->IsEmpty()) {
123137
{% if field.return.type == "int" %}
124-
baton->result = {{ field.return.noResults }}; // no results acquired
138+
baton->result = baton->defaultResult; // no results acquired
125139
{% endif %}
126140

127141
baton->done = true;
@@ -163,7 +177,7 @@
163177
};
164178

165179
Nan::TryCatch tryCatch;
166-
Local<v8::Value> result = instance->{{ field.name }}->Call({{ field.args|jsArgsCount }}, argv);
180+
Local<v8::Value> result = instance->{{ field.name }}.GetCallback()->Call({{ field.args|jsArgsCount }}, argv);
167181

168182
uv_close((uv_handle_t*) &baton->req, NULL);
169183

@@ -187,12 +201,12 @@
187201
baton->result = (int)result->ToNumber()->Value();
188202
}
189203
else {
190-
baton->result = {{ field.return.noResults }};
204+
baton->result = baton->defaultResult;
191205
}
192206
{% endif %}
193207
}
194208
else {
195-
baton->result = {{ field.return.noResults }};
209+
baton->result = baton->defaultResult;
196210
}
197211
{% endeach %}
198212
baton->done = true;
@@ -220,12 +234,12 @@
220234
baton->result = (int)result->ToNumber()->Value();
221235
}
222236
else{
223-
baton->result = {{ field.return.noResults }};
237+
baton->result = baton->defaultResult;
224238
}
225239
{% endif %}
226240
}
227241
else {
228-
baton->result = {{ field.return.noResults }};
242+
baton->result = baton->defaultResult;
229243
}
230244
{% endeach %}
231245
}

‎generate/templates/templates/class_content.cc‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ extern "C" {
1111
#include "../include/lock_master.h"
1212
#include "../include/functions/copy.h"
1313
#include "../include/{{ filename }}.h"
14-
#include "../include/functions/sleep_for_ms.h"
1514

1615
{% each dependencies as dependency %}
1716
#include "{{ dependency }}"

0 commit comments

Comments
 (0)