Catch file:sendfile badmatch to keep replica reader alive

Makefile

@@ -22,12 +22,13 @@ DEPS = gen_batch_server seshat
 # TEST_DEPS=eunit_formatters looking_glass
 dep_tls_gen = git https://github.com/rabbitmq/tls-gen.git main
 dep_proper = hex 1.4.0
-TEST_DEPS=eunit_formatters tls_gen proper
+dep_meck = hex 1.0.0
+TEST_DEPS=eunit_formatters tls_gen proper meck
 
 dep_looking_glass = git https://github.com/rabbitmq/looking-glass.git master
 # PLT_APPS += eunit syntax_tools erts kernel stdlib common_test inets ssh ssl meck looking_glass gen_batch_server inet_tcp_proxy
 
 DIALYZER_OPTS += --src -r test -Wunmatched_returns -Werror_handling
-PLT_APPS += seshat ssl eunit common_test proper
+PLT_APPS += seshat ssl eunit common_test proper meck
 EUNIT_OPTS = no_tty, {report, {eunit_progress, [colored, profile]}}
 include $(if $(ERLANG_MK_FILENAME),$(ERLANG_MK_FILENAME),erlang.mk)

rebar.config

@@ -6,6 +6,12 @@
 
 ]}.
 
+{profiles, [
+    {test, [
+        {deps, [{meck, "1.0.0"}]}
+    ]}
+]}.
+
 {format, [
     {files, ["src/*.erl", "test/*.erl"]},
     {formatter, default_formatter},

src/osiris_log.erl

@@ -2655,14 +2655,30 @@ max_segment_size_reached(
 sendfile(_UseSendfile, _Transport, _Fd, _Sock, _Pos, 0) ->
     ok;
 sendfile(true, Transport, Fd, Sock, Pos, ToSend) ->
-    case file:sendfile(Fd, Sock, Pos, ToSend, []) of
+    %% file:sendfile/5 can raise (not just return {error, _}) on a peer socket
+    %% close that races the send. prim_inet:sendfile/4 checks the port is still
+    %% connected, then calls sendfile_maybe_cork/1 -> getprotocol/1, which does
+    %% `{name,Drv} = erlang:port_info(S, name)`. If the peer closes the port in
+    %% between, port_info/2 returns `undefined` and that match raises
+    %% `{badmatch,undefined}`. (A fully-closed socket instead fails the earlier
+    %% connected check and returns {error, einval}; only the race raises.) An
+    %% uncaught exception here would terminate the replica reader with a noisy
+    %% crash report, so map that specific badmatch to the {error, _} the caller
+    %% already handles gracefully. Any other exception is a real fault and is
+    %% left to propagate. The underlying getprotocol/1 badmatch is an OTP bug
+    %% (see rabbitmq/osiris#230) that will be investigated and patched upstream;
+    %% this guard is defence-in-depth for current OTP releases.
+    try file:sendfile(Fd, Sock, Pos, ToSend, []) of
         {ok, 0} ->
             %% TODO add counter for this?
             sendfile(true, Transport, Fd, Sock, Pos, ToSend);
         {ok, BytesSent} ->
             sendfile(true, Transport, Fd, Sock, Pos + BytesSent, ToSend - BytesSent);
         {error, _} = Err ->
             Err
+    catch
+        error:{badmatch, _} = Reason ->
+            {error, {sendfile, Reason}}
     end;
 sendfile(false, Transport, Fd, Sock, Pos, ToSend) ->
     case file:pread(Fd, Pos, ToSend) of

test/osiris_log_SUITE.erl

@@ -12,6 +12,11 @@
 
 -export([]).
 
+%% send_file_raises_are_returned_as_errors mocks file:sendfile with a fun that
+%% always raises, which -Werror_handling flags as "only terminates with
+%% explicit exception". That is intentional for this regression test.
+-dialyzer({nowarn_function, send_file_raises_are_returned_as_errors/1}).
+
 -include_lib("common_test/include/ct.hrl").
 -include_lib("eunit/include/eunit.hrl").
 -include_lib("kernel/include/file.hrl").
@@ -99,6 +104,7 @@ all_tests() ->
      init_with_unexpected_file,
      overview_with_missing_segment,
      overview_with_missing_index_at_start,
+     send_file_raises_are_returned_as_errors,
      read_ahead_send_file,
      read_ahead_send_file_filter,
      read_ahead_send_file_on_off,
@@ -2515,6 +2521,43 @@ assert_sendfile_pread(T, ExpectedSendfile, ExpectedPread) ->
             ?assertEqual(ExpectedPread, osiris_tracer:call_count(T, file, pread))
     end.
 
+send_file_raises_are_returned_as_errors(Config) ->
+    %% Regression test for rabbitmq/osiris#230. file:sendfile/5 can raise
+    %% (rather than return {error, _}) when the peer closes the replication
+    %% socket in the window between prim_inet:sendfile/4's connected check and
+    %% getprotocol/1's port_info/2 call, which badmatches on the `undefined`
+    %% a closed port returns. osiris_log:sendfile/6 must map that raised
+    %% badmatch to the {error, _} send_file/3 already handles, so the replica
+    %% reader does not crash. file:sendfile is mocked to raise deterministically
+    %% because the real race is not reproducible on demand.
+    Conf0 = ?config(osiris_conf, Config),
+    Wr0 = osiris_log:init(Conf0),
+    Shared = osiris_log:get_shared(Wr0),
+    RConf = Conf0#{shared => Shared, transport => tcp},
+    {ok, Rd0} = osiris_log:init_offset_reader(first, RConf),
+
+    %% a chunk large enough to force the sendfile path rather than read-ahead
+    Entries = [binary:copy(<<"a">>, 8192)],
+    {_, Wr1} = write_committed(Entries, Wr0),
+
+    {CS, _SS, _} = CSC = client_server_connect(),
+
+    meck:new(file, [passthrough, unstick, no_link]),
+    try
+        meck:expect(file, sendfile,
+                    fun(_Fd, _Sock, _Pos, _ToSend, _Opts) ->
+                            error({badmatch, undefined})
+                    end),
+        ?assertEqual({error, {sendfile, {badmatch, undefined}}},
+                     osiris_log:send_file(CS, Rd0))
+    after
+        meck:unload(file),
+        client_server_close(CSC),
+        osiris_log:close(Rd0),
+        osiris_log:close(Wr1)
+    end,
+    ok.
+
 read_ahead_send_file(Config) ->
     RAL = 4096, %% read ahead limit
     HS = ?HEADER_SIZE_B,