Make Socket#transmit and #close idempotent.#6
Merged
ioquatix merged 8 commits intoMay 22, 2026
Conversation
joeljunstrom
force-pushed
the
socket-idempotent-transmit-close
branch
2 times, most recently
from
9d20a69 to
48a5fe4
Compare
`transmit` pushed to the output queue unconditionally, raising `ClosedQueueError` if the socket had already been closed. That bubbles out of `ActionCable::Server::Base#restart`, which calls `close` on every connection (which itself calls `transmit` to send the disconnect frame) and surfaces as a 500 on the request that triggered the reload. Guard both methods on `@output.closed?` (the same primitive they mutate) The `rescue ClosedQueueError` covers a concurrent close racing past the check.
ioquatix
force-pushed
the
socket-idempotent-transmit-close
branch
from
48a5fe4 to
18c411c
Compare
Member
|
@copilot resolve the merge conflicts in this pull request |
Member
|
I decided to revert this change.
However, in principle, I agree that close should be idempotent, which it already is, and I will retain test for this behaviour. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
transmitpushed to the output queue unconditionally, raisingClosedQueueErrorif the socket had already been closed.That bubbles out of
ActionCable::Server::Base#restart, which callscloseon every connection (which itself callstransmitto send the disconnect frame) and surfaces as a 500 on the request that triggered the reload.Guard both methods on
@output.closed?(the same primitive they mutate). Therescue ClosedQueueErrorcovers a concurrent close racing past the check.This kinda mirrors the question / guidance from actioncable-next#16 (comment)
#closeshould tolerate "already closed" rather than have callers gate on it.Not sure where is the correct seam for this! Just know that would be nice to not have my colleagues naging me about issues running this in rails development =)
Types of Changes
Contribution