nodegit · johnhaley81 · Mar 29, 2016 · Mar 24, 2016
diff --git a/generate/templates/manual/include/lock_master.h b/generate/templates/manual/include/lock_master.h
@@ -4,8 +4,15 @@
 class LockMasterImpl;

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

-  static bool enabled;
+private:
+  static Status status;

  LockMasterImpl *impl;

@@ -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;
    }
@@ -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
@@ -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

diff --git a/generate/templates/manual/src/lock_master.cc b/generate/templates/manual/src/lock_master.cc
@@ -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;
  }

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

-bool LockMaster::enabled = false;
+LockMaster::Status LockMaster::status = LockMaster::Disabled;
diff --git a/generate/templates/partials/async_function.cc b/generate/templates/partials/async_function.cc
@@ -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%}

diff --git a/generate/templates/partials/sync_function.cc b/generate/templates/partials/sync_function.cc
@@ -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()

diff --git a/generate/templates/templates/nodegit.cc b/generate/templates/templates/nodegit.cc
@@ -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) {
@@ -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();
 }


diff --git a/test/runner.js b/test/runner.js
@@ -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.

diff --git a/test/tests/index.js b/test/tests/index.js
@@ -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() {

diff --git a/test/tests/thread_safety.js b/test/tests/thread_safety.js
@@ -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);
    }
  });
 });