From 985aec0800fb1d401bc9a323e9daa8a202377118 Mon Sep 17 00:00:00 2001 From: Julian Mesa Date: Mon, 28 Jul 2025 19:52:18 +0200 Subject: [PATCH 1/3] Releases ThreadSafeFunction callbacks on stop to prevent event loop from persisting. --- includes/NSFW.h | 1 - src/NSFW.cpp | 12 ++++-------- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/includes/NSFW.h b/includes/NSFW.h index e65fc17..5b1b0d8 100644 --- a/includes/NSFW.h +++ b/includes/NSFW.h @@ -26,7 +26,6 @@ class NSFW : public Napi::ObjectWrap { std::string mPath; std::thread mPollThread; std::atomic mRunning; - std::atomic mFinalizing; std::condition_variable mWaitPoolEvents; std::mutex mRunningLock; std::vector mExcludedPaths; diff --git a/src/NSFW.cpp b/src/NSFW.cpp index d616a04..611d231 100644 --- a/src/NSFW.cpp +++ b/src/NSFW.cpp @@ -99,7 +99,6 @@ NSFW::NSFW(const Napi::CallbackInfo &info): NSFW::~NSFW() { if (mRunning) { - mFinalizing = true; { std::lock_guard lock(mRunningLock); mRunning = false; @@ -213,6 +212,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 lock(mNSFW->mInterfaceLock); mNSFW->mInterface.reset(nullptr); mNSFW->mQueue->clear(); @@ -443,13 +446,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() { From 6d289681d4a1b742d2f65dbdabbd0ebfcb06899b Mon Sep 17 00:00:00 2001 From: Julian Mesa Date: Mon, 28 Jul 2025 19:52:39 +0200 Subject: [PATCH 2/3] Improves handling of NSFW options and lifecycle Enhances robustness in options processing by validating error callback and excluded paths. Ensures proper thread joining and queue management during object destruction to prevent potential crashes. Refines error messages for clarity and user understanding, providing more informative feedback in case of failures. Adds checks to ensure queue and interface are valid before calling methods on them. --- src/NSFW.cpp | 82 +++++++++++++++++++++++++++++++--------------------- 1 file changed, 49 insertions(+), 33 deletions(-) diff --git a/src/NSFW.cpp b/src/NSFW.cpp index 611d231..a7ce868 100644 --- a/src/NSFW.cpp +++ b/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::New(env, [](const Napi::CallbackInfo &info) {}), - "nsfw", - 0, - 1 - ); + if (maybeErrorCallback.IsFunction()) { + mErrorCallback = Napi::ThreadSafeFunction::New( + env, + maybeErrorCallback.As(), + "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(); - 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(); + 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."); } } @@ -104,9 +114,9 @@ NSFW::~NSFW() { mRunning = false; } mWaitPoolEvents.notify_one(); - } - if (mPollThread.joinable()) { - mPollThread.join(); + if (mPollThread.joinable()) { + mPollThread.join(); + } } if (gcEnabled) { instanceCount--; @@ -159,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: @@ -227,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()); } } @@ -256,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()); } } @@ -285,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()); } } @@ -343,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); @@ -373,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() { From fdba49b80e71645400cf5c44b877fadff962b050 Mon Sep 17 00:00:00 2001 From: Julian Mesa Date: Mon, 28 Jul 2025 19:53:08 +0200 Subject: [PATCH 3/3] Refactors excluded path handling and validation Improves excluded path validation by normalizing paths before comparison, ensuring accurate subdirectory checks. Updates debounceMS validation to include an upper bound, preventing excessively long debounce intervals. Provides a more robust and consistent way of retrieving excluded paths, handling cases where the underlying implementation might not support it. Fixes a bug where file modification detection was not working correctly due to incorrect time comparison. --- index.d.ts | 8 +++--- js/src/index.js | 67 ++++++++++++++++++++++++++++++++++--------------- 2 files changed, 52 insertions(+), 23 deletions(-) diff --git a/index.d.ts b/index.d.ts index 2a8ef52..a73893b 100644 --- a/index.d.ts +++ b/index.d.ts @@ -21,8 +21,10 @@ declare module 'nsfw' { start: () => Promise; /** Returns a Promise that resolves when NSFW object has halted. */ stop: () => Promise; - /** Returns a Promise that resolves when NSFW object has updated the excluded ppaths. */ - updateExcludedPaths: (excludedPaths: [string]) => Promise; + /** Returns a Promise that resolves when NSFW object has updated the excluded paths. */ + updateExcludedPaths: (excludedPaths: string[]) => Promise; + /** Returns a Promise that resolves with the current excluded paths. */ + getExcludedPaths: () => Promise; } 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[]; } } diff --git a/js/src/index.js b/js/src/index.js index 86ae655..61a1eb6 100644 --- a/js/src/index.js +++ b/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;