From b9f1921d87f9f80c399ec89ae8bb193552338941 Mon Sep 17 00:00:00 2001 From: Micael Karlberg Date: Tue, 9 Apr 2024 12:32:16 +0200 Subject: [PATCH 1/6] [esock] Read on Windows with timeout = 0 failure On Windows, when a recv (recv, recvfrom and recvmsg) is called with Timeout = 0 (zero) and the operation could not complete immediately complete, this would result in a (case clause) crash (in the socket module). OTP-19063 --- erts/emulator/nifs/win32/win_socket_asyncio.c | 60 +++++++++---------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/erts/emulator/nifs/win32/win_socket_asyncio.c b/erts/emulator/nifs/win32/win_socket_asyncio.c index bb6ad47960b1..6c4b6c86857a 100644 --- a/erts/emulator/nifs/win32/win_socket_asyncio.c +++ b/erts/emulator/nifs/win32/win_socket_asyncio.c @@ -3869,7 +3869,7 @@ ERL_NIF_TERM recv_check_result(ErlNifEnv* env, } else { - eres = esock_atom_ok; + eres = esock_atom_timeout; // Will trigger {error, timeout} } @@ -3901,7 +3901,7 @@ ERL_NIF_TERM recv_check_ok(ErlNifEnv* env, ERL_NIF_TERM sockRef, ERL_NIF_TERM recvRef) { - ERL_NIF_TERM data, result; + ERL_NIF_TERM data, eres; DWORD read = 0, flags = 0; SSDBG( descP, @@ -3940,7 +3940,7 @@ ERL_NIF_TERM recv_check_ok(ErlNifEnv* env, ESOCK_CNT_INC(env, descP, sockRef, esock_atom_read_fails, &descP->readFails, 1); - result = esock_make_error(env, esock_atom_closed); + eres = esock_make_error(env, esock_atom_closed); } else { @@ -3980,7 +3980,7 @@ ERL_NIF_TERM recv_check_ok(ErlNifEnv* env, if (read > descP->readPkgMax) descP->readPkgMax = read; - result = esock_make_ok2(env, data); + eres = esock_make_ok2(env, data); } @@ -4000,8 +4000,8 @@ ERL_NIF_TERM recv_check_ok(ErlNifEnv* env, if (! IS_ZERO(recvRef)) { - result = recv_check_pending(env, descP, opP, caller, - sockRef, recvRef); + eres = recv_check_pending(env, descP, opP, caller, + sockRef, recvRef); } else { /* But we are not allowed to wait! => cancel */ @@ -4024,11 +4024,11 @@ ERL_NIF_TERM recv_check_ok(ErlNifEnv* env, "\r\n %T" "\r\n", sockRef, descP->sock, reason) ); - result = esock_make_error(env, MKT2(env, tag, reason)); + eres = esock_make_error(env, MKT2(env, tag, reason)); } else { - result = esock_atom_ok; // Will trigger {error, timeout} + eres = esock_atom_timeout; // Will trigger {error, timeout} } } @@ -4050,7 +4050,7 @@ ERL_NIF_TERM recv_check_ok(ErlNifEnv* env, MUNLOCK(ctrl.cntMtx); - result = esock_make_error(env, reason); + eres = esock_make_error(env, reason); } break; } @@ -4061,7 +4061,7 @@ ERL_NIF_TERM recv_check_ok(ErlNifEnv* env, "\r\n", sockRef, descP->sock) ); - return result; + return eres; } @@ -4310,7 +4310,7 @@ ERL_NIF_TERM recvfrom_check_result(ErlNifEnv* env, } else { - eres = esock_atom_ok; // Will trigger {error, timeout} + eres = esock_atom_timeout; // Will trigger {error, timeout} } @@ -4342,7 +4342,7 @@ ERL_NIF_TERM recvfrom_check_ok(ErlNifEnv* env, ERL_NIF_TERM sockRef, ERL_NIF_TERM recvRef) { - ERL_NIF_TERM data, result; + ERL_NIF_TERM data, eres; DWORD read = 0, flags = 0; SSDBG( descP, @@ -4393,7 +4393,7 @@ ERL_NIF_TERM recvfrom_check_ok(ErlNifEnv* env, * This is: {ok, {Source, Data}} * But it should really be: {ok, {Source, Flags, Data}} */ - result = esock_make_ok2(env, MKT2(env, eSockAddr, data)); + eres = esock_make_ok2(env, MKT2(env, eSockAddr, data)); } else { @@ -4411,8 +4411,8 @@ ERL_NIF_TERM recvfrom_check_ok(ErlNifEnv* env, if (! IS_ZERO(recvRef)) { - result = recv_check_pending(env, descP, opP, caller, - sockRef, recvRef); + eres = recv_check_pending(env, descP, opP, caller, + sockRef, recvRef); } else { @@ -4436,11 +4436,11 @@ ERL_NIF_TERM recvfrom_check_ok(ErlNifEnv* env, "\r\n %T" "\r\n", sockRef, descP->sock, reason) ); - result = esock_make_error(env, MKT2(env, tag, reason)); + eres = esock_make_error(env, MKT2(env, tag, reason)); } else { - result = esock_atom_ok; // Will trigger {error, timeout} + eres = esock_atom_timeout; // Will trigger {error, timeout} } } @@ -4462,7 +4462,7 @@ ERL_NIF_TERM recvfrom_check_ok(ErlNifEnv* env, MUNLOCK(ctrl.cntMtx); - result = esock_make_error(env, reason); + eres = esock_make_error(env, reason); } break; } @@ -4472,9 +4472,9 @@ ERL_NIF_TERM recvfrom_check_ok(ErlNifEnv* env, ("WIN-ESAIO", "recvfrom_check_ok(%T) {%d} -> done with" "\r\n result: %T" "\r\n", - sockRef, descP->sock, result) ); + sockRef, descP->sock, eres) ); - return result; + return eres; } @@ -4722,7 +4722,7 @@ ERL_NIF_TERM recvmsg_check_result(ErlNifEnv* env, } else { - eres = esock_atom_ok; // Will trigger {error, timeout} + eres = esock_atom_timeout; // Will trigger {error, timeout} } @@ -4757,7 +4757,7 @@ ERL_NIF_TERM recvmsg_check_ok(ErlNifEnv* env, ERL_NIF_TERM sockRef, ERL_NIF_TERM recvRef) { - ERL_NIF_TERM eMsg, result; + ERL_NIF_TERM eMsg, eres; DWORD read = 0, flags = 0; SSDBG( descP, @@ -4793,7 +4793,7 @@ ERL_NIF_TERM recvmsg_check_ok(ErlNifEnv* env, if (read > descP->readPkgMax) descP->readPkgMax = read; - result = esock_make_ok2(env, eMsg); + eres = esock_make_ok2(env, eMsg); } else { @@ -4811,8 +4811,8 @@ ERL_NIF_TERM recvmsg_check_ok(ErlNifEnv* env, if (! IS_ZERO(recvRef)) { - result = recv_check_pending(env, descP, opP, caller, - sockRef, recvRef); + eres = recv_check_pending(env, descP, opP, caller, + sockRef, recvRef); } else { @@ -4836,11 +4836,11 @@ ERL_NIF_TERM recvmsg_check_ok(ErlNifEnv* env, "\r\n %T" "\r\n", sockRef, descP->sock, reason) ); - result = esock_make_error(env, MKT2(env, tag, reason)); + eres = esock_make_error(env, MKT2(env, tag, reason)); } else { - result = esock_atom_ok; // Will trigger {error, timeout} + eres = esock_atom_timeout; // Will trigger {error, timeout} } } @@ -4862,7 +4862,7 @@ ERL_NIF_TERM recvmsg_check_ok(ErlNifEnv* env, MUNLOCK(ctrl.cntMtx); - result = esock_make_error(env, reason); + eres = esock_make_error(env, reason); } break; } @@ -4872,9 +4872,9 @@ ERL_NIF_TERM recvmsg_check_ok(ErlNifEnv* env, ("WIN-ESAIO", "recvmsg_check_ok(%T) {%d} -> done with" "\r\n result: %T" "\r\n", - sockRef, descP->sock, result) ); + sockRef, descP->sock, eres) ); - return result; + return eres; } From 8d07a31178a669db75f0ae7703e1ccee5dd6d035 Mon Sep 17 00:00:00 2001 From: Micael Karlberg Date: Tue, 9 Apr 2024 12:36:34 +0200 Subject: [PATCH 2/6] [esock] Preliminary test case OTP-19063 --- lib/kernel/test/socket_SUITE.erl | 56 +++++++++++++++++++++++++++++--- 1 file changed, 51 insertions(+), 5 deletions(-) diff --git a/lib/kernel/test/socket_SUITE.erl b/lib/kernel/test/socket_SUITE.erl index 15d52cdc91a4..4decec796459 100644 --- a/lib/kernel/test/socket_SUITE.erl +++ b/lib/kernel/test/socket_SUITE.erl @@ -749,7 +749,8 @@ otp16359_maccept_tcpL/1, otp18240_accept_mon_leak_tcp4/1, otp18240_accept_mon_leak_tcp6/1, - otp18635/1 + otp18635/1, + otp19063/1 ]). @@ -2353,7 +2354,8 @@ tickets_cases() -> [ {group, otp16359}, {group, otp18240}, - otp18635 + otp18635, + otp19063 ]. otp16359_cases() -> @@ -51697,7 +51699,7 @@ do_otp18635(_) -> %% ok = socket:setopt(LSock, otp, debug, true), - % show handle returned from nowait accept + % show handle returned from nowait accept ?P("try accept with timeout = nowait - expect select when" "~n (gen socket) info: ~p" "~n Sockets: ~p", @@ -51738,7 +51740,7 @@ do_otp18635(_) -> ?P("[connector] try create socket"), {ok, CSock} = socket:open(inet, stream), ?P("[connector] try connect: " - "~n (server) ~p", [SA]), + "~n (server) ~p", [SA]), ok = socket:connect(CSock, SA), ?P("[connector] connected - inform parent"), Parent ! {self(), connected}, @@ -51749,7 +51751,7 @@ do_otp18635(_) -> (catch socket:close(CSock)), exit(normal) end - end), + end), ?P("await (connection-) confirmation from connector (~p)", [Connector]), receive @@ -51817,6 +51819,42 @@ do_otp18635(_) -> Result. +%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% + +%% This test case is to verify recv on UDP with timeout zero (0) on Windows. +otp19063(Config) when is_list(Config) -> + ?TT(?SECS(10)), + tc_try(?FUNCTION_NAME, + fun() -> + is_windows(), + has_support_ipv4() + end, + fun() -> + InitState = #{}, + ok = do_otp19063(InitState) + end). + + +do_otp19063(_) -> + ?P("Get \"proper\" local socket address"), + LSA = which_local_socket_addr(inet), + + ?P("Create socket"), + {ok, Sock} = socket:open(inet, dgram), + + ?P("bind socket to: " + "~n ~p", [LSA]), + ok = socket:bind(Sock, LSA), + + ?SLEEP(?SECS(1)), + + {error, timeout} = socket:recv(Sock, 0, 0), + + ?P("done"), + + ok. + + %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% sock_open(Domain, Type, Proto) -> @@ -52275,6 +52313,14 @@ is_not_windows() -> ok end. +is_windows() -> + case os:type() of + {win32, nt} -> + ok; + _ -> + skip("This does not work on *non* Windows"); + end. + is_not_platform(Platform, PlatformStr) when is_atom(Platform) andalso is_list(PlatformStr) -> case os:type() of From a9cc6de93cacee12955d889eeed893e49be3bad8 Mon Sep 17 00:00:00 2001 From: Micael Karlberg Date: Tue, 9 Apr 2024 17:52:30 +0200 Subject: [PATCH 3/6] [esock|test] Improve test case OTP-19063 --- lib/kernel/test/socket_SUITE.erl | 124 ++++++++++++++++++++++++++++--- 1 file changed, 115 insertions(+), 9 deletions(-) diff --git a/lib/kernel/test/socket_SUITE.erl b/lib/kernel/test/socket_SUITE.erl index 4decec796459..7da225e7967b 100644 --- a/lib/kernel/test/socket_SUITE.erl +++ b/lib/kernel/test/socket_SUITE.erl @@ -51697,9 +51697,7 @@ do_otp18635(_) -> ?P("get sockname for listen socket"), {ok, SA} = socket:sockname(LSock), - %% ok = socket:setopt(LSock, otp, debug, true), - - % show handle returned from nowait accept + %% show handle returned from nowait accept ?P("try accept with timeout = nowait - expect select when" "~n (gen socket) info: ~p" "~n Sockets: ~p", @@ -51826,7 +51824,7 @@ otp19063(Config) when is_list(Config) -> ?TT(?SECS(10)), tc_try(?FUNCTION_NAME, fun() -> - is_windows(), + %% is_windows(), has_support_ipv4() end, fun() -> @@ -51836,19 +51834,127 @@ otp19063(Config) when is_list(Config) -> do_otp19063(_) -> + Parent = self(), + ?P("Get \"proper\" local socket address"), LSA = which_local_socket_addr(inet), - ?P("Create socket"), - {ok, Sock} = socket:open(inet, dgram), + + %% --- recv --- + + ?P("Testing recv (tcp) - create (listen) socket"), + {ok, LSock1} = socket:open(inet, stream), + + ?P("bind (listen) socket to: " + "~n ~p", [LSA]), + ok = socket:bind(LSock1, LSA), + + ?P("make listen socket"), + ok = socket:listen(LSock1), + + ?P("get sockname for listen socket"), + {ok, SA1} = socket:sockname(LSock1), + + ?P("attempt a nowait-accept"), + {Tag, Handle} = + case socket:accept(LSock1, nowait) of + {select, {select_info, _, SH}} -> + {select, SH}; + {completion, {completion_info, _, CH}} -> + {completion, CH} + end, + + ?P("spawn the connector process"), + {Connector, MRef} = + spawn_monitor( + fun() -> + ?P("[connector] try create socket"), + {ok, CSock1} = socket:open(inet, stream), + ?P("[connector] try connect: " + "~n (server) ~p", [SA1]), + ok = socket:connect(CSock1, SA1), + ?P("[connector] connected - inform parent"), + Parent ! {self(), connected}, + ?P("[connector] await termination command"), + receive + {Parent, terminate} -> + ?P("[connector] terminate - close socket"), + (catch socket:close(CSock1)), + exit(normal) + end + end), + + ?P("await (connection-) confirmation from connector (~p)", [Connector]), + receive + {Connector, connected} -> + ?P("connector connected"), + ok + end, + + ?P("receive the accepted socket"), + ASock1 = + receive + {'$socket', LSock1, completion, {Handle, {ok, AS}}} + when (Tag =:= completion) -> + AS; + {'$socket', LSock1, completion, {Handle, {error, Reason1C}}} + when (Tag =:= completion) -> + exit({accept_failed, Reason1C}); + {'$socket', LSock1, select, Handle} -> + case socket:accept(LSock1, nowait) of + {ok, AS} -> + AS; + {error, Reason1S} -> + exit({accept_failed, Reason1S}) + end + end, + + ?SLEEP(?SECS(1)), + + ?P("and finally try recv"), + {error, timeout} = socket:recv(ASock1, 0, 0), + + + %% --- recvfrom --- + + ?P("Testing recvfrom - create socket"), + {ok, Sock2} = socket:open(inet, dgram), + + ?P("bind socket to: " + "~n ~p", [LSA]), + ok = socket:bind(Sock2, LSA), + + ?SLEEP(?SECS(1)), + + {error, timeout} = socket:recvfrom(Sock2, 1024, 0), + + + %% --- recvmsg --- + + ?P("Testing recvmsg - create socket"), + {ok, Sock3} = socket:open(inet, dgram), ?P("bind socket to: " "~n ~p", [LSA]), - ok = socket:bind(Sock, LSA), + ok = socket:bind(Sock3, LSA), ?SLEEP(?SECS(1)), - {error, timeout} = socket:recv(Sock, 0, 0), + {error, timeout} = socket:recvmsg(Sock3, 0, 0), + + + ?P("cleanup"), + + Connector ! {self(), terminate}, + receive + {'DOWN', MRef, process, Connector, _} -> + ?P("connector terminated"), + ok + end, + _ = socket:close(ASock1), + _ = socket:close(LSock1), + _ = socket:close(Sock2), + _ = socket:close(Sock3), ?P("done"), @@ -52318,7 +52424,7 @@ is_windows() -> {win32, nt} -> ok; _ -> - skip("This does not work on *non* Windows"); + skip("This does not work on *non* Windows") end. is_not_platform(Platform, PlatformStr) From 50555347d3300f5bcdc7958c37c36b5b21838f86 Mon Sep 17 00:00:00 2001 From: Micael Karlberg Date: Tue, 9 Apr 2024 18:52:50 +0200 Subject: [PATCH 4/6] [esock|test] More test OTP-19063 --- lib/kernel/test/socket_SUITE.erl | 40 +++++++++++++++++++------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/lib/kernel/test/socket_SUITE.erl b/lib/kernel/test/socket_SUITE.erl index 7da225e7967b..93c07b83fe14 100644 --- a/lib/kernel/test/socket_SUITE.erl +++ b/lib/kernel/test/socket_SUITE.erl @@ -51837,25 +51837,27 @@ do_otp19063(_) -> Parent = self(), ?P("Get \"proper\" local socket address"), - LSA = which_local_socket_addr(inet), + LSA0 = which_local_socket_addr(inet), + LSA = LSA0#{port => 0}, + %% --- recv --- - ?P("Testing recv (tcp) - create (listen) socket"), + ?P("[recv] - create (listen) socket"), {ok, LSock1} = socket:open(inet, stream), - ?P("bind (listen) socket to: " + ?P("[recv] bind (listen) socket to: " "~n ~p", [LSA]), ok = socket:bind(LSock1, LSA), - ?P("make listen socket"), + ?P("[recv] make listen socket"), ok = socket:listen(LSock1), - ?P("get sockname for listen socket"), + ?P("[recv] get sockname for listen socket"), {ok, SA1} = socket:sockname(LSock1), - ?P("attempt a nowait-accept"), + ?P("[recv] attempt a nowait-accept"), {Tag, Handle} = case socket:accept(LSock1, nowait) of {select, {select_info, _, SH}} -> @@ -51864,12 +51866,15 @@ do_otp19063(_) -> {completion, CH} end, - ?P("spawn the connector process"), + ?P("[recv] spawn the connector process"), {Connector, MRef} = spawn_monitor( fun() -> ?P("[connector] try create socket"), {ok, CSock1} = socket:open(inet, stream), + ?P("[connector] bind socket to: " + "~n ~p", [LSA]), + ok = socket:bind(CSock1, LSA), ?P("[connector] try connect: " "~n (server) ~p", [SA1]), ok = socket:connect(CSock1, SA1), @@ -51884,14 +51889,15 @@ do_otp19063(_) -> end end), - ?P("await (connection-) confirmation from connector (~p)", [Connector]), + ?P("[recv] await (connection-) confirmation from connector (~p)", + [Connector]), receive {Connector, connected} -> - ?P("connector connected"), + ?P("[recv] connector connected"), ok end, - ?P("receive the accepted socket"), + ?P("[recv] receive the accepted socket"), ASock1 = receive {'$socket', LSock1, completion, {Handle, {ok, AS}}} @@ -51911,36 +51917,38 @@ do_otp19063(_) -> ?SLEEP(?SECS(1)), - ?P("and finally try recv"), + ?P("[recv] try read"), {error, timeout} = socket:recv(ASock1, 0, 0), %% --- recvfrom --- - ?P("Testing recvfrom - create socket"), + ?P("[recvfrom} create socket"), {ok, Sock2} = socket:open(inet, dgram), - ?P("bind socket to: " + ?P("[recvfrom} bind socket to: " "~n ~p", [LSA]), ok = socket:bind(Sock2, LSA), ?SLEEP(?SECS(1)), + ?P("[recvfrom] try read"), {error, timeout} = socket:recvfrom(Sock2, 1024, 0), %% --- recvmsg --- - ?P("Testing recvmsg - create socket"), + ?P("[recvmsg] create socket"), {ok, Sock3} = socket:open(inet, dgram), - ?P("bind socket to: " + ?P("[recvmsg] bind socket to: " "~n ~p", [LSA]), ok = socket:bind(Sock3, LSA), ?SLEEP(?SECS(1)), - {error, timeout} = socket:recvmsg(Sock3, 0, 0), + ?P("[recvmsg] try read"), + {error, timeout} = socket:recvmsg(Sock3, 0), ?P("cleanup"), From 640b8525e3e9ae15afb745a126c672b0e4a006fa Mon Sep 17 00:00:00 2001 From: Micael Karlberg Date: Wed, 10 Apr 2024 10:09:54 +0200 Subject: [PATCH 5/6] [esock|test] Fixes OTP-19063 --- lib/kernel/test/socket_SUITE.erl | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/kernel/test/socket_SUITE.erl b/lib/kernel/test/socket_SUITE.erl index 93c07b83fe14..54ad0b621dc3 100644 --- a/lib/kernel/test/socket_SUITE.erl +++ b/lib/kernel/test/socket_SUITE.erl @@ -51824,7 +51824,7 @@ otp19063(Config) when is_list(Config) -> ?TT(?SECS(10)), tc_try(?FUNCTION_NAME, fun() -> - %% is_windows(), + is_windows(), has_support_ipv4() end, fun() -> @@ -51918,7 +51918,10 @@ do_otp19063(_) -> ?SLEEP(?SECS(1)), ?P("[recv] try read"), - {error, timeout} = socket:recv(ASock1, 0, 0), + case socket:recv(ASock1, 0, 0) of + {error, timeout} -> ok; + Any1 -> ?P("Unexpected result: ~p", [Any1]), exit({unexpected_recv_result, Any1}) + end, %% --- recvfrom --- From daa5bbf25a52f3db0ad722578a99f540461b2844 Mon Sep 17 00:00:00 2001 From: Micael Karlberg Date: Tue, 21 May 2024 17:49:08 +0200 Subject: [PATCH 6/6] [kernel] Handle timeout OTP-19063 --- lib/kernel/src/socket.erl | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/kernel/src/socket.erl b/lib/kernel/src/socket.erl index 68e79e599c34..36952aa504a7 100644 --- a/lib/kernel/src/socket.erl +++ b/lib/kernel/src/socket.erl @@ -3440,6 +3440,8 @@ recv(?socket(SockRef), Length, Flags, Timeout) case prim_socket:recv(SockRef, Length, Flags, zero) of ok -> {error, timeout}; + timeout -> + {error, timeout}; Result -> Result end; @@ -3838,6 +3840,8 @@ recvfrom(?socket(SockRef), BufSz, Flags, Timeout) case prim_socket:recvfrom(SockRef, BufSz, Flags, zero) of ok -> {error, timeout}; + timeout -> + {error, timeout}; Result -> recvfrom_result(Result) end; @@ -4137,6 +4141,8 @@ recvmsg(?socket(SockRef), BufSz, CtrlSz, Flags, Timeout) case prim_socket:recvmsg(SockRef, BufSz, CtrlSz, Flags, zero) of ok -> {error, timeout}; + timeout -> + {error, timeout}; Result -> recvmsg_result(Result) end;