[Debugger] Fix nested JS callback registration (fix #2147) (#2157)

* [Debugger] Fix nested JS callback registration (fix #2147)

* [Debugger] Handle JS socket ungraceful disconnect

Source/Project64/UserInterface/Debugger/ScriptAPI/JSSocketWorker.cpp

@@ -100,7 +100,7 @@ void CJSSocketWorker::WorkerProc()
     timeout.tv_sec = 0;
     timeout.tv_usec = TIMEOUT_MS * 1000;
 
-    bool bWritable = false;
+    bool bHaveConnection = false;
     bool bConnectPending = false;
     bool bWritesPending = false;
     bool bRecvClosed = false;
@@ -112,8 +112,8 @@ void CJSSocketWorker::WorkerProc()
 
     if (!bConnectPending)
     {
-        // assume it's already writable
+        // assume it's already connected
-        bWritable = true;
+        bHaveConnection = true;
     }
 
     if (bConnectPending && ProcConnect())
@@ -158,7 +158,7 @@ void CJSSocketWorker::WorkerProc()
             FD_SET(m_Socket, pReadFds);
         }
 
-        if (bWritesPending || !bWritable)
+        if (bWritesPending || !bHaveConnection)
         {
             pWriteFds = &writeFds;
             FD_ZERO(pWriteFds);
@@ -169,6 +169,7 @@ void CJSSocketWorker::WorkerProc()
         if (numFds == SOCKET_ERROR)
         {
             JSEmitError("select() error");
+            break;
         }
 
         if (numFds == 0)
@@ -176,11 +177,14 @@ void CJSSocketWorker::WorkerProc()
             continue;
         }
 
-        if (pWriteFds && FD_ISSET(m_Socket, pWriteFds))
+        bool bWritable = pWriteFds && FD_ISSET(m_Socket, pWriteFds);
+        bool bReadable = pReadFds && FD_ISSET(m_Socket, pReadFds);
+
+        if (bWritable && !m_Queue.bSendClosed)
         {
-            if (!bWritable)
+            if (!bHaveConnection)
             {
-                bWritable = true;
+                bHaveConnection = true;
                 JSEmitConnect();
             }
 
@@ -189,8 +193,13 @@ void CJSSocketWorker::WorkerProc()
                 ProcSendData();
             }
         }
+        else if (bHaveConnection)
+        {
+            JSEmitError("connection reset");
+            break;
+        }
 
-        if (pReadFds && FD_ISSET(m_Socket, pReadFds))
+        if (bReadable)
         {
             ProcRecvData();
         }

Source/Project64/UserInterface/Debugger/ScriptAPI/ScriptAPI.cpp

@@ -375,7 +375,7 @@ JSAppCallbackID ScriptAPI::AddAppCallback(duk_context* ctx, duk_idx_t callbackId
 JSAppCallbackID ScriptAPI::AddAppCallback(duk_context* ctx, JSAppHookID hookId, JSAppCallback& callback)
 {
     CScriptInstance* inst = GetInstance(ctx);
-    JSAppCallbackID callbackId = inst->System()->QueueAddAppCallback(hookId, callback);
+    JSAppCallbackID callbackId = inst->System()->RawAddAppCallback(hookId, callback);
 
     if(callbackId == JS_INVALID_CALLBACK)
     {
@@ -429,7 +429,7 @@ duk_ret_t ScriptAPI::js__AppCallbackFinalizer(duk_context* ctx)
     JSAppCallbackID callbackId = (JSAppCallbackID)duk_get_uint(ctx, -1);
     duk_pop_n(ctx, 2);
 
-    inst->System()->QueueRemoveAppCallback(hookId, callbackId);
+    inst->System()->RawRemoveAppCallback(hookId, callbackId);
 
     return 0;
 }
@@ -531,6 +531,8 @@ duk_ret_t ScriptAPI::js__Emitter_emit(duk_context* ctx)
     duk_get_prop_string(ctx, -1, eventName);
     duk_enum(ctx, -1, 0);
 
+    int count = 0;
+
     while (duk_next(ctx, -1, (duk_bool_t)true))
     {
         duk_push_this(ctx);
@@ -546,6 +548,14 @@ duk_ret_t ScriptAPI::js__Emitter_emit(duk_context* ctx)
         }
 
         duk_pop_n(ctx, 2);
+        count++;
+    }
+
+    // throw if there are no listeners for error event
+    if (count == 0 && strcmp("error", eventName) == 0)
+    {
+        duk_dup(ctx, 1);
+        duk_throw(ctx);
     }
 
     return 0;

Source/Project64/UserInterface/Debugger/ScriptSystem.cpp

@@ -168,6 +168,7 @@ bool CScriptSystem::HaveAppCallbacks(JSAppHookID hookId)
 void CScriptSystem::InvokeAppCallbacks(JSAppHookID hookId, void* env)
 {
     CGuard guard(m_InstancesCS);
+    RefreshCallbackMaps();
 
     JSAppCallbackList& callbacks = m_AppCallbackHooks[hookId];
 
@@ -175,7 +176,7 @@ void CScriptSystem::InvokeAppCallbacks(JSAppHookID hookId, void* env)
 
     for (JSAppCallback& callback : callbacks)
     {
-        if (!callback.m_ConditionFunc(&callback, env))
+        if (callback.m_bDisabled || !callback.m_ConditionFunc(&callback, env))
         {
             continue;
         }
@@ -257,68 +258,28 @@ void CScriptSystem::UpdateCpuCbListInfo(volatile JSCpuCbListInfo& info, JSAppCal
 
 void CScriptSystem::RefreshCallbackMaps()
 {
-    for (JSQueuedCallbackRemove& cbRemove : m_CbRemoveQueue)
+    for (JSAppCallbackList& callbacks : m_AppCallbackHooks)
     {
-        RawRemoveAppCallback(cbRemove.hookId, cbRemove.callbackId);
+        JSAppCallbackList::iterator it = callbacks.begin();
-    }
 
-    for (JSQueuedCallbackAdd& cbAdd : m_CbAddQueue)
+        while (it != callbacks.end())
-    {
+        {
-        RawAddAppCallback(cbAdd.hookId, cbAdd.callback);
+            if (it->m_bDisabled)
+            {
+                callbacks.erase(it);
+            }
+            else
+            {
+                it++;
+            }
+        }
     }
 
-    m_CbRemoveQueue.clear();
-    m_CbAddQueue.clear();
-
     UpdateCpuCbListInfo(m_CpuExecCbInfo, m_AppCallbackHooks[JS_HOOK_CPU_EXEC]);
     UpdateCpuCbListInfo(m_CpuReadCbInfo, m_AppCallbackHooks[JS_HOOK_CPU_READ]);
     UpdateCpuCbListInfo(m_CpuWriteCbInfo, m_AppCallbackHooks[JS_HOOK_CPU_WRITE]);
 }
 
-JSAppCallbackID CScriptSystem::QueueAddAppCallback(JSAppHookID hookId, JSAppCallback callback)
-{
-    if (hookId >= JS_NUM_APP_HOOKS)
-    {
-        return JS_INVALID_CALLBACK;
-    }
-
-    callback.m_CallbackId = m_NextAppCallbackId++;
-    m_CbAddQueue.push_back({ hookId, callback });
-    callback.m_Instance->IncRefCount();
-    return callback.m_CallbackId;
-}
-
-void CScriptSystem::QueueRemoveAppCallback(JSAppHookID hookId, JSAppCallbackID callbackId)
-{
-    // todo also remove from addqueue
-
-    for (size_t i = 0; i < m_CbRemoveQueue.size(); i++)
-    {
-        if (m_CbRemoveQueue[i].hookId == hookId &&
-            m_CbRemoveQueue[i].callbackId == callbackId)
-        {
-            return;
-        }
-    }
-
-    JSAppCallbackList::iterator it;
-    for (it = m_AppCallbackHooks[hookId].begin(); it != m_AppCallbackHooks[hookId].end(); it++)
-    {
-        if (it->m_CallbackId == callbackId)
-        {
-            break;
-        }
-    }
-
-    if (it == m_AppCallbackHooks[hookId].end())
-    {
-        return;
-    }
-
-    m_CbRemoveQueue.push_back({ hookId, callbackId });
-    it->m_Instance->DecRefCount();
-}
-
 void CScriptSystem::DoMouseEvent(JSAppHookID hookId, int x, int y, DWORD uMsg)
 {
     int button = -1;
@@ -474,7 +435,7 @@ void CScriptSystem::OnSweep(bool bIfDone)
         {
             NotifyStatus(inst->Name().c_str(), JS_STATUS_STOPPED);
             delete inst;
-            m_Instances.erase(it++);
+            m_Instances.erase(it);
         }
         else
         {
@@ -509,28 +470,34 @@ bool CScriptSystem::RawRemoveInstance(const char *name)
     return true;
 }
 
-void CScriptSystem::RawAddAppCallback(JSAppHookID hookId, JSAppCallback& callback)
+JSAppCallbackID CScriptSystem::RawAddAppCallback(JSAppHookID hookId, JSAppCallback& callback)
 {
     if(hookId >= JS_NUM_APP_HOOKS)
     {
-        return;
+        return JS_INVALID_CALLBACK;
     }
 
+    callback.m_Instance->IncRefCount();
+    callback.m_CallbackId = m_NextAppCallbackId++;
     m_AppCallbackHooks[hookId].push_back(callback);
     m_AppCallbackCount++;
+    return callback.m_CallbackId;
 }
 
 void CScriptSystem::RawRemoveAppCallback(JSAppHookID hookId, JSAppCallbackID callbackId)
 {
-    JSAppCallbackList::iterator it;
+    JSAppCallbackList::iterator it = m_AppCallbackHooks[hookId].begin();
-    for (it = m_AppCallbackHooks[hookId].begin(); it != m_AppCallbackHooks[hookId].end(); it++)
+    while (it != m_AppCallbackHooks[hookId].end())
     {
         if (it->m_CallbackId == callbackId)
         {
-            m_AppCallbackHooks[hookId].erase(it);
             m_AppCallbackCount--;
+            it->m_Instance->DecRefCount();
+            it->m_bDisabled = true;
             return;
         }
+
+        it++;
     }
 }
 

Source/Project64/UserInterface/Debugger/ScriptSystem.h

@@ -17,18 +17,6 @@ class CScriptSystem
     typedef std::map<JSInstanceName, JSInstanceStatus> JSInstanceStatusMap;
     typedef std::vector<JSSysCommand> JSSysCommandQueue;
 
-    struct JSQueuedCallbackAdd
-    {
-        JSAppHookID hookId;
-        JSAppCallback callback;
-    };
-
-    struct JSQueuedCallbackRemove
-    {
-        JSAppHookID hookId;
-        JSAppCallbackID callbackId;
-    };
-
     enum { JS_CPU_CB_RANGE_CACHE_SIZE = 256 };
 
     struct JSCpuCbListInfo
@@ -55,9 +43,6 @@ class CScriptSystem
     JSAppCallbackList   m_AppCallbackHooks[JS_NUM_APP_HOOKS];
     JSAppCallbackID     m_NextAppCallbackId;
     
-    std::vector<JSQueuedCallbackRemove> m_CbRemoveQueue;
-    std::vector<JSQueuedCallbackAdd> m_CbAddQueue;
-
     volatile size_t   m_AppCallbackCount;
 
     volatile JSCpuCbListInfo m_CpuExecCbInfo;
@@ -125,9 +110,6 @@ public:
     static void UpdateCpuCbListInfo(volatile JSCpuCbListInfo& info, JSAppCallbackList& callbacks);
 
     void DoMouseEvent(JSAppHookID hookId, int x, int y, DWORD uMsg = (DWORD)-1);
-
-    JSAppCallbackID QueueAddAppCallback(JSAppHookID hookId, JSAppCallback callback);
-    void QueueRemoveAppCallback(JSAppHookID hookId, JSAppCallbackID callbackId);
     void InvokeAppCallbacks(JSAppHookID hookId, void* env = nullptr);
 
     void ExecAutorunList();
@@ -135,6 +117,9 @@ public:
     void LoadAutorunList();
     void SaveAutorunList();
 
+    JSAppCallbackID RawAddAppCallback(JSAppHookID hookId, JSAppCallback& callback);
+    void RawRemoveAppCallback(JSAppHookID hookId, JSAppCallbackID callbackId);
+
 private:
     inline bool HaveCpuCallbacks(volatile JSCpuCbListInfo& info, JSAppCallbackList& callbacks, uint32_t address)
     {
@@ -189,9 +174,6 @@ private:
     void OnSweep(bool bIfDone);
 
     bool RawRemoveInstance(const char* key);
-    
-    void RawAddAppCallback(JSAppHookID hookId, JSAppCallback& callback);
-    void RawRemoveAppCallback(JSAppHookID hookId, JSAppCallbackID callbackId);
     void RefreshCallbackMaps();
 
     static stdstr FixStringReturns(const char* str);

Source/Project64/UserInterface/Debugger/ScriptTypes.h

@@ -77,6 +77,7 @@ struct JSAppCallback
 {
     // assigned by scriptsys when this is added to a callback map
     JSAppCallbackID          m_CallbackId;
+    bool                     m_bDisabled;
 
     CScriptInstance         *m_Instance;
     void                    *m_DukFuncHeapPtr;
@@ -98,12 +99,13 @@ struct JSAppCallback
                   JSAppCallbackCondFunc condFunc = nullptr,
                   JSDukArgSetupFunc argSetupFunc = nullptr,
                   JSAppCallbackCleanupFunc cleanupFunc = nullptr) :
+        m_CallbackId(JS_INVALID_CALLBACK),
+        m_bDisabled(false),
         m_Instance(instance),
         m_DukFuncHeapPtr(dukFuncHeapPtr),
         m_ConditionFunc(condFunc),
         m_DukArgSetupFunc(argSetupFunc),
-        m_CleanupFunc(cleanupFunc),
+        m_CleanupFunc(cleanupFunc)
-        m_CallbackId(JS_INVALID_CALLBACK)
     {
         if (m_ConditionFunc == nullptr)
         {
@@ -114,12 +116,13 @@ struct JSAppCallback
     }
 
     JSAppCallback() :
+        m_CallbackId(JS_INVALID_CALLBACK),
+        m_bDisabled(false),
         m_Instance(nullptr),
         m_DukFuncHeapPtr(nullptr),
         m_ConditionFunc(nullptr),
         m_DukArgSetupFunc(nullptr),
-        m_CleanupFunc(nullptr),
+        m_CleanupFunc(nullptr)
-        m_CallbackId(JS_INVALID_CALLBACK)
     {
         if (m_ConditionFunc == nullptr)
         {
@@ -163,7 +166,8 @@ struct JSHookSpTaskEnv
     uint32_t yieldDataSize;
 };
 
-struct JSHookPiDmaEnv {
+struct JSHookPiDmaEnv
+{
     int      direction;
     uint32_t dramAddress;
     uint32_t cartAddress;
@@ -208,6 +212,9 @@ struct JSSysCMethodCall
 
     ~JSSysCMethodCall()
     {
-        delete[] (char*)m_ArgSetupParam;
+        if (m_ArgSetupParam != nullptr)
+        {
+            delete[](char*)m_ArgSetupParam;
+        }
     }
 };