From b06aeaea607ef6cdb44b5c49fe8c56ce8a4fefb3 Mon Sep 17 00:00:00 2001 From: Christopher Kohlhoff Date: Thu, 9 Oct 2008 06:33:34 +0000 Subject: [PATCH] Only use TerminateThread when explicitly requested by the user by calling asio::detail::thread::set_terminate_threads(true). This fixes a memory leak that may occur with internally created threads. [SVN r49201] --- include/boost/asio/detail/null_thread.hpp | 5 +- include/boost/asio/detail/posix_thread.hpp | 5 +- include/boost/asio/detail/win_thread.hpp | 117 +++++++++++++-------- include/boost/asio/detail/wince_thread.hpp | 5 +- 4 files changed, 78 insertions(+), 54 deletions(-) diff --git a/include/boost/asio/detail/null_thread.hpp b/include/boost/asio/detail/null_thread.hpp index 507904ba..5aed2112 100644 --- a/include/boost/asio/detail/null_thread.hpp +++ b/include/boost/asio/detail/null_thread.hpp @@ -39,12 +39,9 @@ class null_thread : private noncopyable { public: - // The purpose of the thread. - enum purpose { internal, external }; - // Constructor. template - null_thread(Function f, purpose = internal) + null_thread(Function f) { boost::system::system_error e( boost::asio::error::operation_not_supported, "thread"); diff --git a/include/boost/asio/detail/posix_thread.hpp b/include/boost/asio/detail/posix_thread.hpp index 6dbc3b8f..1e386184 100644 --- a/include/boost/asio/detail/posix_thread.hpp +++ b/include/boost/asio/detail/posix_thread.hpp @@ -43,12 +43,9 @@ class posix_thread : private noncopyable { public: - // The purpose of the thread. - enum purpose { internal, external }; - // Constructor. template - posix_thread(Function f, purpose = internal) + posix_thread(Function f) : joined_(false) { std::auto_ptr arg(new func(f)); diff --git a/include/boost/asio/detail/win_thread.hpp b/include/boost/asio/detail/win_thread.hpp index fe95b3fb..c8058d80 100644 --- a/include/boost/asio/detail/win_thread.hpp +++ b/include/boost/asio/detail/win_thread.hpp @@ -40,50 +40,67 @@ namespace detail { unsigned int __stdcall win_thread_function(void* arg); -class win_thread - : private noncopyable +#if (WINVER < 0x0500) +void __stdcall apc_function(ULONG data); +#else +void __stdcall apc_function(ULONG_PTR data); +#endif + +template +class win_thread_base { public: - // The purpose of the thread. - enum purpose { internal, external }; + static bool terminate_threads() + { + return ::InterlockedExchangeAdd(&terminate_threads_, 0) != 0; + } + static void set_terminate_threads(bool b) + { + ::InterlockedExchange(&terminate_threads_, b ? 1 : 0); + } + +private: + static long terminate_threads_; +}; + +template +long win_thread_base::terminate_threads_ = 0; + +class win_thread + : private noncopyable, + public win_thread_base +{ +public: // Constructor. template - win_thread(Function f, purpose p = internal) + win_thread(Function f) : exit_event_(0) { std::auto_ptr arg(new func(f)); ::HANDLE entry_event = 0; - if (p == internal) + arg->entry_event_ = entry_event = ::CreateEvent(0, true, false, 0); + if (!entry_event) { - arg->entry_event_ = entry_event = ::CreateEvent(0, true, false, 0); - if (!entry_event) - { - DWORD last_error = ::GetLastError(); - boost::system::system_error e( - boost::system::error_code(last_error, - boost::asio::error::get_system_category()), - "thread.entry_event"); - boost::throw_exception(e); - } - - arg->exit_event_ = exit_event_ = ::CreateEvent(0, true, false, 0); - if (!exit_event_) - { - DWORD last_error = ::GetLastError(); - ::CloseHandle(entry_event); - boost::system::system_error e( - boost::system::error_code(last_error, - boost::asio::error::get_system_category()), - "thread.exit_event"); - boost::throw_exception(e); - } + DWORD last_error = ::GetLastError(); + boost::system::system_error e( + boost::system::error_code(last_error, + boost::asio::error::get_system_category()), + "thread.entry_event"); + boost::throw_exception(e); } - else + + arg->exit_event_ = exit_event_ = ::CreateEvent(0, true, false, 0); + if (!exit_event_) { - arg->entry_event_ = 0; - arg->exit_event_ = 0; + DWORD last_error = ::GetLastError(); + ::CloseHandle(entry_event); + boost::system::system_error e( + boost::system::error_code(last_error, + boost::asio::error::get_system_category()), + "thread.exit_event"); + boost::throw_exception(e); } unsigned int thread_id = 0; @@ -123,14 +140,15 @@ public: // Wait for the thread to exit. void join() { - if (exit_event_) + ::WaitForSingleObject(exit_event_, INFINITE); + ::CloseHandle(exit_event_); + if (terminate_threads()) { - ::WaitForSingleObject(exit_event_, INFINITE); - ::CloseHandle(exit_event_); ::TerminateThread(thread_, 0); } else { + ::QueueUserAPC(apc_function, thread_, 0); ::WaitForSingleObject(thread_, INFINITE); } } @@ -138,6 +156,12 @@ public: private: friend unsigned int __stdcall win_thread_function(void* arg); +#if (WINVER < 0x0500) + friend void __stdcall apc_function(ULONG); +#else + friend void __stdcall apc_function(ULONG_PTR); +#endif + class func_base { public: @@ -175,21 +199,30 @@ inline unsigned int __stdcall win_thread_function(void* arg) std::auto_ptr func( static_cast(arg)); - if (func->entry_event_) - ::SetEvent(func->entry_event_); + ::SetEvent(func->entry_event_); func->run(); - if (HANDLE exit_event = func->exit_event_) - { - func.reset(); - ::SetEvent(exit_event); - ::Sleep(INFINITE); - } + // Signal that the thread has finished its work, but rather than returning go + // to sleep to put the thread into a well known state. If the thread is being + // joined during global object destruction then it may be killed using + // TerminateThread (to avoid a deadlock in DllMain). Otherwise, the SleepEx + // call will be interrupted using QueueUserAPC and the thread will shut down + // cleanly. + HANDLE exit_event = func->exit_event_; + func.reset(); + ::SetEvent(exit_event); + ::SleepEx(INFINITE, TRUE); return 0; } +#if (WINVER < 0x0500) +inline void __stdcall apc_function(ULONG) {} +#else +inline void __stdcall apc_function(ULONG_PTR) {} +#endif + } // namespace detail } // namespace asio } // namespace boost diff --git a/include/boost/asio/detail/wince_thread.hpp b/include/boost/asio/detail/wince_thread.hpp index 6b624872..7b24ec25 100644 --- a/include/boost/asio/detail/wince_thread.hpp +++ b/include/boost/asio/detail/wince_thread.hpp @@ -43,12 +43,9 @@ class wince_thread : private noncopyable { public: - // The purpose of the thread. - enum purpose { internal, external }; - // Constructor. template - wince_thread(Function f, purpose = internal) + wince_thread(Function f) { std::auto_ptr arg(new func(f)); DWORD thread_id = 0;