Skip to content

Remove reply promise from FIFO when a send is interrupted#13

Merged
henrikbjorn merged 1 commit into
masterfrom
cancellation-safe-send
Jul 15, 2026
Merged

Remove reply promise from FIFO when a send is interrupted#13
henrikbjorn merged 1 commit into
masterfrom
cancellation-safe-send

Conversation

@henrikbjorn

Copy link
Copy Markdown
Member

Summary

Fixes a gradual, process-wide stall on long-lived inbound connections caused by reply-promise poisoning of the shared FIFO.

Base#send_message pushes its reply promise onto @reply_promises before writing the command. If the sending fiber was cancelled (Async::Stop, e.g. a call torn down on hangup while its command was still being flushed) or errored inside send_data, nothing removed the promise — it stayed queued with no corresponding command on the wire, so no reply would ever arrive for it. Since replies are matched purely positionally (@reply_promises.shift in receive_message), one orphaned promise permanently shifts reply correlation by one for every later sender on the same connection: each reply resolves the previous sender's promise, and the newest sender hangs forever with no error.

On an inbound socket (one connection shared by all calls for the process lifetime) this accumulates with every mid-send teardown until every new call stalls silently on its very first round-trip; only a reconnect or process restart clears it. Outbound is immune because its FIFO dies with the per-call socket.

Prior art: a3bb505 / 30a8306 already moved @app_promises off FIFO matching onto Event-UUID correlation for the same fragility; command/reply / api/response carry no correlation token, so the FIFO must instead be kept strictly in sync with the wire.

The fix

If send_data raises (including Async::Stop — hence rescue Exception), remove the promise from the FIFO and reject it, preserving the invariant that a queued promise always corresponds to a command that fully reached the wire. The lone partial-write case degrades to FreeSWITCH replying -ERR to one merged garbage frame, which lands on the sender whose command was swallowed — a truthful failure, with the FIFO staying in sync.

Tests

New regression tests in test/functional/librevox/listener/cancellation_poisoning_test.rb, using a fake connection that can park a sender inside send_data so it can be deterministically stopped in the vulnerable window:

  • a sender stopped mid-send does not eat a later sender's reply
  • reply routing stays correct for all subsequent senders after a cancellation
  • a send that fails with an exception takes the same cleanup path

All three fail on master (starved promise / cross-talked reply / blocked FIFO slot) and pass with the fix. Full suite: 120 runs, 414 assertions, 0 failures.

send_message pushes its reply promise onto the shared @reply_promises
FIFO before writing the command. If the sending fiber was cancelled
(Async::Stop) or errored while blocked inside send_data, the promise
stayed queued with no command on the wire, so no reply would ever
arrive for it. Because replies are matched purely positionally, one
orphaned promise permanently shifted reply correlation by one for every
later sender on the same connection: each reply resolved the previous
sender's promise and the newest sender hung forever, silently.

On a long-lived inbound socket this accumulated with every mid-send
teardown (e.g. a caller hanging up while a command was being flushed)
until every new call stalled on its first round-trip — recoverable only
by reconnect or process restart. Outbound was immune since its FIFO
dies with the per-call socket.

Now send_message removes the promise from the FIFO and rejects it when
send_data raises, keeping the invariant that a queued promise always
corresponds to a command that fully reached the wire.
@henrikbjorn henrikbjorn self-assigned this Jul 15, 2026
@henrikbjorn
henrikbjorn requested a review from bgoRelatel July 15, 2026 11:46
@henrikbjorn
henrikbjorn merged commit c7fc724 into master Jul 15, 2026
3 checks passed
@henrikbjorn
henrikbjorn deleted the cancellation-safe-send branch July 15, 2026 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants