From 5b0b50f1645f06bb77b57020348dafa498958ad6 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 15 Apr 2021 18:59:13 +0100 Subject: [PATCH 1/4] Use symbolic TLS_OUT_OF_INDEXES constant instead of literal one This constant has the same value but is more readable. No real changes. --- src/msw/thread.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/msw/thread.cpp b/src/msw/thread.cpp index 04d16742d8..86c1ec6411 100644 --- a/src/msw/thread.cpp +++ b/src/msw/thread.cpp @@ -90,8 +90,9 @@ enum wxThreadState // this module globals // ---------------------------------------------------------------------------- -// TLS index of the slot where we store the pointer to the current thread -static DWORD gs_tlsThisThread = 0xFFFFFFFF; +// TLS index of the slot where we store the pointer to the current thread, the +// initial value is special and means that it's not initialized yet +static DWORD gs_tlsThisThread = TLS_OUT_OF_INDEXES; // id of the main thread - the one which can call GUI functions without first // calling wxMutexGuiEnter() @@ -1226,7 +1227,7 @@ bool wxThreadModule::OnInit() { // allocate TLS index for storing the pointer to the current thread gs_tlsThisThread = ::TlsAlloc(); - if ( gs_tlsThisThread == 0xFFFFFFFF ) + if ( gs_tlsThisThread == TLS_OUT_OF_INDEXES ) { // in normal circumstances it will only happen if all other // TLS_MINIMUM_AVAILABLE (>= 64) indices are already taken - in other @@ -1241,7 +1242,7 @@ bool wxThreadModule::OnInit() if ( !::TlsSetValue(gs_tlsThisThread, (LPVOID)0) ) { ::TlsFree(gs_tlsThisThread); - gs_tlsThisThread = 0xFFFFFFFF; + gs_tlsThisThread = TLS_OUT_OF_INDEXES; wxLogSysError(_("Thread module initialization failed: cannot store value in thread local storage")); From f7f874f77a6330771071d3f79126ae8661c52bcd Mon Sep 17 00:00:00 2001 From: Kenneth Porter Date: Thu, 15 Apr 2021 06:45:58 -0700 Subject: [PATCH 2/4] Invalidate TLS slot index when shutting down the library Not doing it could result in using the now invalid or, worse, reassigned to some other DLL, TLS slot if the library is initialized again later. Also add asserts checking that the index is initialized before using it. --- src/msw/thread.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/msw/thread.cpp b/src/msw/thread.cpp index 86c1ec6411..d864ba808a 100644 --- a/src/msw/thread.cpp +++ b/src/msw/thread.cpp @@ -526,6 +526,9 @@ THREAD_RETVAL wxThreadInternal::DoThreadStart(wxThread *thread) wxTRY { // store the thread object in the TLS + wxASSERT_MSG( gs_tlsThisThread != TLS_OUT_OF_INDEXES, + "TLS index not set. Is wx initialized?" ); + if ( !::TlsSetValue(gs_tlsThisThread, thread) ) { wxLogSysError(_("Cannot start thread: error writing TLS.")); @@ -914,6 +917,8 @@ bool wxThreadInternal::Resume() wxThread *wxThread::This() { + wxASSERT_MSG( gs_tlsThisThread != TLS_OUT_OF_INDEXES, + "TLS index not set. Is wx initialized?" ); wxThread *thread = (wxThread *)::TlsGetValue(gs_tlsThisThread); // be careful, 0 may be a valid return value as well @@ -1268,6 +1273,10 @@ void wxThreadModule::OnExit() wxLogLastError(wxT("TlsFree failed.")); } + // invalidate slot index to prevent wxThread from trying to reuse it if the + // library is initialized again later + gs_tlsThisThread = TLS_OUT_OF_INDEXES; + wxDELETE(gs_critsectThreadDelete); if ( gs_critsectGui ) From c1cc71066f43cd465a945d6f57635a19c6664981 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 15 Apr 2021 19:01:11 +0100 Subject: [PATCH 3/4] Remove gs_critsectThreadDelete which was never used This variable was added back in 3ef22a5b02 (trying to fix race conditions. double deletions and memory leaks on thread termination..., 2003-09-21), but not used even then, nor ever since, so just remove it. --- src/msw/thread.cpp | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/msw/thread.cpp b/src/msw/thread.cpp index d864ba808a..f947c2a156 100644 --- a/src/msw/thread.cpp +++ b/src/msw/thread.cpp @@ -109,11 +109,6 @@ static wxCriticalSection *gs_critsectGui = NULL; // critical section which protects gs_nWaitingForGui variable static wxCriticalSection *gs_critsectWaitingForGui = NULL; -// critical section which serializes WinThreadStart() and WaitForTerminate() -// (this is a potential bottleneck, we use a single crit sect for all threads -// in the system, but normally time spent inside it should be quite short) -static wxCriticalSection *gs_critsectThreadDelete = NULL; - // number of threads waiting for GUI in wxMutexGuiEnter() static size_t gs_nWaitingForGui = 0; @@ -1259,8 +1254,6 @@ bool wxThreadModule::OnInit() gs_critsectGui = new wxCriticalSection(); gs_critsectGui->Enter(); - gs_critsectThreadDelete = new wxCriticalSection; - wxThread::ms_idMainThread = wxThread::GetCurrentId(); return true; @@ -1277,8 +1270,6 @@ void wxThreadModule::OnExit() // library is initialized again later gs_tlsThisThread = TLS_OUT_OF_INDEXES; - wxDELETE(gs_critsectThreadDelete); - if ( gs_critsectGui ) { gs_critsectGui->Leave(); From e3691dbc13b94e0a304ff3bf6e69f070d0e3fcd1 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Fri, 16 Apr 2021 22:30:09 +0100 Subject: [PATCH 4/4] Explain reason for invalidating gs_tlsThisThread better Just improve a comment, no real changes. --- src/msw/thread.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/msw/thread.cpp b/src/msw/thread.cpp index f947c2a156..2b22bd61c2 100644 --- a/src/msw/thread.cpp +++ b/src/msw/thread.cpp @@ -1266,8 +1266,9 @@ void wxThreadModule::OnExit() wxLogLastError(wxT("TlsFree failed.")); } - // invalidate slot index to prevent wxThread from trying to reuse it if the - // library is initialized again later + // invalidate slot index to make the errors more obvious if we try to use + // it from now on, e.g. if any wxThreads are still running (which shouldn't + // be the case, of course, but might still happen) gs_tlsThisThread = TLS_OUT_OF_INDEXES; if ( gs_critsectGui )