Re: [PATCH] stdlib: Fix data race in __run_exit_handlers

[Paul Pluzhnikov <ppluzhnikov@google.com>]

On Tue, Apr 20, 2021 at 3:51 PM Vitaly Buka <vitalybuka@google.com> wrote:

>> > +static void *cb_arg = NULL;
>> > +static void
>> > +add_handlers (void)
>> > +{
>> > +  int n = 10;
>> > +  for (int i = 0; i < n; ++i)
>> > +    __cxa_atexit (&atexit_cb, ++cb_arg, 0);
>>
>> add_handlers() is called from many threads. This code appears to race on cb_arg.
>
>
> We don't have a data race as add_handlers is called from a single background thread.
> Previous patches had more threads but they didn't help to reproduce the issues.

Thanks.

I think cb_arg can be moved into add_handlers() and doesn't have to be
static anymore (if it were a local, there wouldn't be a question of a
race in the first place).

I've looked at the code and the first data race description at the
start of this thread.
I agree that this is the right fix for it.

I have not yet understood the second interaction (between
__run_exit_handlers and __new_exitfn), but I am not sure I really need
to: the patch seems correct.

One other change I would make is to move the unlock before
PTR_DEMANGLE (since it doesn't use any of the data guarded by this
lock).





--
Paul Pluzhnikov