[FFmpeg-devel] [PATCH] Stop using _explicit atomic operations where not necessary.

[Anton Khirnov]

Quoting Andreas Rheinhardt (2021-06-06 11:13:00)
> Anton Khirnov:
> > Memory ordering constraints other than the default (sequentially
> > consistent) can behave in very unintuitive and unexpected ways, and so
> > should be avoided unless there is a strong (typically performance)
> > reason for using them. This holds especially for memory_order_relaxed,
> > which imposes no ordering constraints.
> 
> I disagree with this: Adding ordering constraints is wrong, as it can
> provide synchronization between different threads and this can mask
> races. Providing synchronization as a byproduct should be avoided: Users
> should know what they get. And if a user wants an allocation limit in
> thread A to be in effect for allocations in thread B, then the user
> should provide the synchronization him/herself.

I do not agree. C11 default is sequential consistency, therefore our
public API should provide sequential consistency.

My other concern is that people are cargo culting this code around
without understanding what it really does. Again, sequentially
consistent atomics make that safer, since they do what you'd expect them
to.

> 
> Here is an example of a program that contains a data race that is masked
> by this patch: If all allocations happen before av_max_alloc(3), then
> tsan will complain about a data race; otherwise, it is silent, as the
> write in av_max_alloc() synchronizes with the read in av_malloc(), so
> that race = 1 is visible in the main thread later.

This example is highly artificial and IMO not relevant, since
- nothing in the libraries should call av_max_malloc() or other
  functions that modify global state
- it is not our job to look for races in user programs

-- 
Anton Khirnov