| Issue |
203411
|
| Summary |
[libc] `EAGAIN/EWOULDBLOCK` can be treated as `ETIMEOUT` in `Mutex` and `RwLock`
|
| Labels |
libc
|
| Assignees |
|
| Reporter |
SchrodingerZhu
|
## Problem Description
Under high contention, timed lock operations (`pthread_mutex_timedlock`, `pthread_rwlock_timedrdlock`, `pthread_rwlock_timedwrlock`) in LLVM libc can spuriously fail and return `ETIMEDOUT` (or `false`) before the specified timeout duration has actually elapsed.
This occurs because the slow path lock acquisition logic in both `RawMutex` and `RawRwLock` incorrectly treats any failure of the underlying `Futex::wait` call as a timeout when a timeout is present.
Specifically, if the futex value in memory changes between the user-space check and the kernel-space check, the kernel's `sys_futex(FUTEX_WAIT)` system call returns `-EAGAIN`. The `Futex::wait` helper propagates this as a `cpp::unexpected(EAGAIN)` error.
The lock implementations check the result as follows:
### In `RawMutex::lock_slow` (src/__support/threads/raw_mutex.h):
```cpp
if (!futex.wait(IN_CONTENTION, timeout, is_pshared).has_value() &&
timeout.has_value())
return false; // Returns false (Timeout) on any error, including EAGAIN
```
### In `RawRwLock::lock_slow` (src/__support/threads/raw_rwlock.h):
```cpp
auto wait_result = queue.wait<role>(serial_number, timeout, is_pshared);
timeout_flag = (!wait_result.has_value() && timeout.has_value()); // Sets timeout_flag on EAGAIN
```
Because `EAGAIN` is a normal transition state indicating that the lock state changed and the thread should retry, treating it as a timeout violates correct lock behavior and causes spurious failures under contention.
---
## Proposed Fix
Modify the timeout check in both `RawMutex::lock_slow` and `RawRwLock::lock_slow` to explicitly verify that the error returned by `wait` is indeed `ETIMEDOUT` before failing. Other transient errors like `EAGAIN` should be ignored, allowing the acquisition loop to retry.
### Fix for `RawMutex` (`src/__support/threads/raw_mutex.h`):
```diff
@@ -83,8 +83,8 @@
if (state != IN_CONTENTION &&
futex.exchange(IN_CONTENTION, cpp::MemoryOrder::ACQUIRE) == UNLOCKED)
return true;
- if (!futex.wait(IN_CONTENTION, timeout, is_pshared).has_value() &&
- timeout.has_value())
- return false;
+ auto wait_result = futex.wait(IN_CONTENTION, timeout, is_pshared);
+ if (!wait_result.has_value() && wait_result.error() == ETIMEDOUT)
+ return false;
state = spin(spin_count);
}
```
### Fix for `RawRwLock` (`src/__support/threads/raw_rwlock.h`):
```diff
@@ -401,5 +401,5 @@
bool timeout_flag = false;
if (!old.can_acquire<role>(get_preference())) {
auto wait_result = queue.wait<role>(serial_number, timeout, is_pshared);
- timeout_flag = (!wait_result.has_value() && timeout.has_value());
+ timeout_flag = (!wait_result.has_value() && wait_result.error() == ETIMEDOUT);
}
```
![]()
_______________________________________________
llvm-bugs mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-bugs
