Make LockMaster shareable across contexts

implausible · implausible · commit ed068ad60398 · 2020-08-17T15:05:05.000-07:00
diff --git a/generate/templates/manual/include/lock_master.h b/generate/templates/manual/include/lock_master.h
@@ -6,16 +6,7 @@
 class LockMasterImpl;
 
 class LockMaster {
-public:
-  enum Status {
-    Disabled = 0,
-    EnabledForAsyncOnly,
-    Enabled
-  };
-
 private:
-  static Status status;
-
   LockMasterImpl *impl;
 
   template<typename T>
@@ -44,7 +35,7 @@ class LockMaster {
 
   // we lock on construction
   template<typename ...Types> LockMaster(bool asyncAction, const Types*... types) {
-    if((status == Disabled) || ((status == EnabledForAsyncOnly) && !asyncAction)) {
+    if(!asyncAction) {
       impl = NULL;
       return;
     }
@@ -85,33 +76,8 @@ class LockMaster {
     }
   };
 
-  static void Initialize();
-
-  // Enables the thread safety system
-  static void Enable() {
-    status = Enabled;
-  }
-
-  static void SetStatus(Status status) {
-    LockMaster::status = status;
-  }
-
-  static void Disable() {
-    status = Disabled;
-  }
-
-  static Status GetStatus() {
-    return status;
-  }
-
-  // Diagnostic information that can be provided to the JavaScript layer
-  // for a minimal level of testing
-  struct Diagnostics {
-    // this counts all stored mutexes - even if they are unlocked:
-    int storedMutexesCount;
-  };
-
-  static Diagnostics GetDiagnostics();
+  static void InitializeGlobal();
+  static void InitializeContext();
 };
 
 
diff --git a/generate/templates/manual/src/lock_master.cc b/generate/templates/manual/src/lock_master.cc
@@ -36,7 +36,9 @@ class LockMasterImpl {
   static NAN_GC_CALLBACK(CleanupMutexes);
 
 public:
-  static void Initialize();
+  static void InitializeGlobal();
+
+  static void InitializeContext();
 
   // INSTANCE variables / methods
 
@@ -53,7 +55,6 @@ class LockMasterImpl {
   static LockMasterImpl *CurrentLockMasterImpl() {
     return (LockMasterImpl *)uv_key_get(&currentLockMasterKey);
   }
-  static LockMaster::Diagnostics GetDiagnostics();
 
   LockMasterImpl() {
     Register();
@@ -76,21 +77,16 @@ std::map<const void *, ObjectInfo> LockMasterImpl::mutexes;
 uv_mutex_t LockMasterImpl::mapMutex;
 uv_key_t LockMasterImpl::currentLockMasterKey;
 
-void LockMasterImpl::Initialize() {
+void LockMasterImpl::InitializeGlobal() {
   uv_mutex_init(&mapMutex);
   uv_key_create(&currentLockMasterKey);
+}
+
+void LockMasterImpl::InitializeContext() {
   Nan::AddGCEpilogueCallback(CleanupMutexes);
 }
 
 NAN_GC_CALLBACK(LockMasterImpl::CleanupMutexes) {
-  // skip cleanup if thread safety is disabled
-  // 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::GetStatus() == LockMaster::Disabled) {
-    return;
-  }
-
   uv_mutex_lock(&mapMutex);
 
   for (auto it = mutexes.begin(); it != mutexes.end(); )
@@ -113,8 +109,12 @@ NAN_GC_CALLBACK(LockMasterImpl::CleanupMutexes) {
   uv_mutex_unlock(&mapMutex);
 }
 
-void LockMaster::Initialize() {
-  LockMasterImpl::Initialize();
+void LockMaster::InitializeGlobal() {
+  LockMasterImpl::InitializeGlobal();
+}
+
+void LockMaster::InitializeContext() {
+  LockMasterImpl::InitializeContext();
 }
 
 std::vector<uv_mutex_t *> LockMasterImpl::GetMutexes(int useCountDelta) {
@@ -200,14 +200,6 @@ void LockMasterImpl::Unlock(bool releaseMutexes) {
   GetMutexes(releaseMutexes * -1);
 }
 
-LockMaster::Diagnostics LockMasterImpl::GetDiagnostics() {
-  LockMaster::Diagnostics diagnostics;
-  uv_mutex_lock(&LockMasterImpl::mapMutex);
-  diagnostics.storedMutexesCount = mutexes.size();
-  uv_mutex_unlock(&LockMasterImpl::mapMutex);
-  return diagnostics;
-}
-
 // LockMaster
 
 void LockMaster::ConstructorImpl() {
@@ -226,10 +218,6 @@ void LockMaster::ObjectsToLockAdded() {
   impl->Lock(true);
 }
 
-LockMaster::Diagnostics LockMaster::GetDiagnostics() {
-  return LockMasterImpl::GetDiagnostics();
-}
-
 // LockMaster::TemporaryUnlock
 
 void LockMaster::TemporaryUnlock::ConstructorImpl() {
@@ -242,5 +230,3 @@ void LockMaster::TemporaryUnlock::ConstructorImpl() {
 void LockMaster::TemporaryUnlock::DestructorImpl() {
   impl->Lock(false);
 }
-
-LockMaster::Status LockMaster::status = LockMaster::Disabled;
diff --git a/generate/templates/templates/nodegit.cc b/generate/templates/templates/nodegit.cc
@@ -23,56 +23,24 @@
 #include "../include/convenient_hunk.h"
 #include "../include/filter_registry.h"
 
-v8::Local<v8::Value> GetPrivate(v8::Local<v8::Object> object, v8::Local<v8::String> key) {
-  v8::Local<v8::Value> value;
+using namespace v8;
+
+Local<Value> GetPrivate(Local<Object> object, Local<String> key) {
+  Local<Value> value;
   Nan::Maybe<bool> result = Nan::HasPrivate(object, key);
   if (!(result.IsJust() && result.FromJust()))
-    return v8::Local<v8::Value>();
+    return Local<Value>();
   if (Nan::GetPrivate(object, key).ToLocal(&value))
     return value;
-  return v8::Local<v8::Value>();
+  return Local<Value>();
 }
 
-void SetPrivate(v8::Local<v8::Object> object, v8::Local<v8::String> key, v8::Local<v8::Value> value) {
+void SetPrivate(Local<Object> object, Local<String> key, Local<Value> value) {
   if (value.IsEmpty())
     return;
   Nan::SetPrivate(object, key, value);
 }
 
-void LockMasterEnable(const FunctionCallbackInfo<Value>& info) {
-  LockMaster::Enable();
-}
-
-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 = Nan::To<v8::Int32>(info[0]).ToLocalChecked();
-    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 LockMasterGetStatus(const FunctionCallbackInfo<Value>& info) {
-  info.GetReturnValue().Set(Nan::New(LockMaster::GetStatus()));
-}
-
-void LockMasterGetDiagnostics(const FunctionCallbackInfo<Value>& info) {
-  LockMaster::Diagnostics diagnostics(LockMaster::GetDiagnostics());
-
-  // return a plain JS object with properties
-  v8::Local<v8::Object> result = Nan::New<v8::Object>();
-  Nan::Set(result, Nan::New("storedMutexesCount").ToLocalChecked(), Nan::New(diagnostics.storedMutexesCount));
-  info.GetReturnValue().Set(result);
-}
-
 static uv_mutex_t *opensslMutexes;
 
 void OpenSSL_LockingCallback(int mode, int type, const char *, int) {
@@ -101,8 +69,8 @@ void OpenSSL_ThreadSetup() {
 // TODO initialize a thread pool per context. Replace uv_default_loop() with node::GetCurrentEventLoop(isolate);
 ThreadPool libgit2ThreadPool(10, uv_default_loop());
 
-std::once_flag libraryInitializedFlag;
-std::mutex libraryInitializationMutex;
+static std::once_flag libraryInitializedFlag;
+static std::mutex libraryInitializationMutex;
 
 NAN_MODULE_INIT(init) {
   {
@@ -116,6 +84,8 @@ NAN_MODULE_INIT(init) {
       init_ssh2();
       // Initialize libgit2.
       git_libgit2_init();
+
+      LockMaster::InitializeGlobal();
     });
   }
 
@@ -133,19 +103,7 @@ NAN_MODULE_INIT(init) {
   ConvenientPatch::InitializeComponent(target);
   GitFilterRegistry::InitializeComponent(target);
 
-  NODE_SET_METHOD(target, "enableThreadSafety", LockMasterEnable);
-  NODE_SET_METHOD(target, "setThreadSafetyStatus", LockMasterSetStatus);
-  NODE_SET_METHOD(target, "getThreadSafetyStatus", LockMasterGetStatus);
-  NODE_SET_METHOD(target, "getThreadSafetyDiagnostics", LockMasterGetDiagnostics);
-
-  v8::Local<v8::Object> threadSafety = Nan::New<v8::Object>();
-  Nan::Set(threadSafety, Nan::New("DISABLED").ToLocalChecked(), Nan::New((int)LockMaster::Disabled));
-  Nan::Set(threadSafety, Nan::New("ENABLED_FOR_ASYNC_ONLY").ToLocalChecked(), Nan::New((int)LockMaster::EnabledForAsyncOnly));
-  Nan::Set(threadSafety, Nan::New("ENABLED").ToLocalChecked(), Nan::New((int)LockMaster::Enabled));
-
-  Nan::Set(target, Nan::New("THREAD_SAFETY").ToLocalChecked(), threadSafety);
-
-  LockMaster::Initialize();
+  LockMaster::InitializeContext();
 }
 
 NODE_MODULE(nodegit, init)
diff --git a/test/runner.js b/test/runner.js
@@ -5,14 +5,6 @@ var exec = require('../utils/execPromise');
 
 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);
-}
-
 var workdirPath = local("repos/workdir");
 
 before(function() {
diff --git a/test/tests/thread_safety.js b/test/tests/thread_safety.js
