[thread] new & improved tss_hooks.cpp
I've attached a proposed simplification of libs/thread/src/tss_hooks.cpp that eliminates the one-time allocation that's been bugging people because their tools report it as a leak. If there are no objections, I'll commit this change to the CVS HEAD.
"Peter Dimov" <pdimov@mmltd.net> writes:
I've attached a proposed simplification of libs/thread/src/tss_hooks.cpp that eliminates the one-time allocation that's been bugging people because their tools report it as a leak.
If there are no objections, I'll commit this change to the CVS HEAD.
Can we have a call to TlsFree in on_process_exit, so we don't leak the TLS slot either? Other than that, it looks good; it's certainly cleaner than the old code. Anthony -- Anthony Williams Software Developer Just Software Solutions Ltd http://www.justsoftwaresolutions.co.uk
Anthony Williams wrote:
"Peter Dimov" <pdimov@mmltd.net> writes:
I've attached a proposed simplification of libs/thread/src/tss_hooks.cpp that eliminates the one-time allocation that's been bugging people because their tools report it as a leak.
If there are no objections, I'll commit this change to the CVS HEAD.
Can we have a call to TlsFree in on_process_exit, so we don't leak the TLS slot either?
It's better not to. We don't leak a TLS slot because TLS slots are per process. When the process is terminated by the OS, all of its TLS slots are "reclaimed" (more precisely, they simply cease to exist.) So we gain nothing because the lack of TlsFree is not observable. On the other hand, if on_process_exit is called when some threads are still running, TlsFree'ing the slot will make it impossible for these threads to clean up after themselves since they won't be able to access their cleanup handler lists. This can be observable if their cleanup handlers have visible side effects.
"Peter Dimov" <pdimov@mmltd.net> writes:
Anthony Williams wrote:
Can we have a call to TlsFree in on_process_exit, so we don't leak the TLS slot either?
It's better not to. We don't leak a TLS slot because TLS slots are per process. When the process is terminated by the OS, all of its TLS slots are "reclaimed" (more precisely, they simply cease to exist.)
So we gain nothing because the lack of TlsFree is not observable.
The same could be said of just about everything that the OS cleans up --- handles, memory, etc. In some circumstances, it *is* best not to do the clean up, to avoid potential race conditions with the clean up code and uses; I just want to ensure this has been thought about, as it's better to explicitly free stuff if you can.
On the other hand, if on_process_exit is called when some threads are still running, TlsFree'ing the slot will make it impossible for these threads to clean up after themselves since they won't be able to access their cleanup handler lists. This can be observable if their cleanup handlers have visible side effects.
on_process_exit is called when the module is unloaded. If there are any threads running at that point which are referencing code in that module (e.g. because they use boost thread cleanup handlers), they have bigger problems than the fact that the TLS slot has been freed --- the whole module is being unloaded. The TLS slot in question is internal to tss_hooks.cpp, and is only used by at_thread_exit/on_thread_exit. If the module is being unloaded (which is why on_process_exit is being run), then on_thread_exit will no longer be accessible, so the clean up handlers won't be run anyway. Anthony -- Anthony Williams Software Developer Just Software Solutions Ltd http://www.justsoftwaresolutions.co.uk
Anthony Williams wrote:
"Peter Dimov" <pdimov@mmltd.net> writes:
Anthony Williams wrote:
Can we have a call to TlsFree in on_process_exit, so we don't leak the TLS slot either?
It's better not to. We don't leak a TLS slot because TLS slots are per process. When the process is terminated by the OS, all of its TLS slots are "reclaimed" (more precisely, they simply cease to exist.)
So we gain nothing because the lack of TlsFree is not observable.
The same could be said of just about everything that the OS cleans up --- handles, memory, etc.
Correct. The only reason to do redundant cleanup is because of inadequate leak detectors and user complaints.
On the other hand, if on_process_exit is called when some threads are still running, TlsFree'ing the slot will make it impossible for these threads to clean up after themselves since they won't be able to access their cleanup handler lists. This can be observable if their cleanup handlers have visible side effects.
on_process_exit is called when the module is unloaded. If there are any threads running at that point which are referencing code in that module (e.g. because they use boost thread cleanup handlers), they have bigger problems than the fact that the TLS slot has been freed --- the whole module is being unloaded.
If on_process_exit is guaranteed to be called after all on_thread_exits (if it's called at all), it will indeed be safe (but still redundant) to TlsFree the slot in it.
"Peter Dimov" <pdimov@mmltd.net> writes:
Anthony Williams wrote:
"Peter Dimov" <pdimov@mmltd.net> writes:
Anthony Williams wrote:
Can we have a call to TlsFree in on_process_exit, so we don't leak the TLS slot either?
If on_process_exit is guaranteed to be called after all on_thread_exits (if it's called at all), it will indeed be safe (but still redundant) to TlsFree the slot in it.
I believe this to be the case. Anthony -- Anthony Williams Software Developer Just Software Solutions Ltd http://www.justsoftwaresolutions.co.uk
Anthony Williams wrote:
"Peter Dimov" <pdimov@mmltd.net> writes:
Anthony Williams wrote:
"Peter Dimov" <pdimov@mmltd.net> writes:
Anthony Williams wrote:
Can we have a call to TlsFree in on_process_exit, so we don't leak the TLS slot either?
If on_process_exit is guaranteed to be called after all on_thread_exits (if it's called at all), it will indeed be safe (but still redundant) to TlsFree the slot in it.
I believe this to be the case.
So it seems. I was thinking that we could choose the other direction and simplify the tss_hooks interface by removing on_process_* and on_thread_enter (I have a call_once in it but it's actually redundant.) But I don't really have a strong opinion one way or the other. So let's do it this way: I'll commit my replacement as-is and you'll add the TlsFree in on_process_exit if you wish. :-)
"Peter Dimov" <pdimov@mmltd.net> writes:
So let's do it this way: I'll commit my replacement as-is and you'll add the TlsFree in on_process_exit if you wish. :-)
Fine by me. Anthony -- Anthony Williams Software Developer Just Software Solutions Ltd http://www.justsoftwaresolutions.co.uk
participants (2)
-
Anthony Williams -
Peter Dimov