Repository navigation
Forward discard! to the replica connection - #178
Merged
Merged
Conversation
ActiveRecord's ForkTracker calls discard! on every adapter in a forked child (PoolConfig.discard_pools!) so the child never closes a socket shared with the parent. Janus forwarded connect!, reconnect!, disconnect! and clear_cache! to the replica but not discard!, so a forked child kept a live replica client. Garbage collecting it in the child sent COM_QUIT down the socket the parent was still using. Refs #176 (finding 2). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
lloydwatkin
changed the title
discard! to the replica connection
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
Fixes finding 2 of #176.
ActiveRecord registers
ActiveSupport::ForkTracker.after_fork { PoolConfig.discard_pools! }, which callsdiscard!on every adapter in a forked child so the child never closes a socket it shares with the parent. Janus forwardedconnect!,reconnect!,disconnect!andclear_cache!to the replica but notdiscard!, so in a forked child (Puma cluster, Sidekiq swarm, Spring, parallel test runners) the replica adapter kept a liveMysql2::Client/Trilogyobject. Garbage collecting it in the child sendsCOM_QUITdown the socket the parent is still using.Tests
New shared example
an adapter forwarding lifecycle calls, included from both the mysql2 and trilogy adapter specs. Verified red before the fix (received: 0 times) and green after. RuboCop clean.Note on the audit text
The issue says "
throw_away!callsdiscard!internally so it is covered by the same override". That is not true in AR 8.1.3.1 —throw_away!ispool.remove self; disconnect!.disconnect!is already forwarded to the replica, so that path is fine, but it does not go throughdiscard!. No extra override needed.The remaining findings in #176 are not addressed here.
🤖 Generated with Claude Code