Merge pull request #936 from pguyot/w45/fix-poll-bug-with-select

Fix a bug in handling listeners and sockets on platforms with poll

On platforms with poll (Linux), the list of fds was not properly built and as a
result, the VM could loop without processing selected fds

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

@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Fixed support for big endian CPUs (such as some MIPS CPUs).
 - Fixed STM32 not aborting when `AVM_ABORT()` is used
 - Fixed a bug that would leave the STM32 trapped in a loop on hard faults, rather than aborting
+- Fixed a bug that would make the VM to loop and failing to process selected fds on Linux
 
 ### Changed
 

src/platforms/generic_unix/lib/sys.c

@@ -178,6 +178,10 @@ static inline void sys_poll_events_with_poll(GlobalContext *glb, int timeout_ms)
     int fd_index;
     if (listeners_poll_count < 0 || select_events_poll_count < 0) {
         // Means it is dirty and should be rebuilt.
+        // The array of polling fds is composed of, in this order:
+        // - the signaling fd (for SMP), which is eventfd or a pipe
+        // - the listeners fd
+        // - the sockets fd
         struct ListHead *select_events = synclist_wrlock(&glb->select_events);
         size_t select_events_new_count;
         if (select_events_poll_count < 0) {
@@ -189,6 +193,7 @@ static inline void sys_poll_events_with_poll(GlobalContext *glb, int timeout_ms)
         size_t listeners_new_count = 0;
         struct ListHead *listeners = NULL;
         struct ListHead *item;
+        // We avoid acquiring a lock on the list of listeners and rebuild the list of fds for listeners if we can
         if (listeners_poll_count < 0) {
             listeners = synclist_rdlock(&glb->listeners);
             LIST_FOR_EACH (item, listeners) {
@@ -217,6 +222,7 @@ static inline void sys_poll_events_with_poll(GlobalContext *glb, int timeout_ms)
 
         fd_index = poll_count;
 
+        // Rebuild the list of fds for listeners if it is dirty
         if (listeners_poll_count < 0) {
             // We put listeners first
             LIST_FOR_EACH (item, listeners) {
@@ -231,6 +237,9 @@ static inline void sys_poll_events_with_poll(GlobalContext *glb, int timeout_ms)
                 }
             }
             synclist_unlock(&glb->listeners);
+        } else {
+            // Else we can reuse the previous list
+            fd_index += listeners_new_count;
         }
 
         // We put select events next
@@ -255,6 +264,9 @@ static inline void sys_poll_events_with_poll(GlobalContext *glb, int timeout_ms)
     poll_count += listeners_poll_count + select_events_poll_count;
 
     int nb_descriptors = poll(fds, poll_count, timeout_ms);
+
+    // After poll, process the list of fds in order, using fd_index as the index
+    // on the list and nb_descriptors as the number of fds to process left
     fd_index = 0;
 #ifndef AVM_NO_SMP
     if (nb_descriptors > 0) {

tests/libs/eavmlib/test_file.erl

@@ -31,6 +31,7 @@ test() ->
     ok = test_gc(HasSelect),
     ok = test_crash_no_leak(HasSelect),
     ok = test_select_with_gone_process(HasSelect),
+    ok = test_select_with_listeners(HasSelect),
     ok.
 
 test_basic_file() ->
@@ -267,3 +268,46 @@ test_select_with_gone_process0(Path) ->
         end,
     ok = atomvm:posix_close(Fd),
     ok.
+
+test_select_with_listeners(false) ->
+    ok;
+test_select_with_listeners(_HasSelect) ->
+    % Create listeners with a udp socket of the inet driver
+    Port = open_port({spawn, "socket"}, []),
+    ok =
+        case
+            port:call(
+                Port,
+                {init, [
+                    {proto, udp},
+                    {port, 0},
+                    {controlling_process, self()},
+                    {active, true},
+                    {buffer, 128},
+                    {timeout, infinity}
+                ]}
+            )
+        of
+            {error, noproc} -> {error, closed};
+            out_of_memory -> {error, enomem};
+            Result -> Result
+        end,
+    Path = "/tmp/atomvm.tmp." ++ integer_to_list(erlang:system_time(millisecond)),
+    ok = atomvm:posix_mkfifo(Path, 8#644),
+    {ok, RdFd} = atomvm:posix_open(Path, [o_rdonly]),
+    {ok, WrFd} = atomvm:posix_open(Path, [o_wronly]),
+    SelectReadRef = make_ref(),
+    ok = atomvm:posix_select_read(RdFd, self(), SelectReadRef),
+    ok =
+        receive
+            {select, RdFd, SelectReadRef, _} -> fail
+        after 200 -> ok
+        end,
+    {ok, 6} = atomvm:posix_write(WrFd, <<" World">>),
+    ok =
+        receive
+            {select, RdFd, SelectReadRef, ready_input} -> ok;
+            M2 -> {unexpected, M2}
+        after 200 -> fail
+        end,
+    ok.