Clarify enif_make_resource usage

Also fix a couple of possible memory leaks in error paths

Signed-off-by: Paul Guyot <pguyot@kallisys.net>

Author: pguyot

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;
 }