Skip to content

BFD listener deletion/creation race #863

Description

@taspelund

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:

  1. DELETE removes the final peer and receives a shutdown handle.
  2. Dispatcher bookkeeping no longer contains the listener.
  3. Before DELETE polls or completes listener shutdown, a concurrent PUT
    acquires the daemon lock.
  4. PUT attempts to bind the same address.
  5. The old listener task still owns the socket, so the bind fails with
    EADDRINUSE.
  6. 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 ShuttingDown entry and have a new add await it;
  • serialize add/remove operations through a tokio::sync::Mutex held
    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

  1. Add a peer as the only user of a listener.
  2. Begin deletion while blocking listener shutdown.
  3. Concurrently attempt to add a peer using the same listen address.
  4. Verify the add waits rather than failing with AddrInUse.
  5. Release shutdown and verify the add succeeds.

Related issues

Context

A similar socket-lifetime race existed in the synchronous implementation.
#796 fixed the sequential DELETE-then-add case but not concurrent
reconciliation.

Metadata

Metadata

Assignees

No one assigned

    Labels

    BugbfdBidirectional Forwarding DetectionmgdMaghemite daemonneeds testingrustPull requests that update rust code

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions