PR116586/PR121141 guidance for shared_timed_mutex

[Mike Crowe]

On Sunday 03 August 2025 at 21:04:54 +0100, Mike Crowe wrote:
> I'm trying to fix https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116586 .
> Whilst fixing it for std::shared_timed_mutex I've uncovered inconsistencies
> in the behaviour of the existing implementation that I think are at the
> very least confusing and suboptimal. I know what I'd like to change but I'm
> worried that there may be reasons not to do that.
> 
> shared_timed_mutex has four "fundamental" try_lock function implementations
> for the product of steady vs system clock and exclusive vs shared. (There
> are others, but they end up calling one of the four.)
> 
> 1. shared_timed_mutex::try_lock_shared_until() on system_clock loops
>    indefinitely if pthread_rwlock_timedrdlock() returns EDEADLK. The
>    comment above justifies this by saying:
> 
>         // For cases where the implementation detects a deadlock we
>         // intentionally block and timeout so that an early return isn't
>         // mistaken for a spurious failure, which might help users realise
>         // there is a deadlock.
> 
>    This makes some sense but shared_timed_mutex::try_lock_shared_until() on
>    steady_clock, and shared_timed_mutex::try_lock_until() on both clocks
>    don't loop on EDEADLK. They just return false if
>    pthread_rwlock_{clockrd,clockwr,timedwr}lock() return EDEADLK. I can't
>    find any other std::*mutex::try_lock_until functions that loop on
>    EDEADLK.
> 
>    IMO if _GLIBCXX_ASSERTIONS are enabled then it would make more sense to
>    always assert on EDEADLK instead. If the looping is a good idea, then it
>    should be done in all four cases on std::shared_timed_mutex, and perhaps
>    for other std::*timed_mutex classes too. If it's not a good idea, then
>    the looping should be removed from this one place.
> 
>    As it stands, I'm inclined to assert on EDEADLK if _GLIBCXX_ASSERTIONS
>    are enabled and not loop. But does Hyrum's Law mean that it's too late
>    to change the behaviour now?
> 
> 2. All the shared_timed_mutex::try_ functions that take a timeout currently
>    return true (incorrectly indicating that the lock has been taken) in
>    reaction to errors that aren't EAGAIN (sometimes), ETIMEDOUT or EDEADLK.
>    This turns makes bugs like
>    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=121141 even nastier. I
>    think that the functions should always return false on error since the
>    lock has not been taken.
> 
> The indicative patch below implements the above (except the loop removal)
> along with tests. It also has tests to show that std::*timed_mutex,
> std::condition_variable and std::future don't suffer from PR116586. I plan
> to add equivalent tests for std::*semaphore::try_acquire_until() too.

When I added tests for std::binary_semaphore::try_acquire_until() I
discovered that atomic.cc's __platform_wait_until() _does_ suffer from
PR116586. Since fixing it doesn't involve having an answer to the questions
above I will submit a separate patch for that.

Mike.