Skip to content

Commit 920619e

Browse files
srajkosrajko
committed
Make remotes self-freeing
1 parent 89915d2 commit 920619e

File tree

6 files changed

+68
-30
lines changed

6 files changed

+68
-30
lines changed

‎generate/input/descriptor.json‎

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@
143143
},
144144
"git_blob_id": {
145145
"return": {
146-
"shouldDuplicate": true
146+
"ownedByThis": true
147147
}
148148
},
149149
"git_blob_rawcontent": {
@@ -389,12 +389,12 @@
389389
},
390390
"git_commit_author": {
391391
"return": {
392-
"shouldDuplicate": true
392+
"ownedByThis": true
393393
}
394394
},
395395
"git_commit_committer": {
396396
"return": {
397-
"shouldDuplicate": true
397+
"ownedByThis": true
398398
}
399399
},
400400
"git_commit_create": {
@@ -424,17 +424,17 @@
424424
},
425425
"git_commit_id": {
426426
"return": {
427-
"shouldDuplicate": true
427+
"ownedByThis": true
428428
}
429429
},
430430
"git_commit_parent_id": {
431431
"return": {
432-
"shouldDuplicate": true
432+
"ownedByThis": true
433433
}
434434
},
435435
"git_commit_tree_id": {
436436
"return": {
437-
"shouldDuplicate": true
437+
"ownedByThis": true
438438
}
439439
}
440440
}
@@ -1594,12 +1594,12 @@
15941594
},
15951595
"git_reference_target": {
15961596
"return": {
1597-
"shouldDuplicate": true
1597+
"ownedByThis": true
15981598
}
15991599
},
16001600
"git_reference_target_peel": {
16011601
"return": {
1602-
"shouldDuplicate": true
1602+
"ownedByThis": true
16031603
}
16041604
}
16051605
}
@@ -1627,6 +1627,7 @@
16271627
},
16281628
"remote": {
16291629
"cType": "git_remote",
1630+
"selfFreeing": true,
16301631
"functions": {
16311632
"git_remote_create": {
16321633
"isAsync": false
@@ -1711,6 +1712,11 @@
17111712
},
17121713
"isAsync": true
17131714
},
1715+
"git_remote_get_refspec": {
1716+
"return": {
1717+
"ownedByThis": true
1718+
}
1719+
},
17141720
"git_remote_list": {
17151721
"args": {
17161722
"out": {
@@ -1748,6 +1754,9 @@
17481754
},
17491755
"git_remote_set_push_refspecs": {
17501756
"ignore": true
1757+
},
1758+
"git_remote_stats": {
1759+
"ownedByThis": true
17511760
}
17521761
}
17531762
},
@@ -2182,7 +2191,7 @@
21822191
},
21832192
"git_tag_id": {
21842193
"return": {
2185-
"shouldDuplicate": true
2194+
"ownedByThis": true
21862195
}
21872196
},
21882197
"git_tag_create_lightweight": {
@@ -2222,7 +2231,7 @@
22222231
},
22232232
"git_tag_tagger": {
22242233
"return": {
2225-
"shouldDuplicate": true
2234+
"ownedByThis": true
22262235
}
22272236
},
22282237
"git_tag_target": {
@@ -2234,7 +2243,7 @@
22342243
},
22352244
"git_tag_target_id": {
22362245
"return": {
2237-
"shouldDuplicate": true
2246+
"ownedByThis": true
22382247
}
22392248
},
22402249
"git_tag_delete": {
@@ -2252,6 +2261,9 @@
22522261
}
22532262
}
22542263
},
2264+
"transfer_progress": {
2265+
"dupFunction": "git_transfer_progress_dup"
2266+
},
22552267
"transport": {
22562268
"cType": "git_transport",
22572269
"needsForwardDeclaration": false,
@@ -2281,28 +2293,28 @@
22812293
"functions": {
22822294
"git_tree_entry_byid": {
22832295
"return": {
2284-
"shouldDuplicate": true
2296+
"ownedByThis": true
22852297
}
22862298
},
22872299
"git_tree_entry_byindex": {
22882300
"jsFunctionName": "_entryByIndex"
22892301
},
22902302
"git_tree_entry_byname": {
22912303
"return": {
2292-
"shouldDuplicate": true
2304+
"ownedByThis": true
22932305
}
22942306
},
22952307
"git_tree_entry_id": {
22962308
"return": {
2297-
"shouldDuplicate": true
2309+
"ownedByThis": true
22982310
}
22992311
},
23002312
"git_tree_entrycount": {
23012313
"jsFunctionName": "entryCount"
23022314
},
23032315
"git_tree_id": {
23042316
"return": {
2305-
"shouldDuplicate": true
2317+
"ownedByThis": true
23062318
}
23072319
},
23082320
"git_tree_walk": {

‎generate/templates/manual/include/functions/copy.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,6 @@ const git_time *git_time_dup(const git_time *arg);
1515
const git_diff_delta *git_diff_delta_dup(const git_diff_delta *arg);
1616
const git_diff_file *git_diff_file_dup(const git_diff_file *arg);
1717

18+
void git_transfer_progress_dup(git_transfer_progress **out, const git_transfer_progress *arg);
19+
1820
#endif

‎generate/templates/manual/src/functions/copy.cc‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,8 @@ const git_error *git_error_dup(const git_error *arg) {
1010
result->message = strdup(arg->message);
1111
return result;
1212
}
13+
14+
void git_transfer_progress_dup(git_transfer_progress **out, const git_transfer_progress *arg) {
15+
*out = (git_transfer_progress *)malloc(sizeof(git_transfer_progress));
16+
memcpy(*out, arg, sizeof(git_transfer_progress));
17+
}

‎generate/templates/partials/convert_to_v8.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@
6161
{% if cppClassName == 'Wrapper' %}
6262
to = {{ cppClassName }}::New({{= parsedName =}});
6363
{% else %}
64-
to = {{ cppClassName }}::New({{= parsedName =}}, {{ selfFreeing|toBool }} {% if shouldDuplicate %}, true{% endif %});
64+
to = {{ cppClassName }}::New({{= parsedName =}}, {{ selfFreeing|toBool }} {% if ownedByThis %}, info.This(){% endif %});
6565
{% endif %}
6666
}
6767
else {

‎generate/templates/templates/class_content.cc‎

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -24,15 +24,24 @@ using namespace v8;
2424
using namespace node;
2525

2626
{% if cType %}
27-
{{ cppClassName }}::{{ cppClassName }}({{ cType }} *raw, bool selfFreeing, bool shouldDuplicate) {
28-
if (shouldDuplicate) {
29-
{% if shouldAlloc %}
30-
this->raw = ({{ cType }} *)malloc(sizeof({{ cType }}));
31-
{{ dupFunction }}(this->raw, raw);
27+
{{ cppClassName }}::{{ cppClassName }}({{ cType }} *raw, bool selfFreeing, Local<v8::Object> owner) {
28+
if (!owner.IsEmpty()) {
29+
// if we have an owner, there are two options - either we duplicate the raw object
30+
// (so we own the duplicate, and can self-free it)
31+
// or we keep a handle on the owner so it doesn't get garbage collected
32+
// while this wrapper is accessible
33+
{% if dupFunction %}
34+
{% if shouldAlloc %}
35+
this->raw = ({{ cType }} *)malloc(sizeof({{ cType }}));
36+
{{ dupFunction }}(this->raw, raw);
37+
{% else %}
38+
{{ dupFunction }}(&this->raw, raw);
39+
{% endif %}
40+
selfFreeing = true;
3241
{% else %}
33-
{{ dupFunction }}(&this->raw, raw);
42+
this->owner.Reset(owner);
43+
this->raw = raw;
3444
{% endif %}
35-
selfFreeing = true;
3645
} else {
3746
this->raw = raw;
3847
}
@@ -117,17 +126,22 @@ using namespace node;
117126
{{ cppClassName }}* object = new {{ cppClassName }}(static_cast<{{ cType }} *>(
118127
Local<External>::Cast(info[0])->Value()),
119128
Nan::To<bool>(info[1]).FromJust(),
120-
info.Length() >= 3 ? Nan::To<bool>(info[2]).FromJust() : false
129+
info.Length() >= 3 && !info[2].IsEmpty() && info[2]->IsObject() ? info[2]->ToObject() : Local<v8::Object>()
121130
);
122131
object->Wrap(info.This());
123132

124133
info.GetReturnValue().Set(info.This());
125134
}
126135

127-
Local<v8::Value> {{ cppClassName }}::New(const {{ cType }} *raw, bool selfFreeing, bool shouldDuplicate) {
136+
Local<v8::Value> {{ cppClassName }}::New(const {{ cType }} *raw, bool selfFreeing, Local<v8::Object> owner) {
128137
Nan::EscapableHandleScope scope;
129-
Local<v8::Value> argv[3] = { Nan::New<External>((void *)raw), Nan::New(selfFreeing), Nan::New(shouldDuplicate) };
130-
return scope.Escape(Nan::NewInstance(Nan::New({{ cppClassName }}::constructor_template), 3, argv).ToLocalChecked());
138+
Local<v8::Value> argv[3] = { Nan::New<External>((void *)raw), Nan::New(selfFreeing), owner };
139+
return scope.Escape(
140+
Nan::NewInstance(
141+
Nan::New({{ cppClassName }}::constructor_template),
142+
owner.IsEmpty() ? 2 : 3, // passing an empty handle as part of the arguments causes a crash
143+
argv
144+
).ToLocalChecked());
131145
}
132146

133147
NAN_METHOD({{ cppClassName }}::GetSelfFreeingInstanceCount) {

‎generate/templates/templates/class_header.h‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ class {{ cppClassName }} : public Nan::ObjectWrap {
4949
{{ cType }} *GetValue();
5050
void ClearValue();
5151

52-
static Local<v8::Value> New(const {{ cType }} *raw, bool selfFreeing, bool shouldDuplicate = false);
52+
static Local<v8::Value> New(const {{ cType }} *raw, bool selfFreeing, Local<v8::Object> owner = Local<v8::Object>());
5353
{%endif%}
5454
bool selfFreeing;
5555

@@ -82,10 +82,15 @@ class {{ cppClassName }} : public Nan::ObjectWrap {
8282

8383

8484
private:
85-
85+
// owner of the object, in the memory management sense. only populated
86+
// when using ownedByThis, and the type doesn't have a dupFunction
87+
// CopyablePersistentTraits are used to get the reset-on-destruct behavior.
88+
{%if not dupFunction %}
89+
Nan::Persistent<Object, Nan::CopyablePersistentTraits<Object> > owner;
90+
{%endif%}
8691

8792
{%if cType%}
88-
{{ cppClassName }}({{ cType }} *raw, bool selfFreeing, bool shouldDuplicate = false);
93+
{{ cppClassName }}({{ cType }} *raw, bool selfFreeing, Local<v8::Object> owner = Local<v8::Object>());
8994
~{{ cppClassName }}();
9095
{%endif%}
9196

0 commit comments

Comments
 (0)