[PATCH v2 08/23] nscd: Fix data races in client retry counters (bug 33654)

[Carlos O'Donell]

On 3/20/26 4:42 PM, Florian Weimer wrote:
> Store the skip counters inside struct mapped_database.  Introduce
> helper functions __nscd_use_database, __nscd_defer_database,
> __nscd_disable_database to access these counters.
> 
> Teach the NSS code to translate to nscd database indices when
> calling these functions.
> 
> Remove the special check from getaddrinfo (in get_nscd_addresses)
> that avoided fallback to an in-process operation for nscd protocol
> errors.  Errors from service modules still produce a response
> from the nscd, so this check only avoided fallback if there were
> certain nscd communication errors.  Checking the skip counters
> in this way for nscd usage was not reliable because another thread
> might have updated the skip counters.
> ---

A quick P&C note from my review with Claude Code v2.1.81 (Sonnet 4.5):

> +bool
> +__nscd_use_database (unsigned int db)
> +{
> +  assert (db < lastdb);
> +
> +  int *pcounter = &__nscd_mapped_databases[db].skip_counter;
> +  int counter;
> +  while (true)
> +    {
> +      counter = atomic_load_relaxed (pcounter);
> +      if (counter <= 0)
> +        break;
> +      else if (counter > 0)
> +        {
> +          int old_counter = counter;
> +          ++counter;
> +          if (counter > NSS_NSCD_RETRY)
> +            counter = 0;
> +          if (atomic_compare_exchange_weak_relaxed
> +	      (pcounter, &old_counter, counter))

The weakened CAS has performance benefits.

It is also possible for this to fail spuriously.

That doesn't change my review, but I thought it was worth mentioning.

We might increment the counter without any use of the database.

We have an outer loop to treat this as-if another thread had changed the value.

It just may not yield the expected number of calls, which should be fine.


> +            break;
> +        }
> +    }
> +  return counter == 0;
> +}



-- 
Cheers,
Carlos.