Re: [PATCH v2] Fix double-checked locking in __gconv_get_path and __gconv_read_conf [BZ #22062]

[Carlos O'Donell <carlos@redhat.com>]

On 10/05/2017 03:20 PM, Florian Weimer wrote:
> On 10/05/2017 10:22 PM, Carlos O'Donell wrote:
>> Is there any reason we need this atomic load?
> 
> Based on the context, it's not clear to which load you are referring.
> 
> Florian

My apologies, clipped, to show context:

> +  if (gconv_path_elem_local == NULL)
> +    {
> +      __gconv_get_path ();

Multiple reads by threads are safe. In this case we can have multiple threads
here, but all writes to __gconv_path_elem are complete. All we are doing is
having multiple readers, which is not a data race.

Is there any reason we need this atomic load?

> +      gconv_path_elem_local = atomic_load_relaxed (&__gconv_path_elem);
                                 ^^^^^^^^^^^^^^^^^^^
In the exiting free_mem case I can see the argument for a detached thread
doing the initialization while free_mem is called, and it should be thread
safe to call exit and these functions, avoiding the data race, but perhaps
still crashing for other reasons. We are being consistent in our use of
atomic accesses to provide data race freedom.

But in this case there is no race. Is it just belt-and-suspenders? If so,
we should comment on that.

-- 
Cheers,
Carlos.