Catch file:sendfile badmatch to keep replica reader alive - #231
Open
lukebakken wants to merge 1 commit into
Open
Catch file:sendfile badmatch to keep replica reader alive#231lukebakken wants to merge 1 commit into
file:sendfile badmatch to keep replica reader alive#231lukebakken wants to merge 1 commit into
Conversation
|
Tick the box to add this pull request to the merge queue (same as
|
This was referenced Aug 7, 2026
A 24-hour high-throughput test crashed `osiris_replica_reader` three
times with `{badmatch,undefined}` raised from inside `file:sendfile/8`.
The crash is a TOCTOU race in OTP's `prim_inet:sendfile/4`: it guards on
`erlang:port_info(S, connected)`, then calls `getprotocol/1`, which does
a second, unguarded `{name,Drv} = erlang:port_info(S, name)`. When the
peer closes the replication socket between those two calls, the second
`port_info/2` returns `undefined` and the match raises.
`osiris_log:sendfile/6` calls `file:sendfile/8` inside a plain `case`
that only handles a returned `{ok,_}` / `{error,_}`, so the thrown
exception is uncaught and propagates through `send_file/3` to
`do_sendfile0/1`, terminating the reader with a noisy crash report even
though the condition is benign and already recovered by reconnect.
Wrap the call in a `try` that catches `error:{badmatch,_}` and maps it
to `{error, {sendfile, Reason}}`, the shape `do_sendfile0/1` already
handles gracefully. Every other exception still propagates so real
faults crash loudly. Add a regression test that mocks `file:sendfile`
to raise the badmatch, since the real race is not reproducible on
demand, and add `meck` as a test dependency.
The getprotocol/1 badmatch is an OTP bug (rabbitmq#230) to be
patched upstream; this guard is defence-in-depth for current releases.
lukebakken
force-pushed
the
fix-replica-reader-sendfile-badmatch-crash
branch
from
August 7, 2026 22:24
5c6ad83 to
681cb09
Compare
This was referenced Aug 11, 2026
Open
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.
Note
This PR was prepared by Claude (Anthropic's Claude Code) under the direction of @lukebakken, who reviewed the change before opening it. The fix was surfaced by a 24-hour long-running high-throughput test. The code and analysis are AI-drafted and human-reviewed.
Problem
A 24-hour high-throughput test crashed
osiris_replica_readerthree times with{badmatch,undefined}raised from insidefile:sendfile/8.The crash is a TOCTOU race in OTP's
prim_inet:sendfile/4: it guards onerlang:port_info(S, connected), then callsgetprotocol/1, which does a second, unguarded{name,Drv} = erlang:port_info(S, name). When the peer closes the replication socket between those two calls, the secondport_info/2returnsundefinedand the match raises. (A fully-closed socket instead fails theconnectedguard and returns{error, einval}; only the race raises.)osiris_log:sendfile/6callsfile:sendfile/8inside a plaincasethat only handles a returned{ok,_}/{error,_}, so the thrown exception is uncaught and propagates throughsend_file/3todo_sendfile0/1, terminating the reader with a noisy[error]crash report even though the condition is benign and already recovered by reconnect.Full analysis in #230.
Solution
Wrap the
file:sendfile/8call in atrythat catcheserror:{badmatch,_}and maps it to{error, {sendfile, Reason}}, the shapedo_sendfile0/1already handles gracefully. Every other exception still propagates so real faults crash loudly.Add a regression test (
send_file_raises_are_returned_as_errors) that mocksfile:sendfileto raise the badmatch, since the real race is not reproducible on demand, and addmeckas a test dependency.The
getprotocol/1badmatch is an OTP bug (see #230) to be patched upstream; this guard is defence-in-depth for current OTP releases.Closes #230