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

From: Johan Nilsson (r.johan.nilsson_at_[hidden])
Date: 2007-05-04 02:28:04

Anthony Williams wrote:
> "Johan Nilsson" <r.johan.nilsson_at_[hidden]> writes:

[snip]

>> - 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?
>
> There isn't a race condition per-se, but there is no logic to
> reinitialize tss_data_native_key once it has been freed, so that
> would have to be added.

I've already been bitten by that problem in the current implementation, so 
this wouldn't be a regression. See:

http://article.gmane.org/gmane.comp.lib.boost.devel/155056

>
>> - A second alternative would be to remove the TlsFree call
>> altogether, as the application is shutting down anyway. A bit dirty
>> perhaps.
>
> Possible, yes --- the code to call TlsFree is relatively new. Dirty,
> yes. I think the TlsFree call should be moved to on_process_exit, but
> that's not a trivial change. You'd still need to call TlsSetValue to
> clear the entry.

Sure.

>
>> 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.
>
> Are you using Boost.Thread as a DLL or a LIB file?

Static library (hint, see subject line :-).

> If you're using it
> as a lib file, then the code in tss_pe.cpp controls when the exit
> stuff is called.

Yes, I know. I'll probably do some experimenting with this when and if I get 
some extra time.

>
> It is possible to make on_thread_exit be called correctly for the
> main thread, by calling it on process exit, but the problem here is
> that at this point in the process shutdown, things like new and
> delete don't necessarily work correctly, and the runtime has already
> been shut down --- the TSS code would need to be changed to use
> HeapAlloc/HeapFree rather than new and delete, and then there's the
> problem of what the destructors for the TSS data do: they can't
> necessarily use any library functions either, and that's not
> enforceable or readily explainable.

Agreed. The RTL should always be in a valid state when the TSS data 
destructors are run, as long as non-pod types can be stored in TSS.

>
> Having global destructors and atexit functions use TSS is a bad idea;
> this is just a manifestation of that.

Being able to use TSS in this way is nice and transparent from a user's 
point of view. I assume that you mean implementation-wise?

>
> Sorry, but I don't think this can be fixed for 1.34.0.
>

No, I've given up this already as 1.34 is already in beta. Perhaps 1.34.1 or 
1.35. I can workaround the problem by patching the local boost installation. 
I'm surprised that no one else seem to have experienced the problem, though.

/ Johan