Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit fbd892b

Browse filesBrowse files
srajkosrajko
committed
Add support for thread safety on async actions only
1 parent 39dfc74 commit fbd892b
Copy full SHA for fbd892b

File tree

Expand file treeCollapse file tree

7 files changed

+71
-45
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

7 files changed

+71
-45
lines changed
Open diff view settings
Collapse file

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

Copy file name to clipboardExpand all lines: generate/templates/manual/include/lock_master.h
+19-8Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,15 @@
44
class LockMasterImpl;
55

66
class LockMaster {
7+
public:
8+
enum Status {
9+
Disabled = 0,
10+
EnabledForAsyncOnly,
11+
Enabled
12+
};
713

8-
static bool enabled;
14+
private:
15+
static Status status;
916

1017
LockMasterImpl *impl;
1118

@@ -34,8 +41,8 @@ class LockMaster {
3441
public:
3542

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

8188
// Enables the thread safety system
8289
static void Enable() {
83-
enabled = true;
90+
status = Enabled;
91+
}
92+
93+
static void SetStatus(Status status) {
94+
LockMaster::status = status;
8495
}
8596

8697
static void Disable() {
87-
enabled = false;
98+
status = Disabled;
8899
}
89100

90-
static bool IsEnabled() {
91-
return enabled;
101+
static Status GetStatus() {
102+
return status;
92103
}
93104

94105
// Diagnostic information that can be provided to the JavaScript layer
Collapse file

‎generate/templates/manual/src/lock_master.cc‎

Copy file name to clipboardExpand all lines: generate/templates/manual/src/lock_master.cc
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ NAN_GC_CALLBACK(LockMasterImpl::CleanupMutexes) {
8787
// this means that turning thread safety on and then off
8888
// could result in remaining mutexes - but they would get cleaned up
8989
// if thread safety is turned on again
90-
if (!LockMaster::IsEnabled()) {
90+
if (LockMaster::GetStatus() == LockMaster::Disabled) {
9191
return;
9292
}
9393

@@ -243,4 +243,4 @@ void LockMaster::TemporaryUnlock::DestructorImpl() {
243243
impl->Lock(false);
244244
}
245245

246-
bool LockMaster::enabled = false;
246+
LockMaster::Status LockMaster::status = LockMaster::Disabled;
Collapse file

‎generate/templates/partials/async_function.cc‎

Copy file name to clipboardExpand all lines: generate/templates/partials/async_function.cc
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,9 @@ NAN_METHOD({{ cppClassName }}::{{ cppFunctionName }}) {
8080

8181
void {{ cppClassName }}::{{ cppFunctionName }}Worker::Execute() {
8282
giterr_clear();
83-
83+
8484
{
85-
LockMaster lockMaster(true{%each args|argsInfo as arg %}
85+
LockMaster lockMaster(/*asyncAction: */true{%each args|argsInfo as arg %}
8686
{%if arg.cType|isPointer%}{%if not arg.cType|isDoublePointer%}
8787
,baton->{{ arg.name }}
8888
{%endif%}{%endif%}
Collapse file

‎generate/templates/partials/sync_function.cc‎

Copy file name to clipboardExpand all lines: generate/templates/partials/sync_function.cc
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,9 @@ if (Nan::ObjectWrap::Unwrap<{{ cppClassName }}>(info.This())->GetValue() != NULL
3535
{% endif %}
3636

3737
giterr_clear();
38-
38+
3939
{
40-
LockMaster lockMaster(true{%each args|argsInfo as arg %}
40+
LockMaster lockMaster(/*asyncAction: */false{%each args|argsInfo as arg %}
4141
{%if arg.cType|isPointer%}{%if not arg.isReturn%}
4242
,{%if arg.isSelf %}
4343
Nan::ObjectWrap::Unwrap<{{ arg.cppClassName }}>(info.This())->GetValue()
Collapse file

‎generate/templates/templates/nodegit.cc‎

Copy file name to clipboardExpand all lines: generate/templates/templates/nodegit.cc
+19-6Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,25 @@ void LockMasterEnable(const FunctionCallbackInfo<Value>& info) {
2525
LockMaster::Enable();
2626
}
2727

28-
void LockMasterDisable(const FunctionCallbackInfo<Value>& info) {
29-
LockMaster::Disable();
28+
void LockMasterSetStatus(const FunctionCallbackInfo<Value>& info) {
29+
Nan::HandleScope scope;
30+
31+
// convert the first argument to Status
32+
if(info.Length() >= 0 && info[0]->IsNumber()) {
33+
v8::Local<v8::Int32> value = info[0]->ToInt32();
34+
LockMaster::Status status = static_cast<LockMaster::Status>(value->Value());
35+
if(status >= LockMaster::Status::Disabled && status <= LockMaster::Status::Enabled) {
36+
LockMaster::SetStatus(status);
37+
return;
38+
}
39+
}
40+
41+
// argument error
42+
Nan::ThrowError("Argument must be one 0, 1 or 2");
3043
}
3144

32-
void LockMasterIsEnabled(const FunctionCallbackInfo<Value>& info) {
33-
info.GetReturnValue().Set(Nan::New(LockMaster::IsEnabled()));
45+
void LockMasterGetStatus(const FunctionCallbackInfo<Value>& info) {
46+
info.GetReturnValue().Set(Nan::New(LockMaster::GetStatus()));
3447
}
3548

3649
void LockMasterGetDiagnostics(const FunctionCallbackInfo<Value>& info) {
@@ -88,8 +101,8 @@ extern "C" void init(Local<v8::Object> target) {
88101
ConvenientPatch::InitializeComponent(target);
89102

90103
NODE_SET_METHOD(target, "enableThreadSafety", LockMasterEnable);
91-
NODE_SET_METHOD(target, "disableThreadSafety", LockMasterDisable);
92-
NODE_SET_METHOD(target, "isThreadSafetyEnabled", LockMasterIsEnabled);
104+
NODE_SET_METHOD(target, "setThreadSafetyStatus", LockMasterSetStatus);
105+
NODE_SET_METHOD(target, "getThreadSafetyStatus", LockMasterGetStatus);
93106
NODE_SET_METHOD(target, "getThreadSafetyDiagnostics", LockMasterGetDiagnostics);
94107

95108
LockMaster::Initialize();
Collapse file

‎test/runner.js‎

Copy file name to clipboardExpand all lines: test/runner.js
+4Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,11 @@ var local = path.join.bind(path, __dirname);
66
var NodeGit = require('..');
77

88
if(process.env.NODEGIT_TEST_THREADSAFETY) {
9+
console.log('Enabling thread safety in NodeGit');
910
NodeGit.enableThreadSafety();
11+
} else if (process.env.NODEGIT_TEST_THREADSAFETY_ASYNC) {
12+
console.log('Enabling thread safety for async actions only in NodeGit');
13+
NodeGit.setThreadSafetyStatus(1);
1014
}
1115

1216
// Have to wrap exec, since it has a weird callback signature.
Collapse file

‎test/tests/thread_safety.js‎

Copy file name to clipboardExpand all lines: test/tests/thread_safety.js
+23-25Lines changed: 23 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -22,43 +22,41 @@ describe("ThreadSafety", function() {
2222
});
2323

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

2727
NodeGit.enableThreadSafety();
28-
assert.equal(true, NodeGit.isThreadSafetyEnabled());
28+
assert.equal(2, NodeGit.getThreadSafetyStatus());
2929

30-
NodeGit.disableThreadSafety();
31-
assert.equal(false, NodeGit.isThreadSafetyEnabled());
30+
NodeGit.setThreadSafetyStatus(1);
31+
assert.equal(1, NodeGit.getThreadSafetyStatus());
3232

33-
// flip the switch again, to make sure we test all transitions
34-
// (we could have started with thread safety enabled)
35-
NodeGit.enableThreadSafety();
36-
assert.equal(true, NodeGit.isThreadSafetyEnabled());
33+
NodeGit.setThreadSafetyStatus(0);
34+
assert.equal(0, NodeGit.getThreadSafetyStatus());
3735

38-
if (originalValue) {
39-
NodeGit.enableThreadSafety();
40-
} else {
41-
NodeGit.disableThreadSafety();
42-
}
36+
NodeGit.setThreadSafetyStatus(originalValue);
4337
});
4438

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

50-
var diagnostics = NodeGit.getThreadSafetyDiagnostics();
51-
if (NodeGit.isThreadSafetyEnabled()) {
52-
// this is a fairly vague test - it just tests that something
53-
// had a mutex created for it at some point (i.e., the thread safety
54-
// code is not completely dead)
55-
assert.ok(diagnostics.storedMutexesCount > 0);
56-
// now test that GC cleans up mutexes
57-
global.gc();
58-
diagnostics = NodeGit.getThreadSafetyDiagnostics();
59-
assert.equal(0, diagnostics.storedMutexesCount);
60-
} else {
61-
assert.equal(0, diagnostics.storedMutexesCount);
46+
diagnostics = NodeGit.getThreadSafetyDiagnostics();
47+
switch(NodeGit.getThreadSafetyStatus()) {
48+
case 2:
49+
// this is a fairly vague test - it just tests that something
50+
// had a mutex created for it at some point (i.e., the thread safety
51+
// code is not completely dead)
52+
assert.ok(diagnostics.storedMutexesCount > 0);
53+
break;
54+
case 1:
55+
assert.equal(originalCount, diagnostics.storedMutexesCount);
56+
break;
57+
58+
case 0:
59+
assert.equal(0, diagnostics.storedMutexesCount);
6260
}
6361
});
6462
});

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.