Merge pull request #631 from UncleGrumpy/fix_send_reg_proc

Allow sending message to registered process name

Changes `erlang:send/2` to accept a pid or registered process name. Updates
`tests/erlang_tests/test_send_nif_and_echo.erl` to also test sending directly
to the registered name, and `tests/erlang_tests/test_send.erl` to test sending
to an an unregistered `atom` name.

closes #98

These changes are made under both the "Apache 2.0" and the "GNU Lesser General
Public License 2.1 or later" license terms (dual license).

SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later

CHANGELOG.md

@@ -56,6 +56,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Fixed a bug in when putting integers in bit syntax with integer field sizes
 - Fixed numerous bugs in memory allocations that could crash the VM
 - Fixed SNTP support that had been broken in IDF 4.x builds
+- Fixed `erlang:send/2` not sending to registered name
 
 ### Breaking Changes
 

src/libAtomVM/globalcontext.c

@@ -204,7 +204,7 @@ COLD_FUNC void globalcontext_destroy(GlobalContext *glb)
     free(glb);
 }
 
-static Context *globalcontext_get_process_nolock(GlobalContext *glb, int32_t process_id)
+Context *globalcontext_get_process_nolock(GlobalContext *glb, int32_t process_id)
 {
     struct ListHead *item;
     Context *p = NULL;

src/libAtomVM/globalcontext.h

@@ -140,6 +140,18 @@ GlobalContext *globalcontext_new();
  */
 void globalcontext_destroy(GlobalContext *glb);
 
+/**
+ * @brief Gets a Context from the process table, without acquiring a lock on the process table.
+ *
+ * @details Retrieves from the process table the context with the given local
+ * process id. If the process can be found, without locking the process table.
+ * This is unsafe unless a lock on the process table has been obtained previously.
+ * @param glb the global context (that owns the process table).
+ * @param process_id the local process id.
+ * @returns a Context * with the requested local process id or NULL if not found.
+ */
+Context *globalcontext_get_process_nolock(GlobalContext *glb, int32_t process_id);
+
 /**
  * @brief Gets a Context from the process table, acquiring a lock on the process
  * table.

src/libAtomVM/nifs.c

@@ -1284,12 +1284,40 @@ static term nif_erlang_spawn(Context *ctx, int argc, term argv[])
 static term nif_erlang_send_2(Context *ctx, int argc, term argv[])
 {
     UNUSED(argc);
+    term target = argv[0];
+    GlobalContext *glb = ctx->global;
+
+    if (term_is_pid(target)) {
+        int32_t local_process_id = term_to_local_process_id(target);
+
+        globalcontext_send_message(glb, local_process_id, argv[1]);
+
+    } else if (term_is_atom(target)) {
+        // We need to hold a lock on the processes_table until the message is sent to avoid a race condition,
+        // otherwise the receiving process could be killed at any point between checking it is registered,
+        // obtaining its process_id, and sending the message, any of which would lead to crashes or incorrect
+        // behavior that does not match OTP.
+        struct ListHead *dummy = synclist_rdlock(&glb->processes_table);
+        UNUSED(dummy);
+        int atom_index = term_to_atom_index(target);
+
+        int local_process_id = globalcontext_get_registered_process(glb, atom_index);
+        if (UNLIKELY(local_process_id == 0)) {
+            synclist_unlock(&glb->processes_table);
+            RAISE_ERROR(BADARG_ATOM);
+        }
 
-    term pid_term = argv[0];
-    VALIDATE_VALUE(pid_term, term_is_pid);
+        Context *p = globalcontext_get_process_nolock(glb, local_process_id);
+        if (IS_NULL_PTR(p)) {
+            synclist_unlock(&glb->processes_table);
+            RAISE_ERROR(BADARG_ATOM);
+        }
 
-    int local_process_id = term_to_local_process_id(pid_term);
-    globalcontext_send_message(ctx->global, local_process_id, argv[1]);
+        globalcontext_send_message_nolock(glb, local_process_id, argv[1]);
+        synclist_unlock(&glb->processes_table);
+    } else {
+        RAISE_ERROR(BADARG_ATOM);
+    }
 
     return argv[1];
 }

tests/erlang_tests/test_send.erl

@@ -20,15 +20,74 @@
 
 -module(test_send).
 
--export([start/0, send2/2]).
+-export([start/0]).
 
 start() ->
-    send2(5, 1) + send2(self(), -1).
+    Dead = spawn(fun() -> ok end),
+    Sent = send(self(), 32),
+    receive
+        Sent ->
+            Recv = Sent;
+        _Any ->
+            Recv = 64
+    after 5000 ->
+        Recv = 128
+    end,
+    T1 = Sent - Recv,
+    T2 = send_mal(5, 3),
+    T3 = send_bad_atom(bogus, 4),
+    T4 = send_dead(Dead, 6),
+    T5 = send_registered(8),
+    T1 + T2 + T3 + T4 + T5.
 
-send2(A, B) ->
+send(A, B) ->
     try erlang:send(A, B) of
-        B -> -1;
-        Any -> Any
+        B -> B;
+        _Any -> -1
+    catch
+        error:badarg -> B - 2;
+        _:_ -> -4
+    end.
+
+send_mal(A, B) ->
+    try erlang:send(A, B) of
+        B -> B;
+        _Any -> -1
+    catch
+        error:badarg -> B - 3;
+        _:_ -> -4
+    end.
+
+send_bad_atom(A, B) ->
+    try erlang:send(A, B) of
+        B -> B;
+        _Any -> -1
+    catch
+        error:badarg -> B - 4;
+        _:_ -> -4
+    end.
+
+send_dead(A, B) ->
+    try erlang:send(A, B) of
+        B -> B - 6;
+        _Any -> -1
+    catch
+        error:badarg -> -2;
+        _:_ -> -4
+    end.
+
+send_registered(B) ->
+    erlang:register(listen, self()),
+    try erlang:send(listen, B) of
+        B ->
+            receive
+                B -> B - 8;
+                _Any -> -1
+            after 5000 ->
+                512
+            end;
+        _Any ->
+            -1
     catch
         error:badarg -> -2;
         _:_ -> -4

tests/erlang_tests/test_send_nif_and_echo.erl

@@ -24,13 +24,20 @@
 
 start() ->
     register(echo, do_open_port(<<"echo">>, [])),
-    byte_size(echo(<<"Hello World">>)).
+    byte_size(echo(<<"Hello World">>)) + byte_size(to_pid(erlang:whereis(echo), <<"Hello World">>)).
 
 do_open_port(PortName, Param) ->
     open_port({spawn, PortName}, Param).
 
 echo(SendValue) ->
-    erlang:send(whereis(echo), {self(), SendValue}),
+    erlang:send(echo, {self(), SendValue}),
+    receive
+        Value ->
+            Value
+    end.
+
+to_pid(Pid, SendValue) ->
+    erlang:send(Pid, {self(), SendValue}),
     receive
         Value ->
             Value

tests/test.c

@@ -174,7 +174,7 @@ struct Test tests[] = {
     TEST_CASE_EXPECTED(long_atoms, 4),
     TEST_CASE_EXPECTED(test_concat_badarg, 4),
     TEST_CASE_EXPECTED(register_and_whereis_badarg, 333),
-    TEST_CASE_EXPECTED(test_send, -3),
+    TEST_CASE(test_send),
     TEST_CASE_EXPECTED(test_open_port_badargs, -21),
     TEST_CASE_EXPECTED(prime_ext, 1999),
     TEST_CASE_EXPECTED(test_try_case_end, 256),
@@ -478,7 +478,7 @@ struct Test tests[] = {
     TEST_CASE_ATOMVM_ONLY(test_close_console_driver, 0),
     TEST_CASE_ATOMVM_ONLY(test_close_echo_driver, 0),
     TEST_CASE_ATOMVM_ONLY(test_regecho_driver, 11),
-    TEST_CASE_ATOMVM_ONLY(test_send_nif_and_echo, 11),
+    TEST_CASE_ATOMVM_ONLY(test_send_nif_and_echo, 22),
 
     TEST_CASE_EXPECTED(test_code_load_binary, 24),
     TEST_CASE_EXPECTED(test_code_load_abs, 24),