From 49e714e127fb21392772f0f61bed7efe5e73cf2a Mon Sep 17 00:00:00 2001 From: =?utf8?q?V=C3=A1clav=20Slav=C3=ADk?= Date: Mon, 8 Mar 2010 12:21:58 +0000 Subject: [PATCH] Fix wxTlsValue<> memory leaks. Win32 API doesn't provide pthreads-like destructors, so we need to emulate them to free per-thread allocated memory. git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@63653 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775 --- include/wx/msw/tls.h | 67 ++++++++++++++++++++++++++++++++++++++++--- include/wx/os2/tls.h | 63 ++++++++++++++++++++++++++++++++++++++-- include/wx/tls.h | 39 ++++++++++++------------- include/wx/unix/tls.h | 11 +++++-- 4 files changed, 150 insertions(+), 30 deletions(-) diff --git a/include/wx/msw/tls.h b/include/wx/msw/tls.h index c27271ca60..2ef70a2183 100644 --- a/include/wx/msw/tls.h +++ b/include/wx/msw/tls.h @@ -12,6 +12,8 @@ #define _WX_MSW_TLS_H_ #include "wx/msw/wrapwin.h" +#include "wx/thread.h" +#include "wx/vector.h" // ---------------------------------------------------------------------------- // wxTlsKey is a helper class encapsulating a TLS slot @@ -21,8 +23,9 @@ class wxTlsKey { public: // ctor allocates a new key - wxTlsKey() + wxTlsKey(wxTlsDestructorFunction destructor) { + m_destructor = destructor; m_slot = ::TlsAlloc(); } @@ -38,19 +41,75 @@ public: // change the key value, return true if ok bool Set(void *value) { - return ::TlsSetValue(m_slot, value) != 0; + void *old = Get(); + + if ( ::TlsSetValue(m_slot, value) == 0 ) + return false; + + if ( old ) + m_destructor(old); + + // update m_allValues list of all values - remove old, add new + wxCriticalSectionLocker lock(m_csAllValues); + if ( old ) + { + for ( wxVector::iterator i = m_allValues.begin(); + i != m_allValues.end(); + ++i ) + { + if ( *i == old ) + { + if ( value ) + *i = value; + else + m_allValues.erase(i); + return true; + } + } + wxFAIL_MSG( "previous wxTlsKey value not recorded in m_allValues" ); + } + + if ( value ) + m_allValues.push_back(value); + + return true; } // free the key ~wxTlsKey() { - if ( IsOk() ) - ::TlsFree(m_slot); + if ( !IsOk() ) + return; + + // Win32 API doesn't have the equivalent of pthread's destructor, so we + // have to keep track of all allocated values and destroy them manually; + // ideally we'd do that at thread exit time, but since we could only + // do that with wxThread and not otherwise created threads, we do it + // here. + // + // TODO: We should still call destructors for wxTlsKey used in the + // thread from wxThread's thread shutdown code, *in addition* + // to doing it in ~wxTlsKey. + // + // NB: No need to lock m_csAllValues, by the time this code is called, + // no other thread can be using this key. + for ( wxVector::iterator i = m_allValues.begin(); + i != m_allValues.end(); + ++i ) + { + m_destructor(*i); + } + + ::TlsFree(m_slot); } private: + wxTlsDestructorFunction m_destructor; DWORD m_slot; + wxVector m_allValues; + wxCriticalSection m_csAllValues; + wxDECLARE_NO_COPY_CLASS(wxTlsKey); }; diff --git a/include/wx/os2/tls.h b/include/wx/os2/tls.h index 4c22721fb3..cfe999fe83 100644 --- a/include/wx/os2/tls.h +++ b/include/wx/os2/tls.h @@ -12,6 +12,8 @@ #define _WX_OS2_TLS_H_ #include "wx/os2/private.h" +#include "wx/thread.h" +#include "wx/vector.h" // ---------------------------------------------------------------------------- // wxTlsKey is a helper class encapsulating a TLS slot @@ -21,8 +23,9 @@ class wxTlsKey { public: // ctor allocates a new key - wxTlsKey() + wxTlsKey(wxTlsDestructorFunction destructor) { + m_destructor = destructor; APIRET rc = ::DosAllocThreadLocalMemory(1, &m_slot); if (rc != NO_ERROR) m_slot = NULL; @@ -40,20 +43,74 @@ public: // change the key value, return true if ok bool Set(void *value) { + void *old = Get(); + m_slot = (ULONG*)value; + + if ( old ) + m_destructor(old); + + // update m_allValues list of all values - remove old, add new + wxCriticalSectionLocker lock(m_csAllValues); + if ( old ) + { + for ( wxVector::iterator i = m_allValues.begin(); + i != m_allValues.end(); + ++i ) + { + if ( *i == old ) + { + if ( value ) + *i = value; + else + m_allValues.erase(i); + return true; + } + } + wxFAIL_MSG( "previous wxTlsKey value not recorded in m_allValues" ); + } + + if ( value ) + m_allValues.push_back(value); + return true; } // free the key ~wxTlsKey() { - if ( IsOk() ) - ::DosFreeThreadLocalMemory(m_slot); + if ( !IsOk() ) + return; + + // Win32 and OS/2 API doesn't have the equivalent of pthread's + // destructor, so we have to keep track of all allocated values and + // destroy them manually; ideally we'd do that at thread exit time, but + // since we could only do that with wxThread and not otherwise created + // threads, we do it here. + // + // TODO: We should still call destructors for wxTlsKey used in the + // thread from wxThread's thread shutdown code, *in addition* + // to doing it in ~wxTlsKey. + // + // NB: No need to lock m_csAllValues, by the time this code is called, + // no other thread can be using this key. + for ( wxVector::iterator i = m_allValues.begin(); + i != m_allValues.end(); + ++i ) + { + m_destructor(*i); + } + + ::DosFreeThreadLocalMemory(m_slot); } private: + wxTlsDestructorFunction m_destructor; ULONG* m_slot; + wxVector m_allValues; + wxCriticalSection m_csAllValues; + wxDECLARE_NO_COPY_CLASS(wxTlsKey); }; diff --git a/include/wx/tls.h b/include/wx/tls.h index 7846af1e3b..bd4e9bad7a 100644 --- a/include/wx/tls.h +++ b/include/wx/tls.h @@ -44,7 +44,13 @@ #define wxTLS_PTR(var) (&(var)) #define wxTLS_VALUE(var) (var) #else // !wxHAS_COMPILER_TLS - #ifdef __WXMSW__ + + extern "C" + { + typedef void (*wxTlsDestructorFunction)(void*); + } + + #if defined(__WXMSW__) #include "wx/msw/tls.h" #elif defined(__OS2__) #include "wx/os2/tls.h" @@ -60,6 +66,14 @@ // wxTlsValue represents a thread-specific value of type T but, unlike // with native compiler thread-specific variables, it behaves like a // (never NULL) pointer to T and so needs to be dereferenced before use + // + // Note: T must be a POD! + // + // Note: On Unix, thread-specific T value is freed when the thread exits. + // On Windows, thread-specific values are freed later, when given + // wxTlsValue is destroyed. The only exception to this is the + // value for the main thread, which is always freed when + // wxTlsValue is destroyed. template class wxTlsValue { @@ -67,32 +81,17 @@ typedef T ValueType; // ctor doesn't do anything, the object is created on first access - // - // FIXME: the thread-specific values are currently not freed under - // Windows, resulting in memory leaks, this must be implemented - // there somehow (probably by keeping a list of all TLS objects - // and cleaning them up in wxThread cleanup) - wxTlsValue() -#if !defined(__OS2__) && defined(__UNIX__) - : m_key(free) -#endif - { - } + wxTlsValue() : m_key(free) {} // dtor is only called in the main thread context and so is not enough // to free memory allocated by us for the other threads, we use // destructor function when using Pthreads for this (which is not // called for the main thread as it doesn't call pthread_exit() but - // just to be safe we also reset the key anyhow) and simply leak the - // memory under Windows (see the FIXME above) + // just to be safe we also reset the key anyhow) ~wxTlsValue() { - void * const value = m_key.Get(); - if ( value) - { - free(value); - m_key.Set(NULL); - } + if ( m_key.Get() ) + m_key.Set(NULL); // this deletes the value } // access the object creating it on demand diff --git a/include/wx/unix/tls.h b/include/wx/unix/tls.h index 126ccd2b17..05f92b7fb9 100644 --- a/include/wx/unix/tls.h +++ b/include/wx/unix/tls.h @@ -21,10 +21,10 @@ class wxTlsKey { public: // ctor allocates a new key and possibly registering a destructor function - // for it (notice that using destructor function is Pthreads-specific and - // not supported in Win32 implementation) - wxTlsKey(void (*destructor)(void *) = NULL) + // for it + wxTlsKey(wxTlsDestructorFunction destructor) { + m_destructor = destructor; if ( pthread_key_create(&m_key, destructor) != 0 ) m_key = 0; } @@ -41,6 +41,10 @@ public: // change the key value, return true if ok bool Set(void *value) { + void *old = Get(); + if ( old ) + m_destructor(old); + return pthread_setspecific(m_key, value) == 0; } @@ -52,6 +56,7 @@ public: } private: + wxTlsDestructorFunction m_destructor; pthread_key_t m_key; wxDECLARE_NO_COPY_CLASS(wxTlsKey); -- 2.45.2