cheng/wxWidgets
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix a memory leak when using non-wx threads

Browse Source 
The wxThreadSpecificInfo object created by wxThreadSpecificInfo::Get()
was only released by the wxThreadInternal's cleanup, which meant that it
was leaked if wxThreadSpecificInfo::Get() was called by a thread not
created by wxThread, e.g. when using std::async or std::thread.

As we still can't rely on C++11 thread local variables in this branch
(see d666d1e222 (Fix a memory leak when using non-wx threads,
2023-05-10) in master), we work around the problem by replacing the
ThreadCleanUp call with platform specific implementations for
wxThreadSpecificInfo::Get() that free the allocated wxThreadSpecificInfo
automatically on thread exit.

Note that the Fls functions used for Windows implementation are not
supported on Windows XP, and thus the problem still persists there.

Closes ##23535.
This commit is contained in:
Antti Nietosvaara 2023-05-10 10:49:51 +03:00 committed by Vadim Zeitlin
parent 1c64bd506c
commit e19984e2dd
5 changed files with 160 additions and 90 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
docs/changes.txt
View File

@ -243,6 +243,7 @@ All:
- Don't use invalid iterator in wxString in UTF-8 build (Ian Day, #23305).
- Fix wrong description for some languages (Ulrich Telle, #23419).
- Fix infinite loop in wxFTP::GetFilesList() (#23519).
- Fix memory leak when using wx from non-wx threads (Antti Nietosvaara, #23535).
- Use grep instead of fgrep and egrep in makefiles/scripts (#23537).
All (GUI):

7
include/wx/private/threadinfo.h
View File

@ -52,13 +52,6 @@ public:
wxLocaleUntranslatedStrings untranslatedStrings;
#endif
#if wxUSE_THREADS
// Cleans up storage for the current thread. Should be called when a thread
// is being destroyed. If it's not called, the only bad thing that happens
// is that the memory is deallocated later, on process termination.
static void ThreadCleanUp();
#endif
private:
wxThreadSpecificInfo() : logger(NULL), loggingDisabled(false) {}
};

80
src/common/threadinfo.cpp
View File

@ -10,86 +10,14 @@
// For compilers that support precompilation, includes "wx.h".
#include "wx/wxprec.h"
#include "wx/private/threadinfo.h"
#if wxUSE_THREADS
#include "wx/tls.h"
#include "wx/thread.h"
#include "wx/sharedptr.h"
#include "wx/vector.h"
// The threaded version for wxThreadSpecificInfo::Get() is defined in platform specific
// thread files: src/msw/thread.cpp or src/unix/threadpsx.cpp
namespace
{
#else
// All thread info objects are stored in a global list so that they are
// freed when global objects are destroyed and no memory leaks are reported.
// Notice that we must be using accessor functions instead of simple global
// variables here as this code could be executed during global initialization
// time, i.e. before any globals in this module were initialzied.
inline wxCriticalSection& GetAllThreadInfosCS()
{
static wxCriticalSection s_csAllThreadInfos;
return s_csAllThreadInfos;
}
typedef wxVector< wxSharedPtr<wxThreadSpecificInfo> > wxAllThreadInfos;
inline wxAllThreadInfos& GetAllThreadInfos()
{
static wxAllThreadInfos s_allThreadInfos;
return s_allThreadInfos;
}
// Pointer to the current thread's instance
inline wxTLS_TYPE_REF(wxThreadSpecificInfo*) GetThisThreadInfo()
{
static wxTLS_TYPE(wxThreadSpecificInfo*) s_thisThreadInfo;
return s_thisThreadInfo;
}
#define wxTHIS_THREAD_INFO wxTLS_VALUE(GetThisThreadInfo())
} // anonymous namespace
wxThreadSpecificInfo& wxThreadSpecificInfo::Get()
{
if ( !wxTHIS_THREAD_INFO )
{
wxTHIS_THREAD_INFO = new wxThreadSpecificInfo;
wxCriticalSectionLocker lock(GetAllThreadInfosCS());
GetAllThreadInfos().push_back(
wxSharedPtr<wxThreadSpecificInfo>(wxTHIS_THREAD_INFO));
}
return *wxTHIS_THREAD_INFO;
}
void wxThreadSpecificInfo::ThreadCleanUp()
{
if ( !wxTHIS_THREAD_INFO )
return; // nothing to do, not used by this thread at all
// find this thread's instance in GetAllThreadInfos() and destroy it
wxCriticalSectionLocker lock(GetAllThreadInfosCS());
for ( wxAllThreadInfos::iterator i = GetAllThreadInfos().begin();
i != GetAllThreadInfos().end();
++i )
{
if ( i->get() == wxTHIS_THREAD_INFO )
{
GetAllThreadInfos().erase(i);
wxTHIS_THREAD_INFO = NULL;
break;
}
}
}
#else // !wxUSE_THREADS
#include "wx/private/threadinfo.h"
wxThreadSpecificInfo& wxThreadSpecificInfo::Get()
{

109
src/msw/thread.cpp
View File

@ -122,6 +122,111 @@ static bool gs_waitingForThread = false;
// Windows implementation of thread and related classes
// ============================================================================
// Create a wrapper class for storing wxThreadSpecificInfo
// using FLS or TLS api. Preferably we want to use FLS
// since it supports freeing the created objects automatically
// on thread exit. However, this is only supported from
// Windows Vista and newer, so TLS is used as a fallback.
// Note that with TLS we need to clean up the objects
// manually in wxThreadInternal::WinThreadStart, and
// if the running thread is not a wxThread, the objects
// will not be freed until program exit. As mentioned,
// this will only affect Windows XP.
class wxThreadSpecificInfoTLS
{
private:
typedef DWORD(WINAPI *AllocCallback_t)(void (WINAPI*)(void*));
AllocCallback_t AllocCallback;
typedef DWORD(WINAPI *Alloc_t)();
Alloc_t Alloc;
typedef BOOL(WINAPI *Free_t)(DWORD);
Free_t Free;
typedef void* (WINAPI *GetValue_t)(DWORD);
GetValue_t GetValue;
typedef BOOL(WINAPI *SetValue_t)(DWORD, void*);
SetValue_t SetValue;
DWORD m_idx;
static void WINAPI DeleteThreadSpecificInfo(void* ptr)
{
delete static_cast<wxThreadSpecificInfo*>(ptr);
}
static const wxThreadSpecificInfoTLS& Instance()
{
static wxThreadSpecificInfoTLS s_instance;
return s_instance;
}
wxThreadSpecificInfoTLS():
AllocCallback(reinterpret_cast<AllocCallback_t>(GetProcAddress(GetModuleHandle(_T("kernel32.dll")), "FlsAlloc"))),
Alloc(NULL),
Free(reinterpret_cast<Free_t>(GetProcAddress(GetModuleHandle(_T("kernel32.dll")), "FlsFree"))),
GetValue(reinterpret_cast<GetValue_t>(GetProcAddress(GetModuleHandle(_T("kernel32.dll")), "FlsGetValue"))),
SetValue(reinterpret_cast<SetValue_t>(GetProcAddress(GetModuleHandle(_T("kernel32.dll")), "FlsSetValue")))
{
if (AllocCallback && Free && GetValue && SetValue)
{
// FLS API was available, so use it to free objects automatically
m_idx = AllocCallback(&DeleteThreadSpecificInfo);
}
else
{
// FLS API was not available (Windows XP?), so use TLS as fallback
AllocCallback = NULL;
Alloc = reinterpret_cast<Alloc_t>(TlsAlloc);
Free = reinterpret_cast<Free_t>(TlsFree);
GetValue = reinterpret_cast<GetValue_t>(TlsGetValue);
SetValue = reinterpret_cast<SetValue_t>(TlsSetValue);
m_idx = Alloc();
}
}
public:
static wxThreadSpecificInfo* Get()
{
return static_cast<wxThreadSpecificInfo*>(Instance().GetValue(Instance().m_idx));
}
// wxThreadSpecificInfo will try to delete info when thread ends
static bool Set(wxThreadSpecificInfo* info)
{
return Instance().SetValue(Instance().m_idx, info);
}
static void CleanUp()
{
if (!Instance().AllocCallback)
{
// FLS API was not available, which means that objects will not be freed automatically.
delete Get();
Set(NULL);
}
}
};
wxThreadSpecificInfo& wxThreadSpecificInfo::Get()
{
wxThreadSpecificInfo* info = wxThreadSpecificInfoTLS::Get();
if (!info)
{
info = new wxThreadSpecificInfo;
if (!wxThreadSpecificInfoTLS::Set(info))
{
// This will crash, but we'd leak memory otherwise which
// could be even worse and less immediately discoverable.
delete info;
info = NULL;
}
}
return *info;
}
// ----------------------------------------------------------------------------
// wxCriticalSection
// ----------------------------------------------------------------------------
@ -587,7 +692,9 @@ THREAD_RETVAL THREAD_CALLCONV wxThreadInternal::WinThreadStart(void *param)
// Do this as the very last thing to ensure that thread-specific info is
// not recreated any longer.
wxThreadSpecificInfo::ThreadCleanUp();
// Note thatg this is necessary only if the wxThreadSpecificInfoTLS cannot
// perform the cleanup automatically. That is, with Windows XP or older.
wxThreadSpecificInfoTLS::CleanUp();
return rc;
}

53
src/unix/threadpsx.cpp
View File

@ -27,7 +27,6 @@
#include "wx/thread.h"
#include "wx/except.h"
#include "wx/scopeguard.h"
#include "wx/private/threadinfo.h"
@ -858,11 +857,6 @@ void *wxPthreadStart(void *ptr)
void *wxThreadInternal::PthreadStart(wxThread *thread)
{
// Ensure that we clean up thread-specific data before exiting the thread
// and do it as late as possible as wxLog calls can recreate it and may
// happen until the very end.
wxON_BLOCK_EXIT0(wxThreadSpecificInfo::ThreadCleanUp);
wxThreadInternal *pthread = thread->m_internal;
wxLogTrace(TRACE_THREADS, wxT("Thread %p started."), THR_ID(pthread));
@ -2031,6 +2025,53 @@ void wxMutexGuiLeaveImpl()
#endif
wxThreadSpecificInfo& wxThreadSpecificInfo::Get()
{
// Since wxTlsValue only allows POD types, we need to use TLS API directly instead
// to free the allocated object automatically on thread exit.
class wxThreadSpecificInfoTLS
{
private:
static void DeleteThreadSpecificInfo(void *ptr)
{
delete static_cast<wxThreadSpecificInfo*>(ptr);
}
pthread_key_t m_key;
public:
wxThreadSpecificInfoTLS()
{
pthread_key_create(&m_key, &DeleteThreadSpecificInfo);
}
~wxThreadSpecificInfoTLS()
{
pthread_key_delete(m_key);
}
wxThreadSpecificInfo& Get() const
{
wxThreadSpecificInfo* info =
static_cast<wxThreadSpecificInfo*>(pthread_getspecific(m_key));
if (!info)
{
info = new wxThreadSpecificInfo;
if (pthread_setspecific(m_key, info) != 0)
{
// This will crash, but we'd leak memory otherwise which
// could be even worse and less immediately discoverable.
delete info;
info = NULL;
}
}
return *info;
}
};
static const wxThreadSpecificInfoTLS s_info;
return s_info.Get();
}
// ----------------------------------------------------------------------------
// include common implementation code
// ----------------------------------------------------------------------------