Fixed a few bugs with the scripts system. I suspect there are more; the

threading code needs some refactoring.  List of changes:

* Script threads no longer call the debugger UI (e.g. refresh) nor do
  they call the script system's cleanup function
  (deleteStoppedInstances).
* Changed API.js Server.on method to queue calls to nativeAccept if .listen()
  hasn't been called yet (otherwise winsock gives errors).
* Added various paranoia thread locks.
* Forced termination of script threads now works.  The (commented) call
  to TerminateThread didn't work because getCurrentThread() doesn't return
  a "real" HANDLE.  The solution is to pass the result of getCurrentThread
  to DuplicateHandle to get a "real" handle.
This commit is contained in:
Joe Eagar 2019-04-28 23:28:12 -07:00
parent 83f445f050
commit 82463fbdd8
8 changed files with 100 additions and 29 deletions

View File

@ -918,12 +918,26 @@ function Server(settings)
{ {
var _this = this; var _this = this;
var _fd = _native.sockCreate() var _fd = _native.sockCreate()
var _listening = false;
var _queued_accept = false;
var _onconnection = function(socket){} var _onconnection = function(socket){}
this.listen = function (port) {
if (_native.sockListen(_fd, port || 80)) {
_listening = true;
} else {
throw new Error("failed to listen");
}
if (_queued_accept) {
_native.sockAccept(_fd, _acceptClient);
}
}
if(settings.port) if(settings.port)
{ {
_native.sockListen(_fd, settings.port || 80) this.listen(settings.port || 80);
} }
// Intermediate callback // Intermediate callback
@ -934,18 +948,17 @@ function Server(settings)
_native.sockAccept(_fd, _acceptClient) _native.sockAccept(_fd, _acceptClient)
} }
this.listen = function(port)
{
_native.sockListen(_fd, port)
}
this.on = function(eventType, callback) this.on = function(eventType, callback)
{ {
switch(eventType) switch(eventType)
{ {
case 'connection': case 'connection':
_onconnection = callback _onconnection = callback;
_native.sockAccept(_fd, _acceptClient) if (_listening) {
_native.sockAccept(_fd, _acceptClient);
} else {
_queued_accept = true;
}
break; break;
} }
} }

View File

@ -19,12 +19,14 @@ CDebugScripts::CDebugScripts(CDebuggerUI* debugger) :
CDebugDialog<CDebugScripts>(debugger) CDebugDialog<CDebugScripts>(debugger)
{ {
m_SelectedScriptName = (char*)malloc(MAX_PATH); m_SelectedScriptName = (char*)malloc(MAX_PATH);
//CScriptSystem::SetScriptsWindow(this); InitializeCriticalSection(&m_CriticalSection);
//CScriptSystem::SetScriptsWindow(this);
} }
CDebugScripts::~CDebugScripts(void) CDebugScripts::~CDebugScripts(void)
{ {
free(m_SelectedScriptName); DeleteCriticalSection(&m_CriticalSection);
free(m_SelectedScriptName);
} }
LRESULT CDebugScripts::OnInitDialog(UINT /*uMsg*/, WPARAM /*wParam*/, LPARAM /*lParam*/, BOOL& /*bHandled*/) LRESULT CDebugScripts::OnInitDialog(UINT /*uMsg*/, WPARAM /*wParam*/, LPARAM /*lParam*/, BOOL& /*bHandled*/)
@ -91,6 +93,8 @@ void CDebugScripts::ConsolePrint(const char* text)
void CDebugScripts::RefreshConsole() void CDebugScripts::RefreshConsole()
{ {
EnterCriticalSection(&m_CriticalSection);
m_Debugger->OpenScriptsWindow(); m_Debugger->OpenScriptsWindow();
CScriptSystem* scriptSystem = m_Debugger->ScriptSystem(); CScriptSystem* scriptSystem = m_Debugger->ScriptSystem();
vector<char*>* logData = scriptSystem->LogData(); vector<char*>* logData = scriptSystem->LogData();
@ -101,6 +105,8 @@ void CDebugScripts::RefreshConsole()
free((*logData)[0]); free((*logData)[0]);
logData->erase(logData->begin() + 0); logData->erase(logData->begin() + 0);
} }
LeaveCriticalSection(&m_CriticalSection);
} }
void CDebugScripts::ConsoleClear() void CDebugScripts::ConsoleClear()
@ -133,13 +139,16 @@ void CDebugScripts::ConsoleCopy()
void CDebugScripts::RefreshList() void CDebugScripts::RefreshList()
{ {
int nIndex = m_ScriptList.GetSelectedIndex(); EnterCriticalSection(&m_CriticalSection);
int nIndex = m_ScriptList.GetSelectedIndex();
CPath SearchPath("Scripts", "*"); CPath SearchPath("Scripts", "*");
if (!SearchPath.FindFirst(CPath::FIND_ATTRIBUTE_ALLFILES)) if (!SearchPath.FindFirst(CPath::FIND_ATTRIBUTE_ALLFILES))
{ {
return; LeaveCriticalSection(&m_CriticalSection);
return;
} }
m_ScriptList.SetRedraw(false); m_ScriptList.SetRedraw(false);
@ -159,6 +168,8 @@ void CDebugScripts::RefreshList()
m_ScriptList.SelectItem(nIndex); m_ScriptList.SelectItem(nIndex);
RefreshStatus(); RefreshStatus();
} }
LeaveCriticalSection(&m_CriticalSection);
} }
LRESULT CDebugScripts::OnClicked(WORD /*wNotifyCode*/, WORD wID, HWND /*hWndCtl*/, BOOL& /*bHandled*/) LRESULT CDebugScripts::OnClicked(WORD /*wNotifyCode*/, WORD wID, HWND /*hWndCtl*/, BOOL& /*bHandled*/)
@ -199,7 +210,8 @@ LRESULT CDebugScripts::OnScriptListDblClicked(NMHDR* pNMHDR)
void CDebugScripts::RefreshStatus() void CDebugScripts::RefreshStatus()
{ {
INSTANCE_STATE state = m_Debugger->ScriptSystem()->GetInstanceState(m_SelectedScriptName); EnterCriticalSection(&m_CriticalSection);
INSTANCE_STATE state = m_Debugger->ScriptSystem()->GetInstanceState(m_SelectedScriptName);
char* szState = ""; char* szState = "";
switch (state) switch (state)
@ -222,6 +234,8 @@ void CDebugScripts::RefreshStatus()
{ {
m_EvalEdit.EnableWindow(FALSE); m_EvalEdit.EnableWindow(FALSE);
} }
LeaveCriticalSection(&m_CriticalSection);
} }
LRESULT CDebugScripts::OnScriptListClicked(NMHDR* pNMHDR) LRESULT CDebugScripts::OnScriptListClicked(NMHDR* pNMHDR)
@ -397,5 +411,7 @@ void CDebugScripts::RunSelected()
void CDebugScripts::StopSelected() void CDebugScripts::StopSelected()
{ {
m_Debugger->ScriptSystem()->StopScript(m_SelectedScriptName); m_Debugger->ScriptSystem()->StopScript(m_SelectedScriptName);
//m_Debugger->Debug_RefreshScriptsWindow();
} }

View File

@ -89,6 +89,7 @@ private:
char* m_SelectedScriptName; char* m_SelectedScriptName;
void RefreshStatus(); void RefreshStatus();
CRITICAL_SECTION m_CriticalSection;
public: public:
enum { IDD = IDD_Debugger_Scripts }; enum { IDD = IDD_Debugger_Scripts };

View File

@ -46,7 +46,9 @@ CDebuggerUI::CDebuggerUI() :
m_DMALog = new CDMALog(); m_DMALog = new CDMALog();
m_CPULog = new CCPULog(); m_CPULog = new CCPULog();
CSymbols::InitializeCriticalSection(); InitializeCriticalSection(&m_CriticalSection);
CSymbols::InitializeCriticalSection();
g_Settings->RegisterChangeCB(GameRunning_InReset, this, (CSettings::SettingChangedFunc)GameReset); g_Settings->RegisterChangeCB(GameRunning_InReset, this, (CSettings::SettingChangedFunc)GameReset);
g_Settings->RegisterChangeCB(Debugger_SteppingOps, this, (CSettings::SettingChangedFunc)SteppingOpsChanged); g_Settings->RegisterChangeCB(Debugger_SteppingOps, this, (CSettings::SettingChangedFunc)SteppingOpsChanged);
g_Settings->RegisterChangeCB(GameRunning_CPU_Running, this, (CSettings::SettingChangedFunc)GameCpuRunningChanged); g_Settings->RegisterChangeCB(GameRunning_CPU_Running, this, (CSettings::SettingChangedFunc)GameCpuRunningChanged);
@ -75,6 +77,7 @@ CDebuggerUI::~CDebuggerUI(void)
delete m_CPULog; delete m_CPULog;
CSymbols::DeleteCriticalSection(); CSymbols::DeleteCriticalSection();
DeleteCriticalSection(&m_CriticalSection);
} }
void CDebuggerUI::SteppingOpsChanged(CDebuggerUI * _this) void CDebuggerUI::SteppingOpsChanged(CDebuggerUI * _this)

View File

@ -54,10 +54,11 @@ CScriptInstance::CScriptInstance(CDebuggerUI* debugger)
CScriptInstance::~CScriptInstance() CScriptInstance::~CScriptInstance()
{ {
UncacheInstance(this); UncacheInstance(this);
//TerminateThread(m_hThread, 0);
//CloseHandle(m_hThread);
DeleteCriticalSection(&m_CriticalSection); DeleteCriticalSection(&m_CriticalSection);
duk_destroy_heap(m_Ctx); duk_destroy_heap(m_Ctx);
TerminateThread(m_hThread, 0);
CloseHandle(m_hThread);
} }
void CScriptInstance::Start(char* path) void CScriptInstance::Start(char* path)
@ -94,8 +95,9 @@ void CScriptInstance::SetState(INSTANCE_STATE state)
void CScriptInstance::StateChanged() void CScriptInstance::StateChanged()
{ {
// todo mutex might be needed here // todo mutex might be needed here
m_Debugger->Debug_RefreshScriptsWindow();
m_ScriptSystem->DeleteStoppedInstances(); //m_Debugger->Debug_RefreshScriptsWindow();
//m_ScriptSystem->DeleteStoppedInstances();
} }
DWORD CALLBACK CScriptInstance::StartThread(CScriptInstance* _this) DWORD CALLBACK CScriptInstance::StartThread(CScriptInstance* _this)
@ -106,7 +108,7 @@ DWORD CALLBACK CScriptInstance::StartThread(CScriptInstance* _this)
void CScriptInstance::StartScriptProc() void CScriptInstance::StartScriptProc()
{ {
m_hThread = GetCurrentThread(); DuplicateHandle(GetCurrentProcess(), GetCurrentThread(), GetCurrentProcess(), &m_hThread, 0, FALSE, DUPLICATE_SAME_ACCESS);
SetState(STATE_STARTED); SetState(STATE_STARTED);
bool bWasUnpaused = false; bool bWasUnpaused = false;
@ -694,8 +696,9 @@ duk_ret_t CScriptInstance::js_ioSockAccept(duk_context* ctx)
IOLISTENER* lpListener = _this->AddListener(fd, EVENT_ACCEPT, jsCallback, data, 0); IOLISTENER* lpListener = _this->AddListener(fd, EVENT_ACCEPT, jsCallback, data, 0);
lpListener->childFd = _this->CreateSocket(); lpListener->childFd = _this->CreateSocket();
static DWORD unused;
AcceptEx( int ok = AcceptEx(
(SOCKET)fd, (SOCKET)fd,
(SOCKET)lpListener->childFd, (SOCKET)lpListener->childFd,
lpListener->data, // local and remote SOCKADDR lpListener->data, // local and remote SOCKADDR
@ -706,7 +709,14 @@ duk_ret_t CScriptInstance::js_ioSockAccept(duk_context* ctx)
(LPOVERLAPPED)lpListener (LPOVERLAPPED)lpListener
); );
duk_pop_n(ctx, 2); duk_pop_n(ctx, 2);
if (!ok) {
duk_push_boolean(ctx, false);
} else {
duk_push_boolean(ctx, true);
}
return 1; return 1;
} }

View File

@ -24,6 +24,8 @@ CScriptSystem::CScriptSystem(CDebuggerUI* debugger)
m_Debugger = debugger; m_Debugger = debugger;
InitializeCriticalSection(&m_CriticalSection);
m_HookCPUExec = new CScriptHook(this); m_HookCPUExec = new CScriptHook(this);
m_HookCPUExecOpcode = new CScriptHook(this); m_HookCPUExecOpcode = new CScriptHook(this);
m_HookCPURead = new CScriptHook(this); m_HookCPURead = new CScriptHook(this);
@ -60,6 +62,8 @@ CScriptSystem::~CScriptSystem()
UnregisterHooks(); UnregisterHooks();
free(m_APIScript); free(m_APIScript);
DeleteCriticalSection(&m_CriticalSection);
} }
const char* CScriptSystem::APIScript() const char* CScriptSystem::APIScript()
@ -72,8 +76,11 @@ void CScriptSystem::RunScript(char* path)
CScriptInstance* scriptInstance = new CScriptInstance(m_Debugger); CScriptInstance* scriptInstance = new CScriptInstance(m_Debugger);
char* pathSaved = (char*)malloc(strlen(path)+1); // freed via DeleteStoppedInstances char* pathSaved = (char*)malloc(strlen(path)+1); // freed via DeleteStoppedInstances
strcpy(pathSaved, path); strcpy(pathSaved, path);
m_RunningInstances.push_back({ pathSaved, scriptInstance });
scriptInstance->Start(pathSaved); EnterCriticalSection(&m_CriticalSection);
m_RunningInstances.push_back({ pathSaved, scriptInstance });
LeaveCriticalSection(&m_CriticalSection);
scriptInstance->Start(pathSaved);
} }
void CScriptSystem::StopScript(char* path) void CScriptSystem::StopScript(char* path)
@ -86,10 +93,13 @@ void CScriptSystem::StopScript(char* path)
} }
scriptInstance->ForceStop(); scriptInstance->ForceStop();
DeleteStoppedInstances();
} }
void CScriptSystem::DeleteStoppedInstances() void CScriptSystem::DeleteStoppedInstances()
{ {
EnterCriticalSection(&m_CriticalSection);
int lastIndex = m_RunningInstances.size() - 1; int lastIndex = m_RunningInstances.size() - 1;
for (int i = lastIndex; i >= 0; i--) for (int i = lastIndex; i >= 0; i--)
{ {
@ -101,29 +111,44 @@ void CScriptSystem::DeleteStoppedInstances()
m_RunningInstances.erase(m_RunningInstances.begin() + i); m_RunningInstances.erase(m_RunningInstances.begin() + i);
} }
} }
LeaveCriticalSection(&m_CriticalSection);
} }
INSTANCE_STATE CScriptSystem::GetInstanceState(char* path) INSTANCE_STATE CScriptSystem::GetInstanceState(char* path)
{ {
for (size_t i = 0; i < m_RunningInstances.size(); i++) EnterCriticalSection(&m_CriticalSection);
for (size_t i = 0; i < m_RunningInstances.size(); i++)
{ {
if (strcmp(m_RunningInstances[i].path, path) == 0) if (strcmp(m_RunningInstances[i].path, path) == 0)
{ {
return m_RunningInstances[i].scriptInstance->GetState(); INSTANCE_STATE ret = m_RunningInstances[i].scriptInstance->GetState();
LeaveCriticalSection(&m_CriticalSection);
return ret;
} }
} }
return STATE_INVALID;
LeaveCriticalSection(&m_CriticalSection);
return STATE_INVALID;
} }
CScriptInstance* CScriptSystem::GetInstance(char* path) CScriptInstance* CScriptSystem::GetInstance(char* path)
{ {
EnterCriticalSection(&m_CriticalSection);
for (size_t i = 0; i < m_RunningInstances.size(); i++) for (size_t i = 0; i < m_RunningInstances.size(); i++)
{ {
if (strcmp(m_RunningInstances[i].path, path) == 0) if (strcmp(m_RunningInstances[i].path, path) == 0)
{ {
return m_RunningInstances[i].scriptInstance; CScriptInstance *ret = m_RunningInstances[i].scriptInstance;
LeaveCriticalSection(&m_CriticalSection);
return ret;
} }
} }
LeaveCriticalSection(&m_CriticalSection);
return NULL; return NULL;
} }

View File

@ -58,6 +58,8 @@ private:
CScriptHook* m_HookCPUGPRValue; CScriptHook* m_HookCPUGPRValue;
CScriptHook* m_HookFrameDrawn; CScriptHook* m_HookFrameDrawn;
CRITICAL_SECTION m_CriticalSection;
void RegisterHook(const char* hookId, CScriptHook* cbList); // associate string id with callback list void RegisterHook(const char* hookId, CScriptHook* cbList); // associate string id with callback list
void UnregisterHooks(); void UnregisterHooks();

View File

@ -102,6 +102,7 @@ private:
CDebuggerUI(const CDebuggerUI&); // Disable copy constructor CDebuggerUI(const CDebuggerUI&); // Disable copy constructor
CDebuggerUI& operator=(const CDebuggerUI&); // Disable assignment CDebuggerUI& operator=(const CDebuggerUI&); // Disable assignment
CRITICAL_SECTION m_CriticalSection;
CDumpMemory * m_MemoryDump; CDumpMemory * m_MemoryDump;
CDebugMemoryView * m_MemoryView; CDebugMemoryView * m_MemoryView;
CDebugMemorySearch * m_MemorySearch; CDebugMemorySearch * m_MemorySearch;