oxidecomputer / oxidecomputer/maghemite
BFD listener deletion/creation race
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 94
- Forks
- 6
- Avg merge
- 1d 8h
- Merged PRs (30d)
- 12
Description
The following is an issue identified by AI when I asked it to do an adversarial code review of #795. This was pre-existing in the sync/pthread impl, which is why it was not addressed as part of the async swap-out.
Problem
Removing the last peer for a listening address removes the listener from
dispatcher bookkeeping before the listener task has been aborted and
joined.
The DELETE handler releases the daemon mutex before awaiting the listener
shutdown handle. This is necessary to avoid awaiting while holding a
synchronous mutex, but it creates the following race:
- DELETE removes the final peer and receives a shutdown handle.
- Dispatcher bookkeeping no longer contains the listener.
- Before DELETE polls or completes listener shutdown, a concurrent PUT
acquires the daemon lock. - PUT attempts to bind the same address.
- The old listener task still owns the socket, so the bind fails with
EADDRINUSE. - PUT returns an internal error even though dispatcher state says the
address is available.
Sequential clients that await DELETE before issuing PUT are protected,
but concurrent reconciliation is not.
Impact
Concurrent configuration reconciliation can lose the old session and
fail to install its replacement. The caller must retry an otherwise
valid configuration update.
Proposed direction
Represent listener shutdown in dispatcher state instead of removing the
resource before its lifetime ends.
Possible designs:
- retain a
ShuttingDownentry and have a new add await it; - serialize add/remove operations through a
tokio::sync::Mutexheld
across listener shutdown; - move listener management into an actor that processes add/remove
operations sequentially.
The synchronization should ideally cover complete session shutdown as
well, including RIB work, rather than only the UDP listener.
Testing
- Add a peer as the only user of a listener.
- Begin deletion while blocking listener shutdown.
- Concurrently attempt to add a peer using the same listen address.
- Verify the add waits rather than failing with
AddrInUse. - Release shutdown and verify the add succeeds.
Related issues
- #787 originally tracked blocking synchronous teardown and noted that
listener/state-machine workers were not reliably joined. - Related follow-up: prevent stale RIB writes from superseded sessions.
Context
A similar socket-lifetime race existed in the synchronous implementation.
#796 fixed the sequential DELETE-then-add case but not concurrent
reconciliation.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the DELETE and PUT handlers, dispatcher bookkeeping, and listener shutdown path described in the issue; review #795, #796, and #787 for existing lifecycle decisions. Reproduce the blocked-shutdown scenario with one peer, then verify that a concurrent add waits for shutdown and succeeds after release without an address-in-use error.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- networking
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100