Fix OWN_GIL safety issues: mutex leak, ABBA deadlock, dangling env

- Fix mutex leak in erlang_module_free: always destroy async_futures_mutex
  regardless of pipe_initialized flag since mutex is always initialized
- Fix ABBA deadlock in event_loop_down and event_loop_destructor: acquire
  GIL before namespaces_mutex to match normal execution path lock ordering
- Add interp_id validation in owngil_execute_*_with_env functions to detect
  env resources from wrong interpreter, preventing dangling pointer access
- Document OWN_GIL callback re-entry limitation: erlang.call() uses
  thread_worker_call rather than suspension/resume protocol

Author: benoitc

c_src/py_callback.c

@@ -1666,6 +1666,13 @@ static PyObject *erlang_call_impl(PyObject *self, PyObject *args) {
      * 2. tl_current_context with callback_handler (old blocking pipe mode)
      * 3. tl_current_worker (legacy worker API)
      * 4. thread_worker_call (spawned threads)
+     *
+     * NOTE: In OWN_GIL mode, erlang.call() goes through thread_worker_call()
+     * rather than using suspension/resume. This is because OWN_GIL contexts
+     * bypass the suspension protocol - the dedicated pthread that owns the GIL
+     * cannot be suspended. As a result, the call executes on a different
+     * context/interpreter (the thread worker), not the calling OWN_GIL context.
+     * Re-entrant calls back to the same OWN_GIL context are not supported.
      */
     bool has_context_suspension = (tl_current_context != NULL && tl_allow_suspension);
     bool has_context_handler = (tl_current_context != NULL && tl_current_context->has_callback_handler);
@@ -1678,6 +1685,7 @@ static PyObject *erlang_call_impl(PyObject *self, PyObject *args) {
          * - threading.Thread instances
          * - concurrent.futures.ThreadPoolExecutor workers
          * - Any other Python threads
+         * - OWN_GIL contexts (which don't support suspension)
          */
         Py_ssize_t nargs = PyTuple_Size(args);
         if (nargs < 1) {
@@ -2783,10 +2791,9 @@ static void erlang_module_free(void *module) {
     Py_XDECREF(state->async_pending_futures);
     state->async_pending_futures = NULL;
 
-    if (state->pipe_initialized) {
-        pthread_mutex_destroy(&state->async_futures_mutex);
-        state->pipe_initialized = false;
-    }
+    /* Always destroy mutex - it was always initialized in create_erlang_module */
+    pthread_mutex_destroy(&state->async_futures_mutex);
+    state->pipe_initialized = false;
 }
 
 /* Module definition */

c_src/py_event_loop.c

@@ -465,29 +465,51 @@ void event_loop_destructor(ErlNifEnv *env, void *obj) {
         loop->msg_env = NULL;
     }
 
-    /* Clean up per-process namespaces.
-     * Note: Same leak-vs-crash tradeoff as above. If we can't safely
-     * acquire the GIL, we skip Py_XDECREF and accept leaking the Python
-     * dict objects. The native namespace struct is always freed. */
-    pthread_mutex_lock(&loop->namespaces_mutex);
-    process_namespace_t *ns = loop->namespaces_head;
-    while (ns != NULL) {
-        process_namespace_t *next = ns->next;
-        /* Only cleanup Python objects if runtime is still running */
-        if (runtime_is_running() && loop->interp_id == 0 &&
-            PyGILState_GetThisThreadState() == NULL &&
-            !PyGILState_Check()) {
-            PyGILState_STATE gstate = PyGILState_Ensure();
+    /*
+     * Clean up per-process namespaces.
+     *
+     * Lock ordering: GIL first, then namespaces_mutex (consistent with normal path).
+     * This prevents ABBA deadlock with execution paths that acquire GIL then mutex.
+     *
+     * For subinterpreters (interp_id != 0), we can't use PyGILState_Ensure.
+     * Just free the native structs without Py_DECREF - Python objects will be
+     * cleaned up when the interpreter is destroyed.
+     */
+    if (runtime_is_running() && loop->interp_id == 0 &&
+        PyGILState_GetThisThreadState() == NULL &&
+        !PyGILState_Check()) {
+        /* Main interpreter: GIL first, then mutex */
+        PyGILState_STATE gstate = PyGILState_Ensure();
+        pthread_mutex_lock(&loop->namespaces_mutex);
+
+        process_namespace_t *ns = loop->namespaces_head;
+        while (ns != NULL) {
+            process_namespace_t *next = ns->next;
             Py_XDECREF(ns->globals);
             Py_XDECREF(ns->locals);
             Py_XDECREF(ns->module_cache);
-            PyGILState_Release(gstate);
+            enif_free(ns);
+            ns = next;
         }
-        enif_free(ns);
-        ns = next;
+        loop->namespaces_head = NULL;
+
+        pthread_mutex_unlock(&loop->namespaces_mutex);
+        PyGILState_Release(gstate);
+    } else {
+        /* Subinterpreter or runtime not running: just free structs */
+        pthread_mutex_lock(&loop->namespaces_mutex);
+
+        process_namespace_t *ns = loop->namespaces_head;
+        while (ns != NULL) {
+            process_namespace_t *next = ns->next;
+            /* Skip Py_XDECREF - can't safely acquire GIL */
+            enif_free(ns);
+            ns = next;
+        }
+        loop->namespaces_head = NULL;
+
+        pthread_mutex_unlock(&loop->namespaces_mutex);
     }
-    loop->namespaces_head = NULL;
-    pthread_mutex_unlock(&loop->namespaces_mutex);
     pthread_mutex_destroy(&loop->namespaces_mutex);
 
     /* Destroy synchronization primitives */
@@ -616,13 +638,45 @@ void timer_resource_destructor(ErlNifEnv *env, void *obj) {
  * @brief Down callback for event loop resources (process monitor)
  *
  * Called when a monitored process dies. Cleans up the process's namespace.
+ *
+ * Lock ordering: GIL first, then namespaces_mutex (consistent with normal path)
  */
 void event_loop_down(ErlNifEnv *env, void *obj, ErlNifPid *pid,
                      ErlNifMonitor *mon) {
     (void)env;
     (void)mon;
     erlang_event_loop_t *loop = (erlang_event_loop_t *)obj;
 
+    /*
+     * For subinterpreters (interp_id != 0), we can't use PyGILState_Ensure.
+     * Just remove from the list without Py_DECREF - the Python objects will
+     * be cleaned up when the interpreter is destroyed.
+     */
+    if (!runtime_is_running() || loop->interp_id != 0) {
+        pthread_mutex_lock(&loop->namespaces_mutex);
+
+        process_namespace_t **pp = &loop->namespaces_head;
+        while (*pp != NULL) {
+            if (enif_compare_pids(&(*pp)->owner_pid, pid) == 0) {
+                process_namespace_t *to_free = *pp;
+                *pp = to_free->next;
+                /* Skip Py_XDECREF - can't safely acquire GIL for subinterp */
+                enif_free(to_free);
+                break;
+            }
+            pp = &(*pp)->next;
+        }
+
+        pthread_mutex_unlock(&loop->namespaces_mutex);
+        return;
+    }
+
+    /*
+     * For main interpreter: acquire GIL FIRST to maintain consistent lock
+     * ordering with the normal execution path (which acquires GIL, then mutex).
+     * This prevents ABBA deadlock.
+     */
+    PyGILState_STATE gstate = PyGILState_Ensure();
     pthread_mutex_lock(&loop->namespaces_mutex);
 
     /* Find and remove namespace for this pid */
@@ -632,14 +686,9 @@ void event_loop_down(ErlNifEnv *env, void *obj, ErlNifPid *pid,
             process_namespace_t *to_free = *pp;
             *pp = to_free->next;
 
-            /* Must hold GIL to free Python objects */
-            if (runtime_is_running() && loop->interp_id == 0) {
-                PyGILState_STATE gstate = PyGILState_Ensure();
-                Py_XDECREF(to_free->globals);
-                Py_XDECREF(to_free->locals);
-                Py_XDECREF(to_free->module_cache);
-                PyGILState_Release(gstate);
-            }
+            Py_XDECREF(to_free->globals);
+            Py_XDECREF(to_free->locals);
+            Py_XDECREF(to_free->module_cache);
 
             enif_free(to_free);
             break;
@@ -648,6 +697,7 @@ void event_loop_down(ErlNifEnv *env, void *obj, ErlNifPid *pid,
     }
 
     pthread_mutex_unlock(&loop->namespaces_mutex);
+    PyGILState_Release(gstate);
 }
 
 /**

c_src/py_nif.c

@@ -2775,6 +2775,17 @@ static void owngil_execute_exec_with_env(py_context_t *ctx) {
         return;
     }
 
+    /* Verify interpreter ownership - prevent dangling pointer access.
+     * Compare env's interp_id with the current Python interpreter's ID. */
+    PyInterpreterState *current_interp = PyInterpreterState_Get();
+    if (current_interp != NULL && penv->interp_id != PyInterpreterState_GetID(current_interp)) {
+        ctx->response_term = enif_make_tuple2(ctx->shared_env,
+            enif_make_atom(ctx->shared_env, "error"),
+            enif_make_atom(ctx->shared_env, "env_wrong_interpreter"));
+        ctx->response_ok = false;
+        return;
+    }
+
     ErlNifBinary code_bin;
     if (!enif_inspect_binary(ctx->shared_env, ctx->request_term, &code_bin)) {
         ctx->response_term = enif_make_tuple2(ctx->shared_env,
@@ -2841,6 +2852,17 @@ static void owngil_execute_eval_with_env(py_context_t *ctx) {
         return;
     }
 
+    /* Verify interpreter ownership - prevent dangling pointer access.
+     * Compare env's interp_id with the current Python interpreter's ID. */
+    PyInterpreterState *current_interp = PyInterpreterState_Get();
+    if (current_interp != NULL && penv->interp_id != PyInterpreterState_GetID(current_interp)) {
+        ctx->response_term = enif_make_tuple2(ctx->shared_env,
+            enif_make_atom(ctx->shared_env, "error"),
+            enif_make_atom(ctx->shared_env, "env_wrong_interpreter"));
+        ctx->response_ok = false;
+        return;
+    }
+
     /* Decode request: {Code, Locals} */
     const ERL_NIF_TERM *tuple_terms;
     int tuple_arity;
@@ -2933,6 +2955,17 @@ static void owngil_execute_call_with_env(py_context_t *ctx) {
         return;
     }
 
+    /* Verify interpreter ownership - prevent dangling pointer access.
+     * Compare env's interp_id with the current Python interpreter's ID. */
+    PyInterpreterState *current_interp = PyInterpreterState_Get();
+    if (current_interp != NULL && penv->interp_id != PyInterpreterState_GetID(current_interp)) {
+        ctx->response_term = enif_make_tuple2(ctx->shared_env,
+            enif_make_atom(ctx->shared_env, "error"),
+            enif_make_atom(ctx->shared_env, "env_wrong_interpreter"));
+        ctx->response_ok = false;
+        return;
+    }
+
     /* Decode request from shared_env: {Module, Func, Args, Kwargs} */
     ERL_NIF_TERM module_term, func_term, args_term, kwargs_term;
     const ERL_NIF_TERM *tuple_terms;