FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Reference leak fix by srajko · Pull Request #977 · nodegit/nodegit · GitHub

Repository navigation

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension .cc  (4) .h  (1) .js  (4) .json  (1) All 4 file types selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
17 changes: 14 additions & 3 deletions generate/input/descriptor.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -1537,6 +1537,7 @@
},
"reference": {
"cppClassName": "GitRefs",
"selfFreeing": true,
"functions": {
"git_reference__alloc": {
"ignore": true
Expand Down Expand Up @@ -1590,6 +1591,16 @@
"isOptional": true
}
}
},
"git_reference_target": {
"return": {
"shouldDuplicate": true
}
},
"git_reference_target_peel": {
"return": {
"shouldDuplicate": true
}
}
}
},
Expand Down Expand Up @@ -1769,10 +1780,10 @@
"ignore": true
},
"repository": {
"dependencies": [
"git2/sys/repository.h"
],
"functions": {
"git_repository__cleanup": {
"ignore": true
},
"git_repository_discover": {
"isAsync": true,
"return": {
Expand Down
27 changes: 19 additions & 8 deletions generate/templates/manual/include/lock_master.h
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,15 @@
class LockMasterImpl;

class LockMaster {
public:
enum Status {
Disabled = 0,
EnabledForAsyncOnly,
Enabled
};

static bool enabled;
private:
static Status status;

LockMasterImpl *impl;

Expand Down Expand Up @@ -34,8 +41,8 @@ class LockMaster {
public:

// we lock on construction
template<typename ...Types> LockMaster(bool emptyGuard, const Types*... types) {
if(!enabled) {
template<typename ...Types> LockMaster(bool asyncAction, const Types*... types) {
if((status == Disabled) || ((status == EnabledForAsyncOnly) && !asyncAction)) {
impl = NULL;
return;
}
Expand All @@ -62,7 +69,7 @@ class LockMaster {
void DestructorImpl();
public:
TemporaryUnlock() {
// We can't return here if enabled is false
// We can't return here if disabled
// It's possible that a LockMaster was fully constructed and registered
// before the thread safety was disabled.
// So we rely on ConstructorImpl to abort if there is no registered LockMaster
Expand All @@ -80,15 +87,19 @@ class LockMaster {

// Enables the thread safety system
static void Enable() {
enabled = true;
status = Enabled;
}

static void SetStatus(Status status) {
LockMaster::status = status;
}

static void Disable() {
enabled = false;
status = Disabled;
}

static bool IsEnabled() {
return enabled;
static Status GetStatus() {
return status;
}

// Diagnostic information that can be provided to the JavaScript layer
Expand Down
4 changes: 2 additions & 2 deletions generate/templates/manual/src/lock_master.cc
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ NAN_GC_CALLBACK(LockMasterImpl::CleanupMutexes) {
// this means that turning thread safety on and then off
// could result in remaining mutexes - but they would get cleaned up
// if thread safety is turned on again
if (!LockMaster::IsEnabled()) {
if (LockMaster::GetStatus() == LockMaster::Disabled) {
return;
}

Expand Down Expand Up @@ -243,4 +243,4 @@ void LockMaster::TemporaryUnlock::DestructorImpl() {
impl->Lock(false);
}

bool LockMaster::enabled = false;
LockMaster::Status LockMaster::status = LockMaster::Disabled;
4 changes: 2 additions & 2 deletions generate/templates/partials/async_function.cc
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,9 @@ NAN_METHOD({{ cppClassName }}::{{ cppFunctionName }}) {

void {{ cppClassName }}::{{ cppFunctionName }}Worker::Execute() {
giterr_clear();

{
LockMaster lockMaster(true{%each args|argsInfo as arg %}
LockMaster lockMaster(/*asyncAction: */true{%each args|argsInfo as arg %}
{%if arg.cType|isPointer%}{%if not arg.cType|isDoublePointer%}
,baton->{{ arg.name }}
{%endif%}{%endif%}
Expand Down
4 changes: 2 additions & 2 deletions generate/templates/partials/sync_function.cc
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ if (Nan::ObjectWrap::Unwrap<{{ cppClassName }}>(info.This())->GetValue() != NULL
{% endif %}

giterr_clear();

{
LockMaster lockMaster(true{%each args|argsInfo as arg %}
LockMaster lockMaster(/*asyncAction: */false{%each args|argsInfo as arg %}
{%if arg.cType|isPointer%}{%if not arg.isReturn%}
,{%if arg.isSelf %}
Nan::ObjectWrap::Unwrap<{{ arg.cppClassName }}>(info.This())->GetValue()
Expand Down
32 changes: 26 additions & 6 deletions generate/templates/templates/nodegit.cc
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,25 @@ void LockMasterEnable(const FunctionCallbackInfo<Value>& info) {
LockMaster::Enable();
}

void LockMasterDisable(const FunctionCallbackInfo<Value>& info) {
LockMaster::Disable();
void LockMasterSetStatus(const FunctionCallbackInfo<Value>& info) {
Nan::HandleScope scope;

// convert the first argument to Status
if(info.Length() >= 0 && info[0]->IsNumber()) {
v8::Local<v8::Int32> value = info[0]->ToInt32();
LockMaster::Status status = static_cast<LockMaster::Status>(value->Value());
if(status >= LockMaster::Disabled && status <= LockMaster::Enabled) {
LockMaster::SetStatus(status);
return;
}
}

// argument error
Nan::ThrowError("Argument must be one 0, 1 or 2");
}

void LockMasterIsEnabled(const FunctionCallbackInfo<Value>& info) {
info.GetReturnValue().Set(Nan::New(LockMaster::IsEnabled()));
void LockMasterGetStatus(const FunctionCallbackInfo<Value>& info) {
info.GetReturnValue().Set(Nan::New(LockMaster::GetStatus()));
}

void LockMasterGetDiagnostics(const FunctionCallbackInfo<Value>& info) {
Expand Down Expand Up @@ -88,10 +101,17 @@ extern "C" void init(Local<v8::Object> target) {
ConvenientPatch::InitializeComponent(target);

NODE_SET_METHOD(target, "enableThreadSafety", LockMasterEnable);
NODE_SET_METHOD(target, "disableThreadSafety", LockMasterDisable);
NODE_SET_METHOD(target, "isThreadSafetyEnabled", LockMasterIsEnabled);
NODE_SET_METHOD(target, "setThreadSafetyStatus", LockMasterSetStatus);
NODE_SET_METHOD(target, "getThreadSafetyStatus", LockMasterGetStatus);
NODE_SET_METHOD(target, "getThreadSafetyDiagnostics", LockMasterGetDiagnostics);

Local<v8::Object> threadSafety = Nan::New<v8::Object>();
threadSafety->Set(Nan::New("DISABLED").ToLocalChecked(), Nan::New((int)LockMaster::Disabled));
threadSafety->Set(Nan::New("ENABLED_FOR_ASYNC_ONLY").ToLocalChecked(), Nan::New((int)LockMaster::EnabledForAsyncOnly));
threadSafety->Set(Nan::New("ENABLED").ToLocalChecked(), Nan::New((int)LockMaster::Enabled));

target->Set(Nan::New("THREAD_SAFETY").ToLocalChecked(), threadSafety);

LockMaster::Initialize();
}

Expand Down
4 changes: 4 additions & 0 deletions test/runner.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@ var local = path.join.bind(path, __dirname);
var NodeGit = require('..');

if(process.env.NODEGIT_TEST_THREADSAFETY) {
console.log('Enabling thread safety in NodeGit');
NodeGit.enableThreadSafety();
} else if (process.env.NODEGIT_TEST_THREADSAFETY_ASYNC) {
console.log('Enabling thread safety for async actions only in NodeGit');
NodeGit.setThreadSafetyStatus(NodeGit.THREAD_SAFETY.ENABLED_FOR_ASYNC_ONLY);
}

// Have to wrap exec, since it has a weird callback signature.
Expand Down
1 change: 0 additions & 1 deletion test/tests/index.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ describe("Index", function() {

after(function() {
this.index.clear();
NodeGit.disableThreadSafety();
});

it("can get the index of a repo and examine entries", function() {
Expand Down
13 changes: 13 additions & 0 deletions test/tests/repository.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,19 @@ describe("Repository", function() {
});
});

it("can be cleaned", function() {
this.repository.cleanup();

// try getting a commit after cleanup (to test that the repo is usable)
return this.repository.getHeadCommit()
.then(function(commit) {
assert.equal(
commit.toString(),
"32789a79e71fbc9e04d3eff7425e1771eb595150"
);
});
});

it("can read the index", function() {
return this.repository.index()
.then(function(index) {
Expand Down
51 changes: 26 additions & 25 deletions test/tests/thread_safety.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -22,43 +22,44 @@ describe("ThreadSafety", function() {
});

it("can enable and disable thread safety", function() {
var originalValue = NodeGit.isThreadSafetyEnabled();
var originalValue = NodeGit.getThreadSafetyStatus();

NodeGit.enableThreadSafety();
assert.equal(true, NodeGit.isThreadSafetyEnabled());
assert.equal(NodeGit.THREAD_SAFETY.ENABLED,
NodeGit.getThreadSafetyStatus());

NodeGit.disableThreadSafety();
assert.equal(false, NodeGit.isThreadSafetyEnabled());
NodeGit.setThreadSafetyStatus(NodeGit.THREAD_SAFETY.ENABLED_FOR_ASYNC_ONLY);
assert.equal(NodeGit.THREAD_SAFETY.ENABLED_FOR_ASYNC_ONLY,
NodeGit.getThreadSafetyStatus());

// flip the switch again, to make sure we test all transitions
// (we could have started with thread safety enabled)
NodeGit.enableThreadSafety();
assert.equal(true, NodeGit.isThreadSafetyEnabled());
NodeGit.setThreadSafetyStatus(NodeGit.THREAD_SAFETY.DISABLED);
assert.equal(NodeGit.THREAD_SAFETY.DISABLED,
NodeGit.getThreadSafetyStatus());

if (originalValue) {
NodeGit.enableThreadSafety();
} else {
NodeGit.disableThreadSafety();
}
NodeGit.setThreadSafetyStatus(originalValue);
});

it("can lock something and cleanup mutex", function() {
var diagnostics = NodeGit.getThreadSafetyDiagnostics();
var originalCount = diagnostics.storedMutexesCount;
// call a sync method to guarantee that it stores a mutex,
// and that it will clean up the mutex in a garbage collection cycle
this.repository.headDetached();

var diagnostics = NodeGit.getThreadSafetyDiagnostics();
if (NodeGit.isThreadSafetyEnabled()) {
// this is a fairly vague test - it just tests that something
// had a mutex created for it at some point (i.e., the thread safety
// code is not completely dead)
assert.ok(diagnostics.storedMutexesCount > 0);
// now test that GC cleans up mutexes
global.gc();
diagnostics = NodeGit.getThreadSafetyDiagnostics();
assert.equal(0, diagnostics.storedMutexesCount);
} else {
assert.equal(0, diagnostics.storedMutexesCount);
diagnostics = NodeGit.getThreadSafetyDiagnostics();
switch(NodeGit.getThreadSafetyStatus()) {
case NodeGit.THREAD_SAFETY.ENABLED:
// this is a fairly vague test - it just tests that something
// had a mutex created for it at some point (i.e., the thread safety
// code is not completely dead)
assert.ok(diagnostics.storedMutexesCount > 0);
break;
case NodeGit.THREAD_SAFETY.ENABLED_FOR_ASYNC_ONLY:
assert.equal(originalCount, diagnostics.storedMutexesCount);
break;

case NodeGit.THREAD_SAFETY.DISABLED:
assert.equal(0, diagnostics.storedMutexesCount);
}
});
});

Back | FazBrowse Home | New Git URL