Fix possible deadlock because of never unlocked mutex during failed unmount (#31)

* Fix possible deadlock because of never unlocked mutex during failed unmount.

* Fix segmentation fault after unmounting.

Scenario:
1. Mount path /a
2. Mount path /b
3. Unmount path /b
4. Appempt to read from path /a will throw segmentation fault.

Reason:
The function bindings_alloc correctly allocates memory and sets the index property of the binding. Afterwards the mount function does memset again for the binding and resets the index to zero. Unmounting will now always NULL the binding at position zero. However the actual binding on position zero is still mounted and the next access to it results in segmentation fault.

abstractions.cc

@@ -1,5 +1,31 @@
 #include "abstractions.h"
 
+#ifndef _WIN32
+
+#include <unistd.h>
+#include <sys/wait.h>
+
+int execute_command_and_wait (char* argv[]) {
+    // Fork our running process.
+    pid_t cpid = vfork();
+
+    // Check if we are the observer or the new process.
+    if (cpid > 0) {
+        int status = 0;
+        waitpid(cpid, &status, 0);
+        return WIFEXITED(status) ? WEXITSTATUS(status) : -1;
+    } else {
+        // At this point we are on our child process.
+        execvp(argv[0], argv);
+        exit(1);
+
+        // Something failed.
+        return -1;
+    }
+}
+
+#endif
+
 #ifdef __APPLE__
 
 #include <unistd.h>
@@ -17,12 +43,10 @@ void thread_join (abstr_thread_t thread) {
     pthread_join(thread, NULL);
 }
 
-void fusermount (char *path) {
+int fusermount (char *path) {
     char *argv[] = {(char *) "umount", path, NULL};
 
-    pid_t cpid = vfork();
-    if (cpid > 0) waitpid(cpid, NULL, 0);
-    else execvp(argv[0], argv);
+    return execute_command_and_wait(argv);
 }
 
 #elif defined(_WIN32)
@@ -37,7 +61,7 @@ void thread_join (HANDLE thread) {
     WaitForSingleObject(thread, INFINITE);
 }
 
-void fusermount (char *path) {
+int fusermount (char *path) {
     char* dokanPath = getenv("DOKAN_INSTALL_DIR");
     char cmdLine[MAX_PATH];
 
@@ -49,9 +73,15 @@ void fusermount (char *path) {
     CreateProcess(NULL, cmdLine, NULL, NULL, false, CREATE_NO_WINDOW, NULL, NULL, &info, &procInfo);
 
     WaitForSingleObject(procInfo.hProcess, INFINITE);
+
+    DWORD exitCode = -1;
+    GetExitCodeProcess(procInfo.hProcess, &exitCode);
+
     CloseHandle(procInfo.hProcess);
     CloseHandle(procInfo.hThread);
 
+    return exitCode;
+
     // dokanctl.exe requires admin permissions for some reason, so if node is not run as admin,
     // it'll fail to create the process for unmounting. The path will be unmounted once
     // the process is killed, however, so there's that!
@@ -74,12 +104,10 @@ void thread_join (abstr_thread_t thread) {
     pthread_join(thread, NULL);
 }
 
-void fusermount (char *path) {
+int fusermount (char *path) {
     char *argv[] = {(char *) "fusermount", (char *) "-q", (char *) "-u", path, NULL};
 
-    pid_t cpid = vfork();
-    if (cpid > 0) waitpid(cpid, NULL, 0);
-    else execvp(argv[0], argv);
+    return execute_command_and_wait(argv);
 }
 
 #endif

abstractions.h

@@ -123,4 +123,4 @@ typedef thread_fn_rtn_t(*thread_fn)(void*);
 void thread_create (abstr_thread_t*, thread_fn, void*);
 void thread_join (abstr_thread_t);
 
-void fusermount (char*);
+int fusermount (char*);

example.js

@@ -55,8 +55,11 @@ fuse.mount(mountPath, {
 })
 
 process.on('SIGINT', function () {
-  fuse.unmount(mountPath, function () {
-    console.log('filesystem at ' + mountPath + ' unmounted')
-    process.exit()
+  fuse.unmount(mountPath, function (err) {
+    if (err) {
+      console.log('filesystem at ' + mountPath + ' not unmounted', err)
+    } else {
+      console.log('filesystem at ' + mountPath + ' unmounted')
+    }
   })
 })

fuse-bindings.cc

@@ -141,17 +141,20 @@ static bindings_t *bindings_find_mounted (char *path) {
   return NULL;
 }
 
-static void bindings_fusermount (char *path) {
-  fusermount(path);
+static int bindings_fusermount (char *path) {
+  return fusermount(path);
 }
 
-static void bindings_unmount (char *path) {
+static int bindings_unmount (char *path) {
   mutex_lock(&mutex);
   bindings_t *b = bindings_find_mounted(path);
-  if (b != NULL) b->gc = 1;
-  bindings_fusermount(path);
-  if (b != NULL) thread_join(b->thread);
+  int result = bindings_fusermount(path);
+  if (b != NULL && result == 0) b->gc = 1;
   mutex_unlock(&mutex);
+
+  if (b != NULL && result == 0) thread_join(b->thread);
+
+  return result;
 }
 
 #if (NODE_MODULE_VERSION > NODE_0_10_MODULE_VERSION && NODE_MODULE_VERSION < IOJS_3_0_MODULE_VERSION)
@@ -1108,7 +1111,6 @@ NAN_METHOD(Mount) {
   mutex_unlock(&mutex);
 
   memset(&empty_stat, 0, sizeof(empty_stat));
-  memset(b, 0, sizeof(bindings_t));
 
   Nan::Utf8String path(info[0]);
   Local<Object> ops = info[1].As<Object>();
@@ -1172,12 +1174,16 @@ NAN_METHOD(Mount) {
 class UnmountWorker : public Nan::AsyncWorker {
  public:
   UnmountWorker(Nan::Callback *callback, char *path)
-    : Nan::AsyncWorker(callback), path(path) {}
+    : Nan::AsyncWorker(callback), path(path), result(0) {}
   ~UnmountWorker() {}
 
   void Execute () {
-    bindings_unmount(path);
+    result = bindings_unmount(path);
     free(path);
+
+    if (result != 0) {
+      SetErrorMessage("Error");
+    }
   }
 
   void HandleOKCallback () {
@@ -1187,6 +1193,7 @@ class UnmountWorker : public Nan::AsyncWorker {
 
  private:
   char *path;
+  int result;
 };
 
 NAN_METHOD(SetCallback) {

index.js

@@ -90,16 +90,7 @@ exports.mount = function (mnt, ops, opts, cb) {
 }
 
 exports.unmount = function (mnt, cb) {
-  var timeout = setTimeout(function () {
-    var err = new Error('Unmount took too long')
-    err.code = 'ETIMEDOUT'
-    if (cb) cb(err)
-  }, 2000)
-
-  fuse.unmount(path.resolve(mnt), function (err) {
-    clearTimeout(timeout)
-    if (cb) cb(err)
-  })
+  fuse.unmount(path.resolve(mnt), cb)
 }
 
 exports.errno = function (code) {