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

@@ -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<void*>::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() )
+        if ( !IsOk() )
-            ::TlsFree(m_slot);
+            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<void*>::iterator i = m_allValues.begin();
+              i != m_allValues.end();
+              ++i )
+        {
+            m_destructor(*i);
+        }
+
+        ::TlsFree(m_slot);
     }
 
 private:
+    wxTlsDestructorFunction m_destructor;
     DWORD m_slot;
 
+    wxVector<void*> m_allValues;
+    wxCriticalSection m_csAllValues;
+
     wxDECLARE_NO_COPY_CLASS(wxTlsKey);
 };
 

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<void*>::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() )
+        if ( !IsOk() )
-            ::DosFreeThreadLocalMemory(m_slot);
+            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<void*>::iterator i = m_allValues.begin();
+              i != m_allValues.end();
+              ++i )
+        {
+            m_destructor(*i);
+        }
+
+        ::DosFreeThreadLocalMemory(m_slot);
     }
 
 private:
+    wxTlsDestructorFunction m_destructor;
     ULONG* m_slot;
 
+    wxVector<void*> m_allValues;
+    wxCriticalSection m_csAllValues;
+
     wxDECLARE_NO_COPY_CLASS(wxTlsKey);
 };
 

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<T> 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<T> is destroyed.  The only exception to this is the
+    //       value for the main thread, which is always freed when
+    //       wxTlsValue<T> is destroyed.
     template <typename T>
     class wxTlsValue
     {
@@ -67,32 +81,17 @@
         typedef T ValueType;
 
         // ctor doesn't do anything, the object is created on first access
-        //
+        wxTlsValue() : m_key(free) {}
-        // 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
-        {
-        }
 
         // 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
+        // just to be safe we also reset the key anyhow)
-        // memory under Windows (see the FIXME above)
         ~wxTlsValue()
         {
-            void * const value = m_key.Get();
+            if ( m_key.Get() )
-            if ( value)
+                m_key.Set(NULL); // this deletes the value
-            {
-                free(value);
-                m_key.Set(NULL);
-            }
         }
 
         // access the object creating it on demand

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
+    // for it
-    // not supported in Win32 implementation)
+    wxTlsKey(wxTlsDestructorFunction destructor)
-    wxTlsKey(void (*destructor)(void *) = NULL)
     {
+        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);