Fix possible race condition in port:call/2,3

Also fix bug in gen_* that hung if underlying socket was closed, by
factorizing logic and invoking port:call/2

This bug created issues with esp32's socket_driver implementation that
terminates the process when the socket is closed from the other end

Also fix semantics of gen_*:close() that should always return ok.

Also update test_port test to actually test with a port as port module no
longer can be used with non-ports.

Signed-off-by: Paul Guyot <[email protected]>

Author: pguyot

libs/eavmlib/src/port.erl

@@ -24,31 +24,26 @@
 -export([call/2, call/3]).
 
 -spec call(pid(), Message :: term()) -> term().
-call(Pid, Message) ->
-    case erlang:is_process_alive(Pid) of
-        false ->
-            {error, noproc};
-        true ->
-            Ref = erlang:make_ref(),
-            Pid ! {self(), Ref, Message},
-            receive
-                out_of_memory -> out_of_memory;
-                {Ref, Reply} -> Reply
-            end
-    end.
+call(Port, Message) ->
+    call(Port, Message, infinity).
 
 -spec call(pid(), Message :: term(), TimeoutMs :: non_neg_integer()) -> term() | {error, timeout}.
-call(Pid, Message, TimeoutMs) ->
-    case erlang:is_process_alive(Pid) of
-        false ->
-            {error, noproc};
-        true ->
-            Ref = erlang:make_ref(),
-            Pid ! {self(), Ref, Message},
-            receive
-                out_of_memory -> out_of_memory;
-                {Ref, Reply} -> Reply
-            after TimeoutMs ->
-                {error, timeout}
-            end
-    end.
+call(Port, Message, TimeoutMs) ->
+    Ref = erlang:make_ref(),
+    MonitorRef = monitor(port, Port),
+    Port ! {self(), Ref, Message},
+    Result =
+        receive
+            {'DOWN', MonitorRef, port, Port, normal} ->
+                {error, noproc};
+            {'DOWN', MonitorRef, port, Port, Reason} ->
+                {error, Reason};
+            out_of_memory ->
+                out_of_memory;
+            {Ref, Ret} ->
+                Ret
+        after TimeoutMs ->
+            {error, timeout}
+        end,
+    demonitor(MonitorRef, [flush]),
+    Result.

libs/estdlib/src/gen_tcp.erl

@@ -237,15 +237,6 @@ connect(DriverPid, Address, Port, Params) ->
             ErrorReason
     end.
 
-%% @private
-call(DriverPid, Msg) ->
-    Ref = erlang:make_ref(),
-    DriverPid ! {self(), Ref, Msg},
-    receive
-        {Ref, Ret} ->
-            Ret
-    end.
-
 %% TODO implement this in lists
 
 %% @private
@@ -286,3 +277,14 @@ normalize_address({A, B, C, D}) when
         "." ++ integer_to_list(D).
 
 %% TODO IPv6
+
+%%
+%% Internal operations
+%%
+
+call(Port, Msg) ->
+    case port:call(Port, Msg) of
+        {error, noproc} -> {error, closed};
+        out_of_memory -> {error, enomem};
+        Result -> Result
+    end.

libs/estdlib/src/gen_udp.erl

@@ -135,14 +135,13 @@ recv(Socket, Length, Timeout) ->
 
 %%-----------------------------------------------------------------------------
 %% @param   Socket the socket to close
-%% @returns ok, if closing the socket succeeded, or {error, Reason}, if
-%%          closing the socket failed for any reason.
+%% @returns ok
 %% @doc     Close the socket.
 %% @end
 %%-----------------------------------------------------------------------------
--spec close(inet:socket()) -> ok | {error, Reason :: reason()}.
+-spec close(inet:socket()) -> ok.
 close(Socket) ->
-    call(Socket, {close}).
+    inet:close(Socket).
 
 %%-----------------------------------------------------------------------------
 %% @param   Socket the socket to which to assign the pid
@@ -179,15 +178,6 @@ init(DriverPid, PortNum, Params) ->
             ErrorReason
     end.
 
-%% @private
-call(DriverPid, Msg) ->
-    Ref = erlang:make_ref(),
-    DriverPid ! {self(), Ref, Msg},
-    receive
-        {Ref, Ret} ->
-            Ret
-    end.
-
 %% TODO implement this in lists
 
 %% @private
@@ -209,3 +199,14 @@ merge(Config, [H | T], Accum) ->
         Value ->
             merge(Config, T, [{Key, Value} | Accum])
     end.
+
+%%
+%% Internal operations
+%%
+
+call(Port, Msg) ->
+    case port:call(Port, Msg) of
+        {error, noproc} -> {error, closed};
+        out_of_memory -> {error, enomem};
+        Result -> Result
+    end.

libs/estdlib/src/inet.erl

@@ -70,7 +70,8 @@ port(Socket) ->
 %%-----------------------------------------------------------------------------
 -spec close(Socket :: socket()) -> ok.
 close(Socket) ->
-    call(Socket, {close}).
+    call(Socket, {close}),
+    ok.
 
 %%-----------------------------------------------------------------------------
 %% @param   Socket the socket
@@ -98,11 +99,9 @@ peername(Socket) ->
 %% Internal operations
 %%
 
-%% @private
-call(Socket, Msg) ->
-    Ref = erlang:make_ref(),
-    Socket ! {self(), Ref, Msg},
-    receive
-        {Ref, Ret} ->
-            Ret
+call(Port, Msg) ->
+    case port:call(Port, Msg) of
+        {error, noproc} -> {error, einval};
+        out_of_memory -> {error, enomem};
+        Result -> Result
     end.

tests/libs/eavmlib/test_port.erl

@@ -20,44 +20,17 @@
 
 -module(test_port).
 
--export([test/0, loop/0]).
+-export([test/0]).
 
 test() ->
-    {Pid, Ref} = spawn_opt(?MODULE, loop, [], [monitor]),
+    ConsolePort = open_port({spawn, "console"}, []),
 
-    pong = port:call(Pid, ping),
-    pong = port:call(Pid, ping, 1000),
-    out_of_memory = port:call(Pid, out_of_memory),
-    out_of_memory = port:call(Pid, out_of_memory, 1000),
-
-    {error, timeout} = port:call(Pid, {sleep, 500}, 250),
-
-    ok = port:call(Pid, halt),
+    ok = port:call(ConsolePort, flush),
+    ok = port:call(ConsolePort, flush, 1000),
+    {error, badarg} = port:call(ConsolePort, unknown_cmd),
+    ConsolePort ! {self(), close},
     receive
-        {'DOWN', Ref, process, Pid, normal} -> ok
+        {ConsolePort, closed} -> ok
     end,
-    {error, noproc} = port:call(Pid, ping),
-    {error, noproc} = port:call(Pid, ping, 1000),
-
+    {error, noproc} = port:call(ConsolePort, flush),
     ok.
-
-loop() ->
-    receive
-        {Pid, Ref, ping} ->
-            Pid ! {Ref, pong},
-            loop();
-        {Pid, _Ref, out_of_memory} ->
-            Pid ! out_of_memory,
-            loop();
-        {Pid, Ref, {sleep, Ms}} ->
-            receive
-            after Ms -> ok
-            end,
-            Pid ! {Ref, ok},
-            loop();
-        {Pid, Ref, halt} ->
-            Pid ! {Ref, ok};
-        Garbage ->
-            erlang:display({unexpected_message, Garbage}),
-            loop()
-    end.

tests/libs/estdlib/test_gen_tcp.erl

@@ -28,6 +28,7 @@ test() ->
     ok = test_echo_server(),
     ok = test_echo_server(true),
     ok = test_listen_connect_parameters(),
+    ok = test_tcp_double_close(),
     ok.
 
 test_echo_server() ->
@@ -229,3 +230,10 @@ test_listen_connect_parameters_server_loop(ListenMode, false = ListenActive, Soc
         Other ->
             {error, {unexpected_result, server, Other}}
     end.
+
+test_tcp_double_close() ->
+    {ok, Socket} = gen_tcp:listen(10543, [{active, false}]),
+    ok = gen_tcp:close(Socket),
+    ok = gen_tcp:close(Socket),
+    {error, closed} = gen_tcp:recv(Socket, 512, 5000),
+    ok.