Merge pull request atomvm#1617 from pguyot/w14/clarify-enif_make_resource-usage

Clarify enif_make_resource usage

Also fix a couple of possible memory leaks in error paths

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

Author: bettio

src/libAtomVM/erl_nif.h

@@ -150,6 +150,17 @@ ErlNifResourceType *enif_init_resource_type(ErlNifEnv *env, const char *name, co
 
 /**
  * @brief Allocate a resource for given type for `size` bytes.
+ * @details following BEAM semantics, the resource is created with a refcount
+ * of 1. A call to `enif_release_resource` will decrement the refcount and
+ * destroy the resource if it is zero.
+ *
+ * Typical usage (as suggested by BEAM documentation) is to call
+ * `enif_make_resource` and then `enif_release_resource` to somewhat transfer
+ * ownership to the garbage collector. `enif_make_resource` will increment
+ * refcount to 2 and also add the resource to the MSO list of the context, so
+ * when the term is no longer referenced in the context heap, the reference
+ * counter will be decremented by gc.
+ *
  * @param type a trype created by `enif_init_resource_type`.
  * @param size the size in bytes.
  * @return a pointer or `NULL` on failure.
@@ -184,9 +195,18 @@ int enif_release_resource(void *resource);
 /**
  * @brief create a term from a resource
  * @details the term can be later passed to `enif_get_resource`.
- * The resource is typically released (by calling `enif_release_resource`)
- * just after calling this function to "transfer ownership" to Erlang code so
- * that it will be destroyed when garbage collected.
+ *
+ * The resource reference counter is incremented and it is added to the MSO
+ * list of the heap of env (which must be a context).
+ *
+ * If the resource was just allocated with `enif_alloc_resource`, the reference
+ * counter should typically be decremented by a call to `enif_release_resource`
+ * matching usage documented by BEAM.
+ *
+ * If the resource was not just allocated with `enif_alloc_resource`, to clear
+ * usage confusion, users should rather call `term_from_resource` and should
+ * not decrement the reference counter.
+ *
  * @param env current environment
  * @param obj resource
  * @return a new term representing the resource

src/libAtomVM/otp_socket.c

@@ -529,6 +529,7 @@ static term nif_socket_open(Context *ctx, int argc, term argv[])
 #ifndef AVM_NO_SMP
     rsrc_obj->socket_lock = smp_rwlock_create();
     if (IS_NULL_PTR(rsrc_obj->socket_lock)) {
+        // destroy resource object without calling dtor
         free(rsrc_obj);
         RAISE_ERROR(OUT_OF_MEMORY_ATOM);
     }
@@ -537,6 +538,7 @@ static term nif_socket_open(Context *ctx, int argc, term argv[])
     rsrc_obj->fd = socket(domain, type, protocol);
     if (UNLIKELY(rsrc_obj->fd == -1 || rsrc_obj->fd == CLOSED_FD)) {
         AVM_LOGE(TAG, "Failed to initialize socket.");
+        rsrc_obj->fd = CLOSED_FD;
         enif_release_resource(rsrc_obj);
         return make_errno_tuple(ctx);
     } else {
@@ -555,6 +557,7 @@ static term nif_socket_open(Context *ctx, int argc, term argv[])
         LWIP_END();
         rsrc_obj->socket_state = SocketStateUDPIdle;
     } else {
+        rsrc_obj->socket_state = SocketStateClosed;
         enif_release_resource(rsrc_obj);
         RAISE_ERROR(BADARG_ATOM);
     }
@@ -586,7 +589,7 @@ static term nif_socket_open(Context *ctx, int argc, term argv[])
             RAISE_ERROR(OUT_OF_MEMORY_ATOM);
         }
         term obj = enif_make_resource(erl_nif_env_from_context(ctx), rsrc_obj);
-        enif_release_resource(rsrc_obj);
+        enif_release_resource(rsrc_obj); // decrement refcount after enif_alloc_resource
 
         size_t requested_size = TUPLE_SIZE(2) + TUPLE_SIZE(2) + REF_SIZE;
         if (UNLIKELY(memory_ensure_free_with_roots(ctx, requested_size, 1, &obj, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) {
@@ -788,6 +791,7 @@ static struct SocketResource *make_accepted_socket_resource(struct tcp_pcb *newp
 #ifndef AVM_NO_SMP
     conn_rsrc_obj->socket_lock = smp_rwlock_create();
     if (IS_NULL_PTR(conn_rsrc_obj->socket_lock)) {
+        // destroy resource without calling destructor
         free(conn_rsrc_obj);
         return NULL;
     }
@@ -1563,7 +1567,7 @@ static term nif_socket_listen(Context *ctx, int argc, term argv[])
 static term make_accepted_socket_term(Context *ctx, struct SocketResource *conn_rsrc_obj)
 {
     term obj = enif_make_resource(erl_nif_env_from_context(ctx), conn_rsrc_obj);
-    enif_release_resource(conn_rsrc_obj);
+    enif_release_resource(conn_rsrc_obj); // decrement refcount after enif_allocate_resource in make_accepted_socket_resource
 
     term socket_term = term_alloc_tuple(2, &ctx->heap);
     uint64_t ref_ticks = globalcontext_get_ref_ticks(ctx->global);
@@ -1636,7 +1640,7 @@ static term nif_socket_accept(Context *ctx, int argc, term argv[])
         TRACE("nif_socket_accept: Created socket on accept fd=%i\n", rsrc_obj->fd);
 
         term new_resource = enif_make_resource(erl_nif_env_from_context(ctx), conn_rsrc_obj);
-        enif_release_resource(conn_rsrc_obj);
+        enif_release_resource(conn_rsrc_obj); // decrement refcount after enif_alloc_resource
 
         size_t requested_size = TUPLE_SIZE(2) + TUPLE_SIZE(2) + REF_SIZE;
         if (UNLIKELY(memory_ensure_free_with_roots(ctx, requested_size, 1, &new_resource, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) {
@@ -1673,6 +1677,7 @@ static term nif_socket_accept(Context *ctx, int argc, term argv[])
             AVM_LOGW(TAG, "Failed to allocate memory: %s:%i.", __FILE__, __LINE__);
             LWIP_END();
             SMP_RWLOCK_UNLOCK(rsrc_obj->socket_lock);
+            enif_release_resource(new_resource);
             RAISE_ERROR(OUT_OF_MEMORY_ATOM);
         }
 

src/libAtomVM/otp_ssl.c

@@ -236,7 +236,7 @@ static term nif_ssl_entropy_init(Context *ctx, int argc, term argv[])
         RAISE_ERROR(OUT_OF_MEMORY_ATOM);
     }
     term obj = enif_make_resource(erl_nif_env_from_context(ctx), rsrc_obj);
-    enif_release_resource(rsrc_obj);
+    enif_release_resource(rsrc_obj); // decrement refcount after enif_alloc_resource
 
     mbedtls_entropy_init(&rsrc_obj->context);
 
@@ -259,7 +259,7 @@ static term nif_ssl_ctr_drbg_init(Context *ctx, int argc, term argv[])
         RAISE_ERROR(OUT_OF_MEMORY_ATOM);
     }
     term obj = enif_make_resource(erl_nif_env_from_context(ctx), rsrc_obj);
-    enif_release_resource(rsrc_obj);
+    enif_release_resource(rsrc_obj); // decrement refcount after enif_alloc_resource
 
     mbedtls_ctr_drbg_init(&rsrc_obj->context);
 
@@ -311,7 +311,7 @@ static term nif_ssl_init(Context *ctx, int argc, term argv[])
         RAISE_ERROR(OUT_OF_MEMORY_ATOM);
     }
     term obj = enif_make_resource(erl_nif_env_from_context(ctx), rsrc_obj);
-    enif_release_resource(rsrc_obj);
+    enif_release_resource(rsrc_obj); // decrement refcount after enif_alloc_resource
 
     mbedtls_ssl_init(&rsrc_obj->context);
 
@@ -364,7 +364,7 @@ static term nif_ssl_config_init(Context *ctx, int argc, term argv[])
         RAISE_ERROR(OUT_OF_MEMORY_ATOM);
     }
     term obj = enif_make_resource(erl_nif_env_from_context(ctx), rsrc_obj);
-    enif_release_resource(rsrc_obj);
+    enif_release_resource(rsrc_obj); // decrement refcount after enif_alloc_resource
 
     mbedtls_ssl_config_init(&rsrc_obj->config);
 

src/libAtomVM/posix_nifs.c

@@ -315,7 +315,7 @@ static term nif_atomvm_posix_open(Context *ctx, int argc, term argv[])
             RAISE_ERROR(OUT_OF_MEMORY_ATOM);
         }
         term obj = enif_make_resource(erl_nif_env_from_context(ctx), fd_obj);
-        enif_release_resource(fd_obj);
+        enif_release_resource(fd_obj); // decrement refcount after enif_alloc_resource
         result = term_alloc_tuple(2, &ctx->heap);
         term_put_tuple_element(result, 0, OK_ATOM);
         term_put_tuple_element(result, 1, obj);
@@ -680,7 +680,7 @@ static term nif_atomvm_posix_opendir(Context *ctx, int argc, term argv[])
             RAISE_ERROR(OUT_OF_MEMORY_ATOM);
         }
         term obj = enif_make_resource(erl_nif_env_from_context(ctx), dir_obj);
-        enif_release_resource(dir_obj);
+        enif_release_resource(dir_obj); // decrement refcount after enif_alloc_resource
         result = term_alloc_tuple(2, &ctx->heap);
         term_put_tuple_element(result, 0, OK_ATOM);
         term_put_tuple_element(result, 1, obj);

src/platforms/emscripten/src/lib/platform_nifs.c

@@ -677,6 +677,7 @@ static EM_BOOL html5api_touch_callback(int eventType, const EmscriptenTouchEvent
         resource->event = event_constant;                                                                                                                                                \
         EMSCRIPTEN_RESULT result = emscripten_set_##callback##_callback_on_thread(target, resource, use_capture, html5api_##event_type##_callback, emscripten_main_runtime_thread_id()); \
         if (result != EMSCRIPTEN_RESULT_SUCCESS && result != EMSCRIPTEN_RESULT_DEFERRED) {                                                                                               \
+            enif_release_resource(resource);                                                                                                                                             \
             return term_from_emscripten_result(result, ctx);                                                                                                                             \
         }                                                                                                                                                                                \
         term resource_term = enif_make_resource(erl_nif_env_from_context(ctx), resource);                                                                                                \

src/platforms/esp32/components/avm_builtins/adc_driver.c

@@ -349,7 +349,7 @@ static term nif_adc_init(Context *ctx, int argc, term argv[])
         RAISE_ERROR(OUT_OF_MEMORY_ATOM);
     }
     ERL_NIF_TERM unit_obj = enif_make_resource(erl_nif_env_from_context(ctx), unit_rsrc);
-    enif_release_resource(unit_rsrc);
+    enif_release_resource(unit_rsrc); // decrement refcount after enif_alloc_resource
 
     // {ok, {'$adc', Unit :: resource(), ref()}}
     size_t requested_size = TUPLE_SIZE(2) + TUPLE_SIZE(3) + REF_SIZE;
@@ -501,7 +501,7 @@ static term nif_adc_acquire(Context *ctx, int argc, term argv[])
     }
 
     term chan_obj = enif_make_resource(erl_nif_env_from_context(ctx), chan_rsrc);
-    enif_release_resource(chan_rsrc);
+    enif_release_resource(chan_rsrc); // decrement refcount after enif_alloc_resource
 
     // {ok, {'$adc', resource(), ref()}}
     size_t requested_size = TUPLE_SIZE(2) + TUPLE_SIZE(3) + REF_SIZE;

src/platforms/esp32/components/avm_builtins/storage_nif.c

@@ -270,7 +270,7 @@ static term nif_esp_mount(Context *ctx, int argc, term argv[])
         term_put_tuple_element(return_term, 0, OK_ATOM);
         term_put_tuple_element(return_term, 1, mount_term);
     }
-    enif_release_resource(mount);
+    enif_release_resource(mount); // decrement refcount after either enif_alloc_resource
 
     return return_term;
 }