Forward port changes from v0.6 release branch

Merge fixe for resources concurrency bug (#1603) and fixes for memory
corruptions (#1600 and #1601) from release-0.6 branch.

Author: bettio

CHANGELOG.md

@@ -91,6 +91,8 @@ memory error
 - Fixed memory corruption issue with `erlang:make_tuple/2`
 - Fix potential use after free with code generated from OTP <= 24
 - Fix `is_function/2` guard
+- Fixed segfault when calling `lists:reverse/1` (#1600)
+- Fixed nif_atomvm_posix_read GC bug
 
 ### Changed
 

src/libAtomVM/context.c

@@ -180,8 +180,7 @@ void context_destroy(Context *ctx)
             struct Monitor *monitor = GET_LIST_ENTRY(item, struct Monitor, monitor_list_head);
             void *resource = term_to_term_ptr(monitor->monitor_obj);
             struct RefcBinary *refc = refc_binary_from_data(resource);
-            resource_type_demonitor(refc->resource_type, monitor->ref_ticks);
-            refc->resource_type->down(&env, resource, &ctx->process_id, &monitor->ref_ticks);
+            resource_type_fire_monitor(refc->resource_type, &env, resource, ctx->process_id, monitor->ref_ticks);
             free(monitor);
         }
     }

src/libAtomVM/nifs.c

@@ -4935,7 +4935,7 @@ static term nif_lists_reverse(Context *ctx, int argc, term argv[])
         RAISE_ERROR(BADARG_ATOM);
     }
 
-    if (UNLIKELY(memory_ensure_free_with_roots(ctx, len * CONS_SIZE, 2, argv, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) {
+    if (UNLIKELY(memory_ensure_free_with_roots(ctx, len * CONS_SIZE, argc, argv, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) {
         RAISE_ERROR(OUT_OF_MEMORY_ATOM);
     }
 

src/libAtomVM/posix_nifs.c

@@ -362,14 +362,17 @@ static term nif_atomvm_posix_read(Context *ctx, int argc, term argv[])
     VALIDATE_VALUE(count_term, term_is_integer);
     int count = term_to_int(count_term);
 
+    size_t size = term_binary_data_size_in_terms(count) + BINARY_HEADER_SIZE + TERM_BOXED_SUB_BINARY_SIZE + TUPLE_SIZE(2);
+    if (UNLIKELY(memory_ensure_free_with_roots(ctx, size, argc, argv, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) {
+        RAISE_ERROR(OUT_OF_MEMORY_ATOM);
+    }
+
     void *fd_obj_ptr;
+
     if (UNLIKELY(!enif_get_resource(erl_nif_env_from_context(ctx), argv[0], glb->posix_fd_resource_type, &fd_obj_ptr))) {
         RAISE_ERROR(BADARG_ATOM);
     }
     struct PosixFd *fd_obj = (struct PosixFd *) fd_obj_ptr;
-    if (UNLIKELY(memory_ensure_free_opt(ctx, term_binary_data_size_in_terms(count) + BINARY_HEADER_SIZE + TERM_BOXED_SUB_BINARY_SIZE + TUPLE_SIZE(2), MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) {
-        RAISE_ERROR(OUT_OF_MEMORY_ATOM);
-    }
     term bin_term = term_create_uninitialized_binary(count, &ctx->heap, glb);
     int res = read(fd_obj->fd, (void *) term_binary_data(bin_term), count);
     if (UNLIKELY(res < 0)) {

src/libAtomVM/refc_binary.c

@@ -66,6 +66,20 @@ void refc_binary_increment_refcount(struct RefcBinary *refc)
 bool refc_binary_decrement_refcount(struct RefcBinary *refc, struct GlobalContext *global)
 {
     if (--refc->ref_count == 0) {
+        if (refc->resource_type && refc->resource_type->down) {
+            // There may be monitors associated with this resource.
+            destroy_resource_monitors(refc, global);
+            // After this point, the resource can no longer be found by
+            // resource_type_fire_monitor
+            // However, resource_type_fire_monitor may have incremented ref_count
+            // to call the monitor handler.
+            // So we check ref_count again. We're not affected by the ABA problem
+            // here as the resource cannot (should not) be monitoring while it is
+            // being destroyed, i.e. no resource monitor will be created now
+            if (UNLIKELY(refc->ref_count != 0)) {
+                return false;
+            }
+        }
         synclist_remove(&global->refc_binaries, &refc->head);
         refc_binary_destroy(refc, global);
         return true;
@@ -78,10 +92,6 @@ void refc_binary_destroy(struct RefcBinary *refc, struct GlobalContext *global)
     UNUSED(global);
 
     if (refc->resource_type) {
-        if (refc->resource_type->down) {
-            // There may be monitors associated with this resource.
-            destroy_resource_monitors(refc, global);
-        }
         if (refc->resource_type->dtor) {
             ErlNifEnv env;
             erl_nif_env_partial_init_from_globalcontext(&env, global);

src/libAtomVM/resources.c

@@ -360,6 +360,31 @@ int enif_monitor_process(ErlNifEnv *env, void *obj, const ErlNifPid *target_pid,
     return 0;
 }
 
+void resource_type_fire_monitor(struct ResourceType *resource_type, ErlNifEnv *env, void *resource, int32_t process_id, uint64_t ref_ticks)
+{
+    struct RefcBinary *refc = NULL;
+    struct ListHead *monitors = synclist_wrlock(&resource_type->monitors);
+    struct ListHead *item;
+    LIST_FOR_EACH (item, monitors) {
+        struct ResourceMonitor *monitor = GET_LIST_ENTRY(item, struct ResourceMonitor, resource_list_head);
+        if (monitor->ref_ticks == ref_ticks) {
+            // Resource still exists.
+            refc = refc_binary_from_data(resource);
+            refc_binary_increment_refcount(refc);
+            list_remove(&monitor->resource_list_head);
+            free(monitor);
+            break;
+        }
+    }
+
+    synclist_unlock(&resource_type->monitors);
+
+    if (refc) {
+        resource_type->down(env, resource, &process_id, &ref_ticks);
+        refc_binary_decrement_refcount(refc, env->global);
+    }
+}
+
 void resource_type_demonitor(struct ResourceType *resource_type, uint64_t ref_ticks)
 {
     struct ListHead *monitors = synclist_wrlock(&resource_type->monitors);

src/libAtomVM/resources.h

@@ -149,6 +149,19 @@ void destroy_resource_monitors(struct RefcBinary *resource, GlobalContext *globa
  */
 term select_event_make_notification(void *rsrc_obj, uint64_t ref_ticks, bool is_write, Heap *heap);
 
+/**
+ * @brief Call down handler for a given resource and remove monitor from list.
+ * @details handler is called while holding lock on the list of monitors and
+ * if monitor is still in the list of resource monitors, thus ensuring that
+ * the resource still exists.
+ * @param resource_type type holding the list of monitors
+ * @param env environment for calling the down handler
+ * @param resource resource that monitored the process
+ * @param process_id id of the process monitored
+ * @param ref_ticks reference of the monitor
+ */
+void resource_type_fire_monitor(struct ResourceType *resource_type, ErlNifEnv *env, void *resource, int32_t process_id, uint64_t ref_ticks);
+
 /**
  * @brief Remove monitor from list of monitors.
  * @param resource_type type holding the list of monitors