Re: [PR18457] Don't require rtld lock to compute DTV addr for static TLS

["Carlos O'Donell" <carlos at redhat dot com>]

On 06/03/2015 11:24 AM, Siddhesh Poyarekar wrote:
> On Wed, Jun 03, 2015 at 03:44:58AM -0300, Alexandre Oliva wrote:
>> We used to store static TLS addrs in the DTV at module load time, but
>> this required one thread to modify another thread's DTV.  Now that we
>> defer the DTV update to the first use in the thread, we should do so
>> without taking the rtld lock if the module is already assigned to static
>> TLS.  Taking the lock introduces deadlocks where there weren't any
>> before.
>>
>> This patch fixes the deadlock caused by tls_get_addr's unnecessarily
>> taking the rtld lock to initialize the DTV entry for tls_dtor_list
>> within __call_dtors_list, which deadlocks with a dlclose of a module
>> whose finalizer joins with that thread.  The patch does not, however,
>> attempt to fix other potential sources of similar deadlocks, such as
>> the explicit rtld locks taken by call_dtors_list, when the dtor list
>> is not empty; lazy relocation of the reference to tls_dtor_list, when
>> TLS Descriptors are in use; when tls dtors call functions through the
>> PLT and lazy relocation needs to be performed, or any such called
>> functions interact with the dynamic loader in ways that require its
>> lock to be taken.
>>
>> Ok to install?
> 
> It's not good enough and is in fact probably just dancing around the
> problem.  The simple patch to the test case below will cause the test
> case to deadlock.  Andreas' reproducer can be fixed by simply setting
> the TLS variables in cxa_thread_atexit as initial exec; I've got a
> patch for it that I'll post shortly.  That would leave two other
> problems:

I agree with Siddhesh and Torvald, it's not a good enough solution.

I spoke at length with Siddhesh about this, and I think we need to
sit down and really fix this once and for all by reasoning through
the TLS code and providing the appropriate atomic accesses, avoiding
data races, and documenting synchronization.

Siddhesh has the ball on this. Thanks for taking a first look, but
we're going to need to spend more time fixing this than I think you've
got :-(

Cheers,
Carlos.