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

[Paul Pluzhnikov <ppluzhnikov@google.com>]

On Sun, Sep 20, 2020 at 5:10 AM Vitaly Buka via Libc-alpha
<libc-alpha@sourceware.org> wrote:

> +static void *
> +threadfunc (void *unused)
> +{
> +  for (; done < 1e6;)
> +    {
> +      if (added < done + 100)
> +        {
> +          __cxa_atexit (&atexitcb, (void *)(++added), __dso_handle);

Isn't there a data race on "added" here (in addition to a data race on "done")?
What prevents two threads from observing "added == 100" at the same
time and adding two calls with value of 101, which would later trigger
abort() in exitcb()?

> +  /* With default 8MiB Linux stack size, creating 1024 threads can cause
> +     VM exhausiton on 32-bit machines.  Reduce stack size of each thread to
> +     128KiB for a maximum required VM size of 128MiB.  */

This comment is far removed from the computation of kStacksize (and
the name violates the naming conventions used here).

I suggest:

  size_t stack_size = 128 << 10; /* 128KiB  */
  if (stack_size < PTHREAD_STACK_MIN) stack_size = PTHREAD_STACK_MIN;

Also, I suspect that 32KiB would be more than enough for stack size here.

> +  for (i = 0; i < kNumThreads; ++i)

Since kNumThreads isn't used anywhere else, I suggest making it a local:

  const int num_threads = 50;

-- 
Paul Pluzhnikov