Forward port changes from v0.6 release branch

Merge a number of fixes and improvements from release-0.6 into main.
Including UART improvements on ESP32 and
"run GC when there is a memory fragment to copy message data", in order
to avoid a out of memory

Author: bettio

CHANGELOG.md

@@ -35,6 +35,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 with nodejs and emscripten)
 - Added preliminary support for ESP32P4 (no networking support yet).
 - Added memory info in `out_of_memory` crash logs to help developers fix memory issues.
+- Added documentation and function specs for uart driver
+- Added `uart:read/2` with a timeout parameter.
 
 ### Fixed
 
@@ -73,6 +75,14 @@ bug when handling errors from BIFs used as NIFs (when called with `CALL_EXT` and
 - Fixed call to funs such as fun erlang:'not'/1, that make use of BIFs
 - Fixed potential crashes or memory leaks caused by a mistake in calculation of reference counts
 and a race condition in otp_socket code
+- Fixed an out of memory issue by forcing GC to copy data from message fragments
+- Fixed a bug where calling repeatedly `process_info` on a stopped process could cause an out of
+memory error
+- Fixed possible concurrency problems in ESP32 UART driver
+
+### Changed
+
+- ESP32 UART driver no longer aborts because of badargs in configuration, instead raising an error
 
 ## [0.6.5] - 2024-10-15
 

libs/eavmlib/src/uart.erl

@@ -19,26 +19,143 @@
 %
 
 -module(uart).
--export([open/1, open/2, close/1, read/1, write/2]).
+-export([open/1, open/2, close/1, read/1, read/2, write/2]).
 
+-type peripheral() :: string() | binary().
+% The peripheral `Name' may be one of: `"UART0"' | `"UART1"' | `"UART2"' | `<<"UART0">>' | `<<"UART1">>' | `<<"UART2">>'.
+
+-type uart_opts() :: [
+    {tx, Tx_pin :: integer()}
+    | {rx, Rx_pin :: integer()}
+    | {rts, Rts_pin :: integer()}
+    | {cts, Cts_pin :: integer()}
+    | {speed, Speed :: pos_integer()}
+    | {data_bits, 5..8}
+    | {stop_bits, 1 | 2}
+    | {event_queue_len, Qlen :: pos_integer()}
+    | {flow_control, none | hardware | software}
+    | {parity, none | even | odd}
+    | {peripheral, peripheral()}
+    | []
+].
+
+%%-----------------------------------------------------------------------------
+%% @param   Name the uart peripheral to be opened
+%% @param   Opts uart configuration options
+%% @returns Pid of the driver.
+%% @doc     Open a connection to the UART driver
+%%
+%%          This function will open a connection to the UART driver.
+%% @end
+%%-----------------------------------------------------------------------------
+-spec open(Name :: peripheral(), Opts :: uart_opts()) -> Pid :: pid() | {error, _Reason :: term()}.
 open(Name, Opts) ->
     open([{peripheral, Name} | Opts]).
 
+%%-----------------------------------------------------------------------------
+%% @param   Opts uart configuration options
+%% @returns Pid of the driver.
+%% @doc     Open a connection to the UART driver default port
+%%
+%%          This function will open a connection to the UART driver.
+%% @end
+%%-----------------------------------------------------------------------------
+-spec open(Opts :: uart_opts()) -> Pid :: pid() | {error, _Reason :: term()}.
 open(Opts) ->
     open_port({spawn, "uart"}, migrate_config(Opts)).
 
-close(Pid) ->
+%%-----------------------------------------------------------------------------
+%% @param   Pid of the uart port to be closed
+%% @returns ok.
+%% @doc     Close a port connection to the UART driver
+%%
+%%          This function will close the given port connection to the UART driver.
+%% @end
+%%-----------------------------------------------------------------------------
+-spec close(Pid :: pid()) -> ok | {error, _Reason :: term()}.
+close(Pid) when is_pid(Pid) ->
     port:call(Pid, close).
 
-read(Pid) ->
+%%-----------------------------------------------------------------------------
+%% @param   Pid of the uart port to be read
+%% @returns {ok, Data} or {error, Reason}
+%% @doc     Read data from a UART port
+%%
+%%          This function will return any data that is available, or return
+%%          a `{error, timeout}' tuple. The driver will sent the next available
+%%          data from the UART driver to the process that made the last read.
+%%          Example:
+%%          ```
+%%          Data = case uart:read(Uart) of
+%%              {ok, Binary} -> Binary;
+%%              {error, timeout} ->
+%%                  receive
+%%                      {ok, RecvBinary} -> RecvBinary;
+%%                      Error -> error(Error)
+%%                  end;
+%%              Error -> error(Error)
+%%          end,
+%%          '''
+%%          Any attempt by another (or the same process) to read from uart before the
+%%          next uart payload is sent by the driver will result in `{error, ealready}'.
+%% @end
+%%-----------------------------------------------------------------------------
+-spec read(Pid :: pid()) -> {ok, Data :: iodata()} | {error, _Reason :: term()}.
+read(Pid) when is_pid(Pid) ->
     port:call(Pid, read).
 
-write(Pid, B) ->
-    case is_iolist(B) of
+%%-----------------------------------------------------------------------------
+%% @param   Pid of the uart port to be read
+%% @param   Timeout millisecond to wait for data to become available
+%% @returns `{ok, Data}', or `{error, Reason}'
+%% @doc     Read data from a UART port
+%%
+%%          This function will return any data that is available within the
+%%          timeout period to the process. After the timeout has expired a new
+%%          read command may be used regardless of whether the last read was
+%%          sent a payload.
+%%          Example:
+%%          ```
+%%          Data = case uart:read(Uart, 3000) of
+%%              {ok, Bin} -> Bin;
+%%              {error, timeout} -> <<"">>;
+%%              Error -> error_handler_fun(Uart, Error)
+%%          end,
+%%          '''
+%%          Any data sent to the esp32 over uart between reads with a timeout will
+%%          be lost, so be sure this is what you want. Most applications will want
+%%          a single process to read from UART and continue to listen until a payload
+%%          is received, and likely pass the payload off for processing and
+%%          immediately begin another read.
+%% @end
+%%-----------------------------------------------------------------------------
+-spec read(Pid :: pid(), Timeout :: pos_integer()) ->
+    {ok, Data :: iodata()} | {error, _Reason :: term()}.
+read(Pid, Timeout) when is_pid(Pid) ->
+    case port:call(Pid, read, Timeout) of
+        {error, timeout} ->
+            port:call(Pid, cancel_read),
+            {error, timeout};
+        Result ->
+            Result
+    end.
+
+%%-----------------------------------------------------------------------------
+%% @param   Pid of the uart port to be written to
+%% @param   Data to be written to the given uart port
+%% @returns ok or {error, Reason}
+%% @doc     Write data to a UART port
+%%
+%%          This function will write the given data to the UART port.
+%% @end
+%%-----------------------------------------------------------------------------
+-spec write(Pid :: pid(), Data :: iodata()) -> ok | {error, _Reason :: term()}.
+write(Pid, Data) ->
+    case is_iolist(Data) andalso is_pid(Pid) of
         true ->
-            port:call(Pid, {write, B});
+            port:call(Pid, {write, Data});
         false ->
-            throw(badarg)
+            error(badarg)
     end.
 
 %% @private

src/libAtomVM/context.c

@@ -23,6 +23,7 @@
 #include <fenv.h>
 #include <math.h>
 
+#include "defaultatoms.h"
 #include "dictionary.h"
 #include "erl_nif.h"
 #include "erl_nif_priv.h"
@@ -196,11 +197,22 @@ void context_process_process_info_request_signal(Context *ctx, struct BuiltInAto
 {
     Context *target = globalcontext_get_process_lock(ctx->global, signal->sender_pid);
     if (target) {
-        term ret;
-        if (context_get_process_info(ctx, &ret, signal->atom)) {
-            mailbox_send_term_signal(target, TrapAnswerSignal, ret);
+        size_t term_size;
+        if (context_get_process_info(ctx, NULL, &term_size, signal->atom, NULL)) {
+            Heap heap;
+            if (UNLIKELY(memory_init_heap(&heap, term_size) != MEMORY_GC_OK)) {
+                mailbox_send_built_in_atom_signal(target, TrapExceptionSignal, OUT_OF_MEMORY_ATOM);
+            } else {
+                term ret;
+                if (context_get_process_info(ctx, &ret, NULL, signal->atom, &heap)) {
+                    mailbox_send_term_signal(target, TrapAnswerSignal, ret);
+                } else {
+                    mailbox_send_built_in_atom_signal(target, TrapExceptionSignal, ret);
+                }
+                memory_destroy_heap(&heap, ctx->global);
+            }
         } else {
-            mailbox_send_built_in_atom_signal(target, TrapExceptionSignal, ret);
+            mailbox_send_built_in_atom_signal(target, TrapExceptionSignal, BADARG_ATOM);
         }
         globalcontext_get_process_unlock(ctx->global, target);
     } // else: sender died
@@ -264,7 +276,7 @@ size_t context_size(Context *ctx)
         + memory_heap_memory_size(&ctx->heap) * BYTES_PER_TERM;
 }
 
-bool context_get_process_info(Context *ctx, term *out, term atom_key)
+bool context_get_process_info(Context *ctx, term *out, size_t *term_size, term atom_key, Heap *heap)
 {
     size_t ret_size;
     switch (atom_key) {
@@ -289,16 +301,19 @@ bool context_get_process_info(Context *ctx, term *out, term atom_key)
             break;
         }
         default:
-            *out = BADARG_ATOM;
+            if (out != NULL) {
+                *out = BADARG_ATOM;
+            }
             return false;
     }
-
-    if (UNLIKELY(memory_ensure_free(ctx, ret_size) != MEMORY_GC_OK)) {
-        *out = OUT_OF_MEMORY_ATOM;
-        return false;
+    if (term_size != NULL) {
+        *term_size = ret_size;
+    }
+    if (out == NULL) {
+        return true;
     }
 
-    term ret = term_alloc_tuple(2, &ctx->heap);
+    term ret = term_alloc_tuple(2, heap);
     switch (atom_key) {
         // heap_size size in words of the heap of the process
         case HEAP_SIZE_ATOM: {
@@ -361,7 +376,7 @@ bool context_get_process_info(Context *ctx, term *out, term atom_key)
                 struct Monitor *monitor = GET_LIST_ENTRY(item, struct Monitor, monitor_list_head);
                 // Links are struct Monitor entries with ref_ticks equal to 0
                 if (monitor->ref_ticks == 0) {
-                    list = term_list_prepend(monitor->monitor_obj, list, &ctx->heap);
+                    list = term_list_prepend(monitor->monitor_obj, list, heap);
                 }
             }
             term_put_tuple_element(ret, 1, list);

src/libAtomVM/context.h

@@ -396,12 +396,14 @@ void context_process_flush_monitor_signal(Context *ctx, uint64_t ref_ticks, bool
  * @brief Get process information.
  *
  * @param ctx the context being executed
- * @param out the answer term
+ * @param out the answer term. Can be NULL if only the size matters.
+ * @param term_size the size of the answer term, in words.
  * @param atom_key the key representing the info to get
+ * @param heap the heap to allocate the answer to
  * @return \c true if successful, \c false in case of an error in which case
- * *out is filled with an exception atom
+ * *out is filled with an exception atom if it was not NULL
  */
-bool context_get_process_info(Context *ctx, term *out, term atom_key);
+bool context_get_process_info(Context *ctx, term *out, size_t *term_size, term atom_key, Heap *heap);
 
 /**
  * @brief Half-link process to another process

src/libAtomVM/memory.c

@@ -159,7 +159,7 @@ enum MemoryGCResult memory_ensure_free_with_roots(Context *c, size_t size, size_
         // Target heap size depends on:
         // - alloc_mode (MEMORY_FORCE_SHRINK takes precedence)
         // - heap growth strategy
-        bool should_gc = free_space < size || (alloc_mode == MEMORY_FORCE_SHRINK);
+        bool should_gc = free_space < size || (alloc_mode == MEMORY_FORCE_SHRINK) || c->heap.root->next != NULL;
         size_t memory_size = 0;
         if (!should_gc) {
             switch (c->heap_growth_strategy) {

src/libAtomVM/nifs.c

@@ -2738,7 +2738,16 @@ static term nif_erlang_process_info(Context *ctx, int argc, term argv[])
 
     term ret = term_invalid_term();
     if (ctx == target) {
-        if (!context_get_process_info(ctx, &ret, item)) {
+        size_t term_size;
+        if (UNLIKELY(!context_get_process_info(ctx, NULL, &term_size, item, NULL))) {
+            globalcontext_get_process_unlock(ctx->global, target);
+            RAISE_ERROR(BADARG_ATOM);
+        }
+        if (UNLIKELY(memory_ensure_free_opt(ctx, term_size, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) {
+            globalcontext_get_process_unlock(ctx->global, target);
+            RAISE_ERROR(OUT_OF_MEMORY_ATOM);
+        }
+        if (UNLIKELY(!context_get_process_info(ctx, &ret, NULL, item, &ctx->heap))) {
             globalcontext_get_process_unlock(ctx->global, target);
             RAISE_ERROR(ret);
         }