From 80371a1c26c772b227cce0a1a64ce4fb5e57b9fb Mon Sep 17 00:00:00 2001
From: Stjepan Rajko <stjepan.rajko@gmail.com>
Date: Tue, 26 Jan 2016 19:34:49 -0700
Subject: [PATCH] Clean mutexes as part of garbage collection cycle instead of
 uv_async

---
 generate/templates/manual/src/lock_master.cc | 27 +++++++++-----------
 test/tests/thread_safety.js                  | 11 +++++---
 2 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/generate/templates/manual/src/lock_master.cc b/generate/templates/manual/src/lock_master.cc
index d7d8239f5..bc26b9900 100644
--- a/generate/templates/manual/src/lock_master.cc
+++ b/generate/templates/manual/src/lock_master.cc
@@ -1,3 +1,4 @@
+#include <nan.h>
 #include <git2.h>
 #include <uv.h>
 #include <set>
@@ -31,10 +32,8 @@ class LockMasterImpl {
   // A libuv key used to store the current thread-specific LockMasterImpl instance
   static uv_key_t currentLockMasterKey;
 
-  // A libuv async handle used to trigger / debounce mutex cleanup
-  static uv_async_t cleanupMutexesHandle;
   // Cleans up any mutexes that are not currently used
-  static void CleanupMutexes(uv_async_t *async);
+  static NAN_GC_CALLBACK(CleanupMutexes);
 
 public:
   static void Initialize();
@@ -49,7 +48,6 @@ class LockMasterImpl {
   std::vector<uv_mutex_t *> GetMutexes(int useCountDelta);
   void Register();
   void Unregister();
-  void CleanupMutexes();
 
 public:
   static LockMasterImpl *CurrentLockMasterImpl() {
@@ -64,7 +62,6 @@ class LockMasterImpl {
   ~LockMasterImpl() {
     Unregister();
     Unlock(true);
-    CleanupMutexes();
   }
 
   void ObjectToLock(const void *objectToLock) {
@@ -78,16 +75,22 @@ class LockMasterImpl {
 std::map<const void *, ObjectInfo> LockMasterImpl::mutexes;
 uv_mutex_t LockMasterImpl::mapMutex;
 uv_key_t LockMasterImpl::currentLockMasterKey;
-uv_async_t LockMasterImpl::cleanupMutexesHandle;
-
 
 void LockMasterImpl::Initialize() {
   uv_mutex_init(&mapMutex);
   uv_key_create(&currentLockMasterKey);
-  uv_async_init(uv_default_loop(), &cleanupMutexesHandle, CleanupMutexes);
+  Nan::AddGCEpilogueCallback(CleanupMutexes);
 }
 
-void LockMasterImpl::CleanupMutexes(uv_async_t *async) {
+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::IsEnabled()) {
+    return;
+  }
+
   uv_mutex_lock(&mapMutex);
 
   for (auto it = mutexes.begin(); it != mutexes.end(); )
@@ -197,12 +200,6 @@ void LockMasterImpl::Unlock(bool releaseMutexes) {
   GetMutexes(releaseMutexes * -1);
 }
 
-void LockMasterImpl::CleanupMutexes() {
-  // schedule mutex cleanup on the main event loop
-  // this somewhat delays and debounces cleanup (uv_async_send coalesces calls)
-  uv_async_send(&cleanupMutexesHandle);
-}
-
 LockMaster::Diagnostics LockMasterImpl::GetDiagnostics() {
   LockMaster::Diagnostics diagnostics;
   uv_mutex_lock(&LockMasterImpl::mapMutex);
diff --git a/test/tests/thread_safety.js b/test/tests/thread_safety.js
index 785d20327..ee341a95a 100644
--- a/test/tests/thread_safety.js
+++ b/test/tests/thread_safety.js
@@ -42,18 +42,21 @@ describe("ThreadSafety", function() {
     }
   });
 
-  it("can lock something", function() {
+  it("can lock something and cleanup mutex", function() {
     // call a sync method to guarantee that it stores a mutex,
-    // and that it will not clean up the mutex (since the cleanup is
-    // scheduled on the main node thread)
+    // and that it will clean up the mutex in a garbage collection cycle
     this.repository.headDetached();
 
     var diagnostics = NodeGit.getThreadSafetyDiagnostics();
-    if (diagnostics.isEnabled) {
+    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);
     }