Skip to content

Commit 4cb018e

Browse files
srajkosrajko
committed
Revert "Merge pull request nodegit#1006 from nodegit/oid-leak-fix"
This reverts commit 206d27d.
1 parent 206d27d commit 4cb018e

File tree

6 files changed

+26
-144
lines changed

6 files changed

+26
-144
lines changed

‎generate/input/descriptor.json‎

Lines changed: 0 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -73,11 +73,6 @@
7373
{
7474
"annotated_commit": {
7575
"functions": {
76-
"git_annotated_commit_id": {
77-
"return": {
78-
"ownedByThis": true
79-
}
80-
}
8176
}
8277
},
8378
"attr": {
@@ -1007,11 +1002,6 @@
10071002
"git_index_add_frombuffer": {
10081003
"ignore": true
10091004
},
1010-
"git_index_checksum": {
1011-
"return": {
1012-
"ownedByThis": true
1013-
}
1014-
},
10151005
"git_index_clear": {
10161006
"isAsync": true,
10171007
"return": {
@@ -1208,11 +1198,6 @@
12081198
"git_indexer_append": {
12091199
"ignore": true
12101200
},
1211-
"git_indexer_hash": {
1212-
"return": {
1213-
"ownedByThis": true
1214-
}
1215-
},
12161201
"git_indexer_new": {
12171202
"ignore": true
12181203
}
@@ -1292,11 +1277,6 @@
12921277
}
12931278
}
12941279
},
1295-
"git_note_id": {
1296-
"return": {
1297-
"ownedByThis": true
1298-
}
1299-
},
13001280
"git_note_remove": {
13011281
"isAsync": true,
13021282
"return": {
@@ -1313,11 +1293,6 @@
13131293
},
13141294
"object": {
13151295
"functions": {
1316-
"git_object_id": {
1317-
"return": {
1318-
"ownedByThis": true
1319-
}
1320-
},
13211296
"git_object_short_id": {
13221297
"args": {
13231298
"out": {
@@ -1429,11 +1404,6 @@
14291404
"cppClassName": "Wrapper",
14301405
"jsClassName": "Buffer"
14311406
}
1432-
},
1433-
"git_odb_object_id": {
1434-
"return": {
1435-
"ownedByThis": true
1436-
}
14371407
}
14381408
},
14391409
"dependencies": [
@@ -1448,7 +1418,6 @@
14481418
"ignore": true
14491419
},
14501420
"oid": {
1451-
"selfFreeing": "true",
14521421
"cpyFunction": "git_oid_cpy",
14531422
"freeFunctionName": "free",
14541423
"shouldAlloc": true,
@@ -1510,11 +1479,6 @@
15101479
"git_packbuilder_foreach": {
15111480
"ignore": true
15121481
},
1513-
"git_packbuilder_hash": {
1514-
"return": {
1515-
"ownedByThis": true
1516-
}
1517-
},
15181482
"git_packbuilder_new": {
15191483
"isAsync": false
15201484
},
@@ -1758,20 +1722,6 @@
17581722
"needsForwardDeclaration": false,
17591723
"ignore": true
17601724
},
1761-
"reflog_entry": {
1762-
"functions": {
1763-
"git_reflog_entry_id_new": {
1764-
"return": {
1765-
"ownedByThis": true
1766-
}
1767-
},
1768-
"git_reflog_entry_id_old": {
1769-
"return": {
1770-
"ownedByThis": true
1771-
}
1772-
}
1773-
}
1774-
},
17751725
"refspec": {
17761726
"cType": "git_refspec",
17771727
"functions": {
@@ -2241,11 +2191,6 @@
22412191
"isErrorCode": true
22422192
}
22432193
},
2244-
"git_submodule_head_id": {
2245-
"return": {
2246-
"ownedByThis": true
2247-
}
2248-
},
22492194
"git_submodule_init": {
22502195
"isAsync": true,
22512196
"return": {
@@ -2266,16 +2211,6 @@
22662211
"type": "int"
22672212
}
22682213
},
2269-
"git_submodule_index_id": {
2270-
"return": {
2271-
"ownedByThis": true
2272-
}
2273-
},
2274-
"git_submodule_wd_id": {
2275-
"return": {
2276-
"ownedByThis": true
2277-
}
2278-
},
22792214
"git_submodule_location": {
22802215
"isAsync": true,
22812216
"args": {

‎generate/input/libgit2-supplement.json‎

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -268,15 +268,6 @@
268268
"git_patch_convenient_from_diff"
269269
]
270270
],
271-
[
272-
"reflog_entry",
273-
[
274-
"git_reflog_entry_committer",
275-
"git_reflog_entry_id_new",
276-
"git_reflog_entry_id_old",
277-
"git_reflog_entry_message"
278-
]
279-
],
280271
[
281272
"revwalk",
282273
[
@@ -714,14 +705,6 @@
714705
"git_merge_head_id"
715706
]
716707
},
717-
"reflog": {
718-
"functions": [
719-
"git_reflog_entry_committer",
720-
"git_reflog_entry_id_new",
721-
"git_reflog_entry_id_old",
722-
"git_reflog_entry_message"
723-
]
724-
},
725708
"status": {
726709
"functions": [
727710
"git_status_list_entrycount",

‎generate/scripts/helpers.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,6 @@ var Helpers = {
220220
field.jsFunctionName = utils.camelCase(field.name);
221221
field.cppClassName = Helpers.cTypeToCppName(field.type);
222222
field.jsClassName = utils.titleCase(Helpers.cTypeToJsName(field.type));
223-
field.ownedByThis = true;
224223

225224
if (Helpers.isCallbackFunction(field.cType)) {
226225
Helpers.processCallback(field);

‎test/tests/commit.js‎

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ var promisify = require("promisify-node");
44
var fse = promisify(require("fs-extra"));
55

66
var garbageCollect = require("../utils/garbage_collect.js");
7-
var leakTest = require("../utils/leak_test");
87

98
var local = path.join.bind(path, __dirname);
109

@@ -368,11 +367,11 @@ describe("Commit", function() {
368367
}).then(function(reflog) {
369368
var reflogEntry = reflog.entryByIndex(0);
370369
assert.equal(
371-
reflogEntry.message(),
370+
NodeGit.Reflog.entryMessage(reflogEntry),
372371
customReflogMessage
373372
);
374373
assert.equal(
375-
reflogEntry.idNew().toString(),
374+
NodeGit.Reflog.entryIdNew(reflogEntry).toString(),
376375
oid
377376
);
378377
// only setTarget should have added to the entrycount
@@ -628,9 +627,30 @@ describe("Commit", function() {
628627
it("does not leak", function() {
629628
var test = this;
630629

631-
return leakTest(NodeGit.Commit, function() {
632-
return NodeGit.Commit.lookup(test.repository, oid);
633-
});
630+
garbageCollect();
631+
var Commit = NodeGit.Commit;
632+
var startSelfFreeingCount = Commit.getSelfFreeingInstanceCount();
633+
var startNonSelfFreeingCount = Commit.getNonSelfFreeingConstructedCount();
634+
635+
var resolve;
636+
var promise = new Promise(function(_resolve) { resolve = _resolve; });
637+
638+
NodeGit.Commit.lookup(test.repository, oid)
639+
.then(function() {
640+
// get out of this promise chain to help GC get rid of the commit
641+
setTimeout(resolve, 0);
642+
});
643+
644+
return promise
645+
.then(function() {
646+
garbageCollect();
647+
var endSelfFreeingCount = Commit.getSelfFreeingInstanceCount();
648+
var endNonSelfFreeingCount = Commit.getNonSelfFreeingConstructedCount();
649+
// any new self-freeing commits should have been freed
650+
assert.equal(startSelfFreeingCount, endSelfFreeingCount);
651+
// no new non-self-freeing commits should have been constructed
652+
assert.equal(startNonSelfFreeingCount, endNonSelfFreeingCount);
653+
});
634654
});
635655

636656
it("duplicates signature", function() {

‎test/tests/oid.js‎

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@ var assert = require("assert");
22
var path = require("path");
33
var local = path.join.bind(path, __dirname);
44

5-
var leakTest = require("../utils/leak_test");
6-
75
describe("Oid", function() {
86
var NodeGit = require("../../");
97
var Oid = NodeGit.Oid;
@@ -72,24 +70,4 @@ describe("Oid", function() {
7270
var oid2 = Oid.fromString("13c633665257696a3800b0a39ff636b4593f918f");
7371
assert(!this.oid.equal(oid2));
7472
});
75-
76-
it("does not leak constructed Oid", function() {
77-
return leakTest(Oid, function() {
78-
return Promise.resolve(
79-
Oid.fromString("13c633665257696a3800b0a39ff636b4593f918f")
80-
);
81-
});
82-
});
83-
84-
it("does not leak owned Oid", function() {
85-
return leakTest(Oid, function() {
86-
return NodeGit.Repository.open(local("../repos/workdir"))
87-
.then(function(repo) {
88-
return NodeGit.Commit.lookup(repo, oid);
89-
})
90-
.then(function(commit) {
91-
return commit.id();
92-
});
93-
});
94-
});
9573
});

‎test/utils/leak_test.js‎

Lines changed: 0 additions & 33 deletions
This file was deleted.

0 commit comments

Comments
 (0)