Re: [boost] [thread] patch for tss.cpp (win32 + static linkingsupport)

From: Johan Nilsson (r.johan.nilsson_at_[hidden])
Date: 2007-05-03 05:09:36

Anthony Williams wrote:
> "Johan Nilsson" <r.johan.nilsson_at_[hidden]> writes:
>
>> I believe I've found a bug in tss.cpp, for details please ref
>> http://article.gmane.org/gmane.comp.lib.boost.user/26591 .
>>
>> The immediate fix is straightforward, it just involves resetting the
>> native tss value after deleting the pointed-to data at thread exit.
>> Patch is attached to this message. The changes only affects the
>> Win32 platform and I've run the supplied tests under
>> libs/thread/test to verify that it doesn't break anything obvious.
>>
>> As the bug involves dereferencing a dangling pointer and isn't
>> detected most of the time, I'd really appreciate if the patch could
>> be accepted as soon as possible. I'm awaiting Roland Schwartz to
>> "bless" the patch. If he agrees, would it be possible to accept this
>> for 1.34?
>
> Your patch calls TlsSetValue after calling cleanup_slots.
> cleanup_slots may call TlsFree, in which case this is not a safe
> thing to do. A correct fix would require further investigation.

[Someone's listening, great]

Yes, you are correct.

TlsFree will be called by tss_data_dec_use (which is in turn is called
by cleanup_slots) when the global reference count reaches 0.

- One solution would be to statically initialize tss_data_native_key to
TLS_OUT_OF_INDEXES, and reset to that value from tss_data_dec_use in the
above case. The code in tss_thread_exit could then check for a valid tls
index before calling TlsSetValue.

There shouldn't be a race condition checking/manipulating the native tls
index as it should exist only one thread using tss data at this point ... 
or? Am I missing something again?

- A second alternative would be to remove the TlsFree call altogether, as
the application is shutting down anyway. A bit dirty perhaps.

Do you have any suggestions for a better fix? If it would be possible to
have tss_thread_exit called at the correct time for the main thread, the
problems would probably be nonexistent.

Still, explicitly (de-)initializing the globals doesn't sound like a bad 
idea.

/ Johan