Re: [RFC PATCH glibc] pthread_setspecific: Provide signal-safety across keys

[Mathieu Desnoyers <mathieu dot desnoyers at efficios dot com>]

----- On Oct 17, 2017, at 6:01 PM, Carlos O'Donell carlos@redhat.com wrote:

> On 10/17/2017 02:54 PM, Florian Weimer wrote:
>> * Mathieu Desnoyers:
>> 
>>>  	  if (value == NULL)
>>>  	    /* We don't have to do anything.  The value would in any case
>>>  	       be NULL.  We can save the memory allocation.  */
>>>  	    return 0;
>>>  
>>> +	  /* Ensure that pthread_setspecific and pthread_getspecific are
>>> +             signal-safe when used on different keys.  */
>>> +          sigfillset (&ss);
>>> +          pthread_sigmask (SIG_BLOCK, &ss, &old_ss);
>>> +          /* Read level2 again with signals disabled.  */
>>> +          level2 = THREAD_GETMEM_NC (self, specific, idx1st);
>>> +          if (level2 != NULL)
>>> +            goto skip_resize;
>>> +
>>>  	  level2
>>>  	    = (struct pthread_key_data *) calloc (PTHREAD_KEY_2NDLEVEL_SIZE,
>>>  						  sizeof (*level2));
>> 
>> I don't see how this adds signal safety.  If the function is called
>> from a signal handler, interrupting the malloc code, then this will
>> still result in undefined behavior (heap corruption or deadlocks).
>> 
>> Does lttng-ust call these functions even if the application does not?
>> Then you really need to switch to real thread-local variables with the
>> intial-exec model, possibly with indirection through a thread-local
>> pointer variable and on-demand allocation using mmap in case you do
>> not want to have these allocations in every thread.
> 
> Agreed. The calloc proceeds in AS context and that leads to undefined
> behaviour.

I indeed plan to spin an updated patch using mmap instead.

> 
> Please also read Florian's recommendation very carefully, you must us
> IE model TLS vars to ensure pre-allocation and avoid potential OOM
> during first variable touch, and to avoid first variable touch in a
> signal handler resulting in the same problem as above (though we are
> looking to fix this).

Hopefully touching the TLS variable from a library constructor sides-steps
this issue. This has worked well in practice so far.

Thanks,

Mathieu

> 
> --
> Cheers,
> Carlos.

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com