cloudflare / cloudflare/pingora
closes_only_unclaimed_fds is racy: it asserts a closed fd number is invalid, but other test threads reuse it
- Dominant language
- Rust
- Stars
- 27.4k
- Forks
- 1.7k
- Avg merge
- 6h 22m
- Merged PRs (30d)
- 3
Description
## Describe the bug
`close_unclaimed_tests::closes_only_unclaimed_fds` in
`pingora-core/src/server/transfer_fd/mod.rs` is racy against any other test in the same binary that
opens a file descriptor.
The test closes `drop_fd` through `Fds::close_unclaimed()` and then asserts that the descriptor
**number** has become invalid:
```rust
let closed = fds.close_unclaimed(&keep);
...
assert_eq!(unsafe { libc::fcntl(drop_fd, libc::F_GETFD) }, -1);
assert_eq!(std::io::Error::last_os_error().raw_os_error(), Some(libc::EBADF));
```
File descriptor numbers are process-wide, and `open`/`socket` hand back the lowest free number. Rust
runs the tests of one binary in parallel threads, and several tests in this same file create sockets
(`test_send_receive_fds`, `test_serde_via_socket`, `test_send_fds_to_respects_configurable_timeout`,
`empty_keep_set_closes_everything`, …). If any of them allocates a descriptor between
`close_unclaimed()` and the `fcntl` check, it gets `drop_fd`'s number back and the assertion fails —
even though `close_unclaimed()` did exactly the right thing.
## Pingora info
**Pingora version**: `main` at `0046038` (pristine clone, `git status` clean)
**Rust version**: `cargo 1.97.1 (c980f4866 2026-06-30)`
**Operating system version**: Debian GNU/Linux 13 (trixie), 12 cores, kernel 6.18.33 (WSL2)
## Steps to reproduce
```bash
git clone --depth 1 https://github.com/cloudflare/pingora
cd pingora
for i in $(seq 1 20); do
cargo test -p pingora-core --lib server::transfer_fd 2>&1 \
| grep -E "closes_only_unclaimed_fds \.\.\. (ok|FAILED)"
done
```
## Expected results
20 passes.
## Observed results
**7 failures out of 20** in the environment above. The failure is always the same assertion:
```
thread 'server::transfer_fd::close_unclaimed_tests::closes_only_unclaimed_fds' panicked at
pingora-core/src/server/transfer_fd/mod.rs:592:9:
assertion `left == right` failed
left: 0
right: -1
```
`left: 0` means `fcntl(drop_fd, F_GETFD)` succeeded — i.e. that descriptor number was already valid
again, because another thread had taken it.
It reproduces less often when the machine is otherwise idle, and I have not seen it on this
repository's CI, so it is load- and core-count dependent rather than universal.
## Additional context
The behaviour under test is fine; only the way it is observed is racy. Ideas, in the order I would
rank them:
- Drop the `fcntl`/`EBADF` pair. The assertions above it (`closed` contains the right key, the
entry is gone from `Fds`) already cover what `close_unclaimed()` promises, and they are not racy.
- Or serialize the descriptor-sensitive tests in this file behind a shared mutex, so no other test
can allocate a descriptor inside that window.
Checking the number is inherently unreliable in a multi-threaded process, so I do not think there is
a variant of the current assertion that is both meaningful and stable.
I noticed this while working on #959 / #960, which touch the same function; it is pre-existing and
unrelated to that change (measured at the same rate with and without it). Happy to send a PR for
either direction if you have a preference.
Contributor guide
Assessment
This issue has not been assessed yet.