From 16620c09c8652d0c78f5a76db7f0baa5fa3c719d Mon Sep 17 00:00:00 2001 From: apnadkarni Date: Sat, 21 Oct 2023 11:29:44 +0000 Subject: Test for Windows socket crash --- tests/socket.test | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/socket.test b/tests/socket.test index 82e908a..1d4e0fc 100644 --- a/tests/socket.test +++ b/tests/socket.test @@ -2581,6 +2581,14 @@ foreach {servip sc} $x { } } +test socket-bug-31fc36fe47 "Crash listening in multiple threads" \ + -constraints thread -body { + close [socket -server xxx 0] + set tid [thread::create] + thread::send $tid "socket -server accept 0" + thread::release $tid + } -result {} + ::tcltest::cleanupTests flush stdout return -- cgit v0.12 From 492a02091d66a47de657f3f6cf527100a4485b96 Mon Sep 17 00:00:00 2001 From: apnadkarni Date: Sat, 21 Oct 2023 13:00:54 +0000 Subject: Proposed fix --- tests/socket.test | 4 +- win/tclWinSock.c | 135 +++++++++++++++++++++++++++--------------------------- 2 files changed, 69 insertions(+), 70 deletions(-) diff --git a/tests/socket.test b/tests/socket.test index 1d4e0fc..b628404 100644 --- a/tests/socket.test +++ b/tests/socket.test @@ -2585,9 +2585,9 @@ test socket-bug-31fc36fe47 "Crash listening in multiple threads" \ -constraints thread -body { close [socket -server xxx 0] set tid [thread::create] - thread::send $tid "socket -server accept 0" + thread::send $tid {close [socket -server accept 0]} thread::release $tid - } -result {} + } -result 0 ::tcltest::cleanupTests flush stdout diff --git a/win/tclWinSock.c b/win/tclWinSock.c index 7dd5b41..c34835b 100644 --- a/win/tclWinSock.c +++ b/win/tclWinSock.c @@ -229,7 +229,7 @@ static WNDCLASSW windowClass; static int TcpConnect(Tcl_Interp *interp, TcpState *state); -static void InitSockets(void); +static void InitSocketWindowClass(void); static TcpState * NewSocketInfo(SOCKET socket); static void SocketExitHandler(void *clientData); static LRESULT CALLBACK SocketProc(HWND hwnd, UINT message, WPARAM wParam, @@ -419,11 +419,11 @@ Tcl_GetHostName(void) * * TclInitSockets -- * - * This function just calls InitSockets(), but is protected by a mutex. + * Initialization of sockets for the thread. Also creates message + * handling window class for the process if needed. * * Results: - * Returns TCL_OK if the system supports sockets, or TCL_ERROR with an - * error in interp (if non-NULL). + * Nothing. Panics on failure. * * Side effects: * If not already prepared, initializes the TSD structure and socket @@ -436,13 +436,58 @@ Tcl_GetHostName(void) void TclInitSockets() { - if (!initialized) { - Tcl_MutexLock(&socketMutex); - if (!initialized) { - InitSockets(); - } - Tcl_MutexUnlock(&socketMutex); + /* Then Per thread initialization. */ + DWORD id; + ThreadSpecificData *tsdPtr = (ThreadSpecificData *)TclThreadDataKeyGet(&dataKey); + + if (tsdPtr != NULL) { + return; + } + + InitSocketWindowClass(); + + /* + * OK, this thread has never done anything with sockets before. Construct + * a worker thread to handle asynchronous events related to sockets + * assigned to _this_ thread. + */ + + tsdPtr = TCL_TSD_INIT(&dataKey); + tsdPtr->pendingTcpState = NULL; + tsdPtr->socketList = NULL; + tsdPtr->hwnd = NULL; + tsdPtr->threadId = Tcl_GetCurrentThread(); + tsdPtr->readyEvent = CreateEventW(NULL, FALSE, FALSE, NULL); + if (tsdPtr->readyEvent == NULL) { + goto initFailure; + } + tsdPtr->socketListLock = CreateEventW(NULL, FALSE, TRUE, NULL); + if (tsdPtr->socketListLock == NULL) { + goto initFailure; } + tsdPtr->socketThread = CreateThread(NULL, 256, SocketThread, tsdPtr, 0, + &id); + if (tsdPtr->socketThread == NULL) { + goto initFailure; + } + + SetThreadPriority(tsdPtr->socketThread, THREAD_PRIORITY_HIGHEST); + + /* + * Wait for the thread to signal when the window has been created and if + * it is ready to go. + */ + + WaitForSingleObject(tsdPtr->readyEvent, INFINITE); + + if (tsdPtr->hwnd != NULL) { + Tcl_CreateEventSource(SocketSetupProc, SocketCheckProc, NULL); + return; + } + + initFailure: + Tcl_Panic("InitSockets failed"); + return; } /* @@ -2326,28 +2371,27 @@ TcpAccept( /* *---------------------------------------------------------------------- * - * InitSockets -- + * InitSocketWindowClass -- * - * Registers the event window for the socket notifier code. - * - * Assumes socketMutex is held. + * Registers the event window class for the socket notifier code. + * Caller must not hold socket mutex lock. * * Results: * None. * * Side effects: - * Register a new window class and creates a - * window for use in asynchronous socket notification. + * Register a new window class. * *---------------------------------------------------------------------- */ static void -InitSockets(void) +InitSocketWindowClass(void) { - DWORD id; - ThreadSpecificData *tsdPtr = (ThreadSpecificData *)TclThreadDataKeyGet(&dataKey); - + if (initialized) { + return; + } + Tcl_MutexLock(&socketMutex); if (!initialized) { initialized = 1; TclCreateLateExitHandler(SocketExitHandler, NULL); @@ -2375,57 +2419,12 @@ InitSockets(void) goto initFailure; } } - - /* - * Check for per-thread initialization. - */ - - if (tsdPtr != NULL) { - return; - } - - /* - * OK, this thread has never done anything with sockets before. Construct - * a worker thread to handle asynchronous events related to sockets - * assigned to _this_ thread. - */ - - tsdPtr = TCL_TSD_INIT(&dataKey); - tsdPtr->pendingTcpState = NULL; - tsdPtr->socketList = NULL; - tsdPtr->hwnd = NULL; - tsdPtr->threadId = Tcl_GetCurrentThread(); - tsdPtr->readyEvent = CreateEventW(NULL, FALSE, FALSE, NULL); - if (tsdPtr->readyEvent == NULL) { - goto initFailure; - } - tsdPtr->socketListLock = CreateEventW(NULL, FALSE, TRUE, NULL); - if (tsdPtr->socketListLock == NULL) { - goto initFailure; - } - tsdPtr->socketThread = CreateThread(NULL, 256, SocketThread, tsdPtr, 0, - &id); - if (tsdPtr->socketThread == NULL) { - goto initFailure; - } - - SetThreadPriority(tsdPtr->socketThread, THREAD_PRIORITY_HIGHEST); - - /* - * Wait for the thread to signal when the window has been created and if - * it is ready to go. - */ - - WaitForSingleObject(tsdPtr->readyEvent, INFINITE); - - if (tsdPtr->hwnd != NULL) { - Tcl_CreateEventSource(SocketSetupProc, SocketCheckProc, NULL); - return; - } + Tcl_MutexUnlock(&socketMutex); + return; initFailure: + Tcl_MutexUnlock(&socketMutex); /* Probably pointless before panicing */ Tcl_Panic("InitSockets failed"); - return; } /* -- cgit v0.12