fs, bugfix: robust change, avoid potential core-dump when calling `fs…

….watch()`.

.gitignore

@@ -7,5 +7,7 @@
 node_modules
 out
 
+/*.dmp
+
 .vs
 *.vcxproj.user

appveyor.yml

@@ -5,8 +5,8 @@ environment:
   CMAKE_RC_COMPILER: 'llvm-rc.exe'
 
 platform:
-  - x86
   - x64
+  - x86
 
 configuration: Release
 

fibjs/build

@@ -146,7 +146,7 @@ if [ $? != 0 ]; then
 	exit 1
 fi
 
-if [ $HOST_OS == "Windows" ]; then
+if [ "$HOST_OS" = "Windows" ]; then
 	get_windows_vctool_install_path;
 	export VCToolsInstallDir;
 fi

fibjs/include/FSWatcher.h

@@ -20,11 +20,12 @@ class FSWatcher : public FSWatcher_base {
         v8::Local<v8::Function> callback,
         bool persistent = true,
         bool recursive = false)
-        : m_filename(target)
+        : m_closed(false)
+        , m_filename(target)
         , m_Persistent(persistent)
         , m_recursiveForDir(recursive)
-        , m_closed(false)
         , m_proc(NULL)
+        , m_vholder(NULL)
     {
         if (!callback.IsEmpty()) {
             exlib::string _chname = "change";
@@ -60,7 +61,7 @@ class FSWatcher : public FSWatcher_base {
 
         virtual void invoke();
 
-        void on_watched(int events, int status)
+        void on_watched(const char* fullfname, int events, int status)
         {
             Variant v[2];
             exlib::string only_evtType;
@@ -79,43 +80,54 @@ class FSWatcher : public FSWatcher_base {
                 v[0] = "rename";
             } else {
                 m_watcher->onError(CALL_E_INVALID_CALL, "Unknown change event type");
-                m_watcher->close();
                 return;
             }
 
-#ifdef Darwin
-            if (m_fs_handle.realpath)
-                v[1] = m_fs_handle.realpath;
-            else if (m_fs_handle.path)
-                v[1] = m_fs_handle.path;
-#else
-            v[1] = m_fs_handle.path;
-#endif
+            v[1] = fullfname;
 
             m_watcher->_emit("change", v, 2);
             if (only_evtType.length() > 0)
                 m_watcher->_emit(only_evtType, v, 2);
         }
 
-        void finished()
+        static void freeSelfAfterUVHandleStop(uv_handle_t* handle)
         {
-            uv_fs_event_stop(&m_fs_handle);
-
-            delete this;
+            AsyncWatchFSProc* proc = (AsyncWatchFSProc*)(handle->data);
+            delete proc;
         }
 
     public:
         FSWatcher* m_watcher;
         uv_fs_event_t m_fs_handle;
+        exlib::Locker m_locker;
 
     private:
-        static void fs_event_cb(uv_fs_event_t* handle, const char* filename, int events, int status)
+        static void fs_event_cb(uv_fs_event_t* fs_event, const char* filename, int events, int status)
         {
             AsyncWatchFSProc* p = NULL;
-            p = ((AsyncWatchFSProc*)((intptr_t)handle - (intptr_t)&p->m_fs_handle));
+            p = (AsyncWatchFSProc*)((intptr_t)fs_event - (intptr_t)&p->m_fs_handle);
+
+            assert(fs_event == &p->m_fs_handle);
+
+            if (!p)
+                return;
+
+            p->m_locker.lock(p);
+
+            uv_handle_t* handle = (uv_handle_t*)fs_event;
+            if (p->m_watcher->m_closed) {
+                handle->data = (void*)p;
+                uv_close(handle, AsyncWatchFSProc::freeSelfAfterUVHandleStop);
+            } else {
+                size_t size = 2074;
+                char fullname[size + 1];
+
+                uv_fs_event_getpath(fs_event, fullname, &size);
+                fullname[size] = '\0';
 
-            assert(&p->m_fs_handle == handle);
-            p->on_watched(events, status);
+                p->on_watched(fullname, events, status);
+            }
+            p->m_locker.unlock(p);
         }
     };
 
@@ -133,51 +145,56 @@ class FSWatcher : public FSWatcher_base {
             return;
         }
 
+        m_vholder = new ValueHolder(wrap());
+
         if (m_Persistent)
             isolate_ref();
 
-        asyncCall(asyncStart, this);
+        asyncCall(startWatchInNativeThread, this);
     };
+
+    static result_t startWatchInNativeThread(FSWatcher* watcher)
+    {
+        (new AsyncWatchFSProc(watcher))->post(0);
+
+        return 0;
+    }
+
+public:
     // FSWatcher_base
     virtual result_t close()
     {
         if (m_closed)
             return 0;
 
-        if (m_Persistent)
+        m_closed = true;
+
+        if (isPersistent())
             isolate_unref();
 
-        m_closed = true;
+        if (m_vholder)
+            m_vholder.Release();
 
         _emit("close");
 
-        if (m_proc)
-            m_proc->finished();
-
         return 0;
     };
 
-public:
-    static result_t asyncStart(FSWatcher* watcher)
-    {
-        (new AsyncWatchFSProc(watcher))->post(0);
-
-        return 0;
-    }
-
 public:
     const char* get_target() { return m_filename.c_str(); }
     bool isPersistent() { return m_Persistent; }
     bool isRecursiveForDir() { return m_recursiveForDir; }
     void setProc(AsyncWatchFSProc* proc) { m_proc = proc; }
 
+    exlib::atomic m_closed;
+
 protected:
     exlib::string m_filename;
 
     bool m_Persistent;
     bool m_recursiveForDir;
-    bool m_closed;
     AsyncWatchFSProc* m_proc;
+    obj_ptr<ValueHolder> m_vholder;
 };
 }
 

fibjs/program/CMakeLists.txt

@@ -131,7 +131,16 @@ endif()
 if(${OS} STREQUAL "Windows")
 	add_definitions(-DWIN32 -D_LIB -D_CRT_SECURE_NO_WARNINGS -D_CRT_RAND_S -DNOMINMAX)
 	set(flags "${flags} -fms-extensions -fmsc-version=1910 -frtti")
-	set(link_flags "${link_flags} -lwinmm -lws2_32 -lpsapi -ldbghelp -lshlwapi -lurlmon -luserenv -ladvapi32 -lkernel32")
+	list(APPEND win_libs winmm ws2_32 psapi dbghelp shlwapi urlmon userenv advapi32 kernel32)
+	foreach(lib ${win_libs})
+		set(link_flags "${link_flags} -l${lib}")
+	endforeach()
+
+	list(APPEND uv_libraries psapi iphlpapi userenv ws2_32)
+	foreach(lib ${uv_libraries})
+		set(link_flags "${link_flags} -l${lib}")
+	endforeach()
+
 	set(link_flags "${link_flags} -Xlinker //OPT:ICF -Xlinker //ERRORREPORT:PROMPT -Xlinker //NOLOGO -Xlinker //TLBID:1")
 
 	if(${ARCH} STREQUAL "amd64")

fibjs/program/build

@@ -147,7 +147,7 @@ if [ $? != 0 ]; then
 	exit 1
 fi
 
-if [ $HOST_OS == "Windows" ]; then
+if [ "$HOST_OS" = "Windows" ]; then
 	get_windows_vctool_install_path;
 	export VCToolsInstallDir;
 fi

fibjs/src/fs/AsyncFSIO.cpp

@@ -16,6 +16,8 @@ int32_t FSWatcher::AsyncWatchFSProc::post(int32_t v)
 {
     s_uvWait.putTail(this);
     uv_async_send(&s_uv_asyncWatcher);
+
+    return 0;
 }
 
 void FSWatcher::AsyncWatchFSProc::invoke()
@@ -38,7 +40,7 @@ class UVAsyncThread : public exlib::OSThread {
 
     virtual void Run()
     {
-        Runtime rt(NULL);
+        Runtime rtForThread(NULL);
 
         uv_async_init(s_uv_loop, &s_uv_asyncWatcher, AsyncEventCallback);
 
@@ -69,5 +71,7 @@ void initializeUVAsyncThread()
 {
     static UVAsyncThread s_afsIO;
     s_afsIO.start();
+
+    s_afsIO.m_lock.lock();
 }
 }

fibjs/src/fs/fs.cpp

@@ -560,8 +560,6 @@ result_t fs_base::watch(exlib::string fname, v8::Local<v8::Object> options, obj_
 
 result_t fs_base::watch(exlib::string fname, v8::Local<v8::Object> options, v8::Local<v8::Function> callback, obj_ptr<FSWatcher_base>& retVal)
 {
-    obj_ptr<File> pFile = new File();
-
     result_t hr;
     exlib::string safe_name;
     path_base::normalize(fname, safe_name);

test/fswatch_test.js

@@ -52,7 +52,7 @@ describe('fs.watch*', () => {
     }
 
     describe("::close", () => {
-        it("robust: allow multiple times close(though it's pointless)", () => {
+        it("robust: multiple times close(though it's pointless)", () => {
             const relpath = `./fswatch_files/nogit-${uuid.snowflake().hex()}.txt`
             // ensure it existed
             writeFile(resolve_reltocwd(relpath), '')
@@ -66,6 +66,23 @@ describe('fs.watch*', () => {
         });
 
         it("robust: allow multiple times close(though it's pointless)", () => {
+            const relpath = `./fswatch_files/nogit-${uuid.snowflake().hex()}.txt`
+            // ensure it existed
+            writeFile(resolve_reltocwd(relpath), '')
+
+            var j = 0;
+            while (++j < 20) {
+                const watcher = fs.watch(
+                    resolve_reltocwd(relpath)
+                )
+
+                coroutine.sleep(1);
+                for (let i = 0; i < 10; i++)
+                    watcher.close();
+            }
+        });
+
+        it("robust: allow multiple times close(it's pointless)", () => {
             var triggedCallback = false;
             const relpath = `./fswatch_files/nogit-${uuid.snowflake().hex()}.txt`
             // ensure it existed