Nodejs process doesnt close after stopping

includes/NSFW.h

@@ -26,7 +26,6 @@ class NSFW : public Napi::ObjectWrap<NSFW> {
     std::string mPath;
     std::thread mPollThread;
     std::atomic<bool> mRunning;
-    std::atomic<bool> mFinalizing;
     std::condition_variable mWaitPoolEvents;
     std::mutex mRunningLock;
     std::vector<std::string> mExcludedPaths;

index.d.ts

@@ -21,8 +21,10 @@ declare module 'nsfw' {
             start: () => Promise<void>;
             /** Returns a Promise that resolves when NSFW object has halted. */
             stop: () => Promise<void>;
-            /** Returns a Promise that resolves when NSFW object has updated the excluded ppaths. */
-            updateExcludedPaths: (excludedPaths: [string]) => Promise<void>;
+            /** Returns a Promise that resolves when NSFW object has updated the excluded paths. */
+            updateExcludedPaths: (excludedPaths: string[]) => Promise<void>;
+            /** Returns a Promise that resolves with the current excluded paths. */
+            getExcludedPaths: () => Promise<string[]>;
         }
 
         export const enum ActionType {
@@ -65,7 +67,7 @@ declare module 'nsfw' {
             /** callback to fire in the case of errors */
             errorCallback?: (err: any) => void;
             /** paths to be excluded */
-            excludedPaths?: [string];
+            excludedPaths?: string[];
         }
     }
 

js/src/index.js

@@ -18,8 +18,8 @@ function NSFWFilePoller(watchPath, eventCallback, debounceMS) {
         fileStatus = status;
         eventCallback([{ action: CREATED, directory, file }]);
       } else if (
-        status.mtime - fileStatus.mtime !== 0 ||
-        status.ctime - fileStatus.ctime !== 0
+        status.mtime.getTime() !== fileStatus.mtime.getTime() ||
+        status.ctime.getTime() !== fileStatus.ctime.getTime()
       ) {
         fileStatus = status;
         eventCallback([{ action: MODIFIED, directory, file }]);
@@ -50,18 +50,24 @@ function NSFWFilePoller(watchPath, eventCallback, debounceMS) {
   this.resume = () => this.start();
 }
 
-
-const buildNSFW = async (watchPath, eventCallback,
-  { debounceMS = 500, errorCallback: _errorCallback, excludedPaths = [] } = {}) => {
+const buildNSFW = async (
+  watchPath,
+  eventCallback,
+  { debounceMS = 500, errorCallback: _errorCallback, excludedPaths = [] } = {}
+) => {
   if (Number.isInteger(debounceMS)) {
-    if (debounceMS < 1) {
-      throw new Error('Minimum debounce is 1ms.');
+    if (debounceMS < 1 || debounceMS > 60000) {
+      throw new Error('debounceMS must be >= 1 and <= 60000.');
     }
   } else {
     throw new Error('debounceMS must be an integer.');
   }
 
-  const errorCallback = _errorCallback || ((nsfwError) => { throw nsfwError; });
+  const errorCallback =
+    _errorCallback ||
+    ((nsfwError) => {
+      throw nsfwError;
+    });
 
   if (!path.isAbsolute(watchPath)) {
     throw new Error('Path to watch must be an absolute path.');
@@ -75,23 +81,37 @@ const buildNSFW = async (watchPath, eventCallback,
   }
 
   if (excludedPaths) {
+    const normalizedWatchPath = watchPath.replace(/[\\/]+$/, '');
     for (let i = 0; i < excludedPaths.length; i++) {
-      const excludedPath = excludedPaths[i];
+      const normalizedExcludedPath = excludedPaths[i].replace(/[\\/]+$/, '');
       if (process.platform === 'win32') {
-        if (!excludedPath.substring(0, watchPath.length - 1).
-          localeCompare(watchPath, undefined, { sensitivity: 'accent' })) {
-          throw new Error('Excluded path must be a valid subdirectory of the watching path.');
+        if (
+          normalizedExcludedPath
+            .substring(0, normalizedWatchPath.length)
+            .localeCompare(normalizedWatchPath, undefined, {
+              sensitivity: 'accent',
+            }) !== 0
+        ) {
+          throw new Error(
+            'Excluded path must be a valid subdirectory of the watching path.'
+          );
         }
       } else {
-        if (!excludedPath.startsWith(watchPath)) {
-          throw new Error('Excluded path must be a valid subdirectory of the watching path.');
+        if (!normalizedExcludedPath.startsWith(normalizedWatchPath)) {
+          throw new Error(
+            'Excluded path must be a valid subdirectory of the watching path.'
+          );
         }
       }
     }
   }
 
   if (stats.isDirectory()) {
-    return new NSFW(watchPath, eventCallback, { debounceMS, errorCallback, excludedPaths });
+    return new NSFW(watchPath, eventCallback, {
+      debounceMS,
+      errorCallback,
+      excludedPaths,
+    });
   } else if (stats.isFile()) {
     return new NSFWFilePoller(watchPath, eventCallback, debounceMS);
   } else {
@@ -101,7 +121,9 @@ const buildNSFW = async (watchPath, eventCallback,
 
 function nsfw(watchPath, eventCallback, options) {
   if (!(this instanceof nsfw)) {
-    return buildNSFW(watchPath, eventCallback, options).then(implementation => new nsfw(implementation));
+    return buildNSFW(watchPath, eventCallback, options).then(
+      (implementation) => new nsfw(implementation)
+    );
   }
 
   const implementation = watchPath;
@@ -110,16 +132,21 @@ function nsfw(watchPath, eventCallback, options) {
   this.stop = () => implementation.stop();
   this.pause = () => implementation.pause();
   this.resume = () => implementation.resume();
-  this.getExcludedPaths = () => implementation.getExcludedPaths();
-  this.updateExcludedPaths = (paths) => implementation.updateExcludedPaths(paths);
+  this.getExcludedPaths = () =>
+    implementation.getExcludedPaths
+      ? implementation.getExcludedPaths()
+      : Promise.resolve([]);
+  this.updateExcludedPaths = (paths) =>
+    implementation.updateExcludedPaths
+      ? implementation.updateExcludedPaths(paths)
+      : Promise.resolve();
 }
 
-
 nsfw.actions = {
   CREATED: 0,
   DELETED: 1,
   MODIFIED: 2,
-  RENAMED: 3
+  RENAMED: 3,
 };
 
 nsfw._native = NSFW;

src/NSFW.cpp

@@ -56,37 +56,47 @@ NSFW::NSFW(const Napi::CallbackInfo &info):
 
     // errorCallback
     Napi::Value maybeErrorCallback = options["errorCallback"];
-    if (options.Has("errorCallback") && !maybeErrorCallback.IsFunction()) {
+    if (options.Has("errorCallback") && !maybeErrorCallback.IsFunction() && !maybeErrorCallback.IsNull() && !maybeErrorCallback.IsUndefined()) {
       throw Napi::TypeError::New(env, "options.errorCallback must be a function.");
     }
 
-    mErrorCallback = Napi::ThreadSafeFunction::New(
-      env,
-      maybeErrorCallback.IsFunction()
-        ? maybeErrorCallback.As<Napi::Function>()
-        : Napi::Function::New(env, [](const Napi::CallbackInfo &info) {}),
-      "nsfw",
-      0,
-      1
-    );
+    if (maybeErrorCallback.IsFunction()) {
+      mErrorCallback = Napi::ThreadSafeFunction::New(
+        env,
+        maybeErrorCallback.As<Napi::Function>(),
+        "nsfw",
+        0,
+        1
+      );
+    } else {
+      mErrorCallback = Napi::ThreadSafeFunction::New(
+        env,
+        Napi::Function::New(env, [](const Napi::CallbackInfo &info) {}),
+        "nsfw",
+        0,
+        1
+      );
+    }
 
     // excludedPaths
     Napi::Value maybeExcludedPaths = options["excludedPaths"];
     if (options.Has("excludedPaths") && !maybeExcludedPaths.IsArray()) {
       throw Napi::TypeError::New(env, "options.excludedPaths must be an array.");
     }
-    Napi::Array paths = maybeExcludedPaths.As<Napi::Array>();
-    for(uint32_t i = 0; i < paths.Length(); i++) {
-      Napi::Value path = paths[i];
-      if (path.IsString())
-      {
-        std::string str = path.ToString().Utf8Value();
-        if (str.back() == '/') {
-          str.pop_back();
+    if (maybeExcludedPaths.IsArray()) {
+      Napi::Array paths = maybeExcludedPaths.As<Napi::Array>();
+      for(uint32_t i = 0; i < paths.Length(); i++) {
+        Napi::Value path = paths[i];
+        if (path.IsString())
+        {
+          std::string str = path.ToString().Utf8Value();
+          if (!str.empty() && (str.back() == '/' || str.back() == '\\')) {
+            str.pop_back();
+          }
+          mExcludedPaths.push_back(str);
+        } else {
+          throw Napi::TypeError::New(env, "options.excludedPaths elements must be strings.");
         }
-        mExcludedPaths.push_back(str);
-      } else {
-        throw Napi::TypeError::New(env, "options.excludedPaths elements must be strings.");
       }
     }
 
@@ -99,15 +109,14 @@ NSFW::NSFW(const Napi::CallbackInfo &info):
 
 NSFW::~NSFW() {
   if (mRunning) {
-    mFinalizing = true;
     {
       std::lock_guard<std::mutex> lock(mRunningLock);
       mRunning = false;
     }
     mWaitPoolEvents.notify_one();
-  }
-  if (mPollThread.joinable()) {
-    mPollThread.join();
+    if (mPollThread.joinable()) {
+      mPollThread.join();
+    }
   }
   if (gcEnabled) {
     instanceCount--;
@@ -160,12 +169,12 @@ void NSFW::StartWorker::OnOK() {
   switch (mStatus) {
     case ALREADY_RUNNING:
       mNSFW->Unref();
-      mDeferred.Reject(Napi::Error::New(env, "This NSFW cannot be started, because it is already running.").Value());
+      mDeferred.Reject(Napi::Error::New(env, "NSFW watcher cannot be started: already running.").Value());
       break;
 
     case COULD_NOT_START:
       mNSFW->Unref();
-      mDeferred.Reject(Napi::Error::New(env, "NSFW was unable to start watching that directory.").Value());
+      mDeferred.Reject(Napi::Error::New(env, "NSFW failed to start: unable to watch directory.").Value());
       break;
 
     case STARTED:
@@ -213,6 +222,10 @@ void NSFW::StopWorker::Execute() {
   mNSFW->mWaitPoolEvents.notify_one();
   mNSFW->mPollThread.join();
 
+  // Release ThreadSafeFunction callbacks to prevent keeping event loop alive
+  mNSFW->mErrorCallback.Release();
+  mNSFW->mEventCallback.Release();
+
   std::lock_guard<std::mutex> lock(mNSFW->mInterfaceLock);
   mNSFW->mInterface.reset(nullptr);
   mNSFW->mQueue->clear();
@@ -224,7 +237,7 @@ void NSFW::StopWorker::OnOK() {
     mNSFW->Unref();
     mDeferred.Resolve(Env().Undefined());
   } else {
-    mDeferred.Reject(Napi::Error::New(Env(), "This NSFW cannot be stopped, because it is not running.").Value());
+    mDeferred.Reject(Napi::Error::New(Env(), "NSFW watcher cannot be stopped: not currently running.").Value());
   }
 }
 
@@ -253,7 +266,7 @@ void NSFW::PauseWorker::OnOK() {
   if (mDidPauseEvents) {
     mDeferred.Resolve(Env().Undefined());
   } else {
-    mDeferred.Reject(Napi::Error::New(Env(), "This NSFW could not be paused.").Value());
+    mDeferred.Reject(Napi::Error::New(Env(), "NSFW watcher could not be paused.").Value());
   }
 }
 
@@ -282,7 +295,7 @@ void NSFW::ResumeWorker::OnOK() {
   if (mDidResumeEvents) {
     mDeferred.Resolve(Env().Undefined());
   } else {
-    mDeferred.Reject(Napi::Error::New(Env(), "This NSFW could not be resumed.").Value());
+    mDeferred.Reject(Napi::Error::New(Env(), "NSFW watcher could not be resumed.").Value());
   }
 }
 
@@ -340,7 +353,7 @@ NSFW::UpdateExcludedPathsWorker::UpdateExcludedPathsWorker(Napi::Env env, const
   for(uint32_t i = 0; i < paths.Length(); i++) {
     Napi::Value path = paths[i];
     std::string str = path.ToString().Utf8Value();
-    if (str.back() == '/') {
+    if (!str.empty() && (str.back() == '/' || str.back() == '\\')) {
       str.pop_back();
     }
     mNSFW->mExcludedPaths.push_back(str);
@@ -370,15 +383,21 @@ Napi::Value NSFW::UpdateExcludedPaths(const Napi::CallbackInfo &info) {
 }
 
 void NSFW::updateExcludedPaths() {
-  mInterface->updateExcludedPaths(mExcludedPaths);
+  if (mInterface) {
+    mInterface->updateExcludedPaths(mExcludedPaths);
+  }
 }
 
 void NSFW::pauseQueue() {
-  mQueue->pause();
+  if (mQueue) {
+    mQueue->pause();
+  }
 }
 
 void NSFW::resumeQueue() {
-  mQueue->resume();
+  if (mQueue) {
+    mQueue->resume();
+  }
 }
 
 void NSFW::pollForEvents() {
@@ -443,13 +462,6 @@ void NSFW::pollForEvents() {
       }
     );
   }
-
-  // If we are destroying NFSW object (destructor) we cannot release the thread safe functions at this point
-  // or we get a segfault
-  if (!mFinalizing) {
-    mErrorCallback.Release();
-    mEventCallback.Release();
-  }
 }
 
 Napi::Value NSFW::ExcludedPaths() {