[thread] Keeping leak detectors still...
Hello!, Its a well-known issue, that Boost-Thread users on Windows machines have observed that popular leak reporting tools loudly notify a missing delete for the dynamically allocated mutex in tss_hooks.cpp. A good overview for this discussion can be found in: http://groups.google.de/group/boost-list/browse_thread/thread/f5788746e458dd... While I don't want to heat up the discussion whether this is actually a "leak" or not, many code style guides of companies give very bad points on such behaviour, so I would like to ask whether it is possible to ensure a proper clean-up of this by code (before the system does), to keep those detector programs quiet... Simple question of a non-expert: Couldn't one simply change the current impl of on_process_exit in tss_hooks.cpp to extern "C" BOOST_THREAD_DECL void on_process_exit(void) { { // Wrap into scope boost::call_once(init_threadmon_mutex, once_init_threadmon_mutex); boost::mutex::scoped_lock lock(*threadmon_mutex); BOOST_ASSERT(attached_thread_count == 0); //Free the tls slot if one was allocated. if (tls_key != invalid_tls_key) { TlsFree(tls_key); tls_key = invalid_tls_key; } } delete threadmon_mutex; // Shouldn't that be the last point of access?? threadmon_mutex = 0; // For those who prefer a defined value... } OK, I see that the docs for this function says: "Must not be omitted; may be called multiple times." Question is: Does there exist at least the guarantee, that there is always the same number of on_process_enter and on_process_exit calls? If yes: Wouldn't a simply reference counting scheme using one of the atomic integer incrementing/decrementing functions (also provided by boost via interlocked.hpp) allow a thread-save deletion of that mutex object? Thanks for your patience, Daniel Krügler
Peter Dimov wrote:
Daniel Krügler wrote:
No ideas, proposals, comments so far?
Please take a look at the attached replacement. Does it work for you? //
Thanks Peter, for your attachment. A first quick test seems to show that the leak detector is quiet now. Interestingly the provided code is much simpler than the older one, but does not call ::TlsFree anymore (I think it cannot call that function to prevent the same problem, which the current mutext attempts to solve). I have not found yet any hints in the documentation of the TLS functions, whether there have to be a matching ::TlsFree for any ::TlsAlloc. Anyone else? Most probably the loss of available TLS indices would be worse than a missing clean-up of a mutex, isn't it? We will test the new code under more realistic conditions to check it, but that need some more time. Any results will be posted here, of course. I would appreciate results of others, too. Thank you very much, Daniel
Daniel Krügler wrote:
Thanks Peter, for your attachment. A first quick test seems to show that the leak detector is quiet now. Interestingly the provided code is much simpler than the older one, but does not call ::TlsFree anymore (I think it cannot call that function to prevent the same problem, which the current mutext attempts to solve). I have not found yet any hints in the documentation of the TLS functions, whether there have to be a matching ::TlsFree for any ::TlsAlloc. Anyone else? Most probably the loss of available TLS indices would be worse than a missing clean-up of a mutex, isn't it?
TLS indices are per-process, and since the TLS index used by the cleanup code cannot be cleaned before process exit, it doesn't matter whether there is an explicit TlsFree. When the process dies, so will all its TLS indices. I could have put a TlsFree in on_process_exit, but it's better not to. If on_process_exit is (for some reason) called before all on_thread_exits, the cleanup handlers won't have a chance to execute and will leak.
Peter Dimov wrote:
TLS indices are per-process, and since the TLS index used by the cleanup code cannot be cleaned before process exit, it doesn't matter whether there is an explicit TlsFree. When the process dies, so will all its TLS indices.
I could have put a TlsFree in on_process_exit, but it's better not to. If on_process_exit is (for some reason) called before all on_thread_exits, the cleanup handlers won't have a chance to execute and will leak.
Thank you very much for your careful explanation, Peter! Given the advantages of *more* simplicity and *less* warnings by often-used tools, wouldn't it be appropriate to use this code as actual implementation of Boost.Thread? Greetings from Bremen, Daniel
Daniel Krügler wrote:
Peter Dimov wrote:
TLS indices are per-process, and since the TLS index used by the cleanup code cannot be cleaned before process exit, it doesn't matter whether there is an explicit TlsFree. When the process dies, so will all its TLS indices.
I could have put a TlsFree in on_process_exit, but it's better not to. If on_process_exit is (for some reason) called before all on_thread_exits, the cleanup handlers won't have a chance to execute and will leak.
Thank you very much for your careful explanation, Peter!
Given the advantages of *more* simplicity and *less* warnings by often-used tools, wouldn't it be appropriate to use this code as actual implementation of Boost.Thread?
Certainly. I'll wait for your (or anyone else's) test results first, though.
participants (2)
-
Daniel Krügler -
Peter Dimov