From 774c31102dd8b6c81e64172bc41cdcfbd1067199 Mon Sep 17 00:00:00 2001 From: Benoit Chesneau Date: Sat, 29 Aug 2026 15:57:39 +0200 Subject: [PATCH] Document lock and ownership contracts on the shared structs The py_context_t header still described dirty-scheduler execution over a shared-GIL pool. It now says which thread owns each field group, what each mutex guards, the lock order, and why a stuck context is leaked rather than freed. The event loop struct gets the same treatment, and the Erlang modules that own a resource state in three lines what they own, who they talk to and what they never do. --- c_src/py_event_loop.h | 53 ++++++++++++++++++++--- c_src/py_nif.h | 91 +++++++++++++++++++++++++++++---------- src/py_buffer.erl | 4 ++ src/py_callback.erl | 6 +++ src/py_context.erl | 32 ++++++++------ src/py_context_router.erl | 4 ++ src/py_event_loop.erl | 7 +++ src/py_event_worker.erl | 7 +++ src/py_isolated.erl | 6 +++ src/py_shm.erl | 8 ++++ src/py_thread_handler.erl | 7 +++ 11 files changed, 183 insertions(+), 42 deletions(-) diff --git a/c_src/py_event_loop.h b/c_src/py_event_loop.h index e9e9413..d8adb75 100644 --- a/c_src/py_event_loop.h +++ b/c_src/py_event_loop.h @@ -259,13 +259,52 @@ typedef struct { /** * @struct erlang_event_loop_t - * @brief Main state for the Erlang-backed asyncio event loop - * - * This structure maintains all state needed for the event loop: - * - Reference to the Erlang worker process (scalable I/O model) - * - Reference to the Erlang router process (legacy) - * - Pending events queue - * - Synchronization primitives + * @brief State of one ErlangEventLoop (asyncio loop backed by enif_select) + * + * Three kinds of thread touch a loop: the loop thread (the context thread + * running `run_forever`, or a scheduler for main-interpreter loops driven by + * py_event_worker), scheduler threads running NIFs (`submit_task`, + * readiness and timer callbacks from py_event_worker), and the interpreter + * thread tearing the loop down. + * + * Lock and ownership contract: + * + * - mutex guards the pending event queue (pending_head/tail, + * pending_capacity, event_freelist, freelist_count, the pending_hash_* + * set), interp and external_attached, and event_cond. Never acquire a + * GIL while holding it: the loop thread can hold the GIL while waiting + * for mutex (loop_gil_acquire attaches under mutex, then takes the GIL + * after releasing it). + * + * - task_queue_mutex guards task_queue (an ErlNifIOQueue of serialized + * task tuples). Producers are scheduler threads in the submit NIFs; the + * consumer is process_ready_tasks on the loop thread, which holds the + * GIL while decoding. task_count and task_wake_pending are atomics used + * to coalesce wakeups. + * + * - env_pool_mutex guards env_pool and env_pool_count only. + * + * - namespaces_mutex guards namespaces_head and pid_env_head. Lock order: + * GIL first, then namespaces_mutex; the Python dicts in a namespace are + * touched only under the GIL. + * + * - py_loop, cached_* and callable_cache are Python objects owned by the + * loop thread and used only under its GIL. msg_env is allocated with the + * loop and freed in the destructor; the notification paths do not use + * it (each builds a local env per message so they need no lock). + * + * - worker_pid/has_worker, self_pid/has_self, loop_id and interp_id are + * set once at creation or by the setter NIFs before the loop runs, then + * read-only. router_pid/has_router are kept for layout compatibility + * only. shutdown is set once by the stop NIF. + * + * - interp becomes NULL in event_loop_detach_interpreter, which then waits + * for external_attached to drop to zero before Py_EndInterpreter; a + * scheduler that finds interp NULL must not attach. + * + * @see loop_gil_acquire + * @see process_ready_tasks + * @see event_loop_detach_interpreter */ typedef struct erlang_event_loop { /** @brief Legacy field - kept for binary compatibility */ diff --git a/c_src/py_nif.h b/c_src/py_nif.h index 7dd3b18..3b1fcf5 100644 --- a/c_src/py_nif.h +++ b/c_src/py_nif.h @@ -642,24 +642,72 @@ typedef struct { /** * @struct py_context_t - * @brief Process-owned Python context with shared-GIL subinterpreter pool - * - * A py_context_t is owned by a single Erlang process, which serializes - * all access to it. For subinterpreters, contexts reference a slot in - * the pre-created subinterpreter pool (shared GIL model). - * - * Execution happens directly on dirty schedulers using PyThreadState_Swap() - * to switch to the subinterpreter's thread state. This avoids: - * - Dedicated pthread per context - * - Mutex/condvar dispatch overhead - * - Term copying between environments - * - * @note Python 3.12+ uses shared-GIL subinterpreters via pool slots - * @note Older Python uses worker mode with main interpreter namespace + * @brief One Python execution environment served by one Erlang process + * + * A context has exactly one pthread that runs Python for it: the context + * thread (worker_context_thread_main for worker mode, + * owngil_context_thread_main for owngil mode). Erlang processes never run + * Python on a context; NIFs enqueue a ctx_request_t and return, the + * context thread dequeues, executes and replies with `{py_result, Id, R}` + * through msg_env. Isolated mode does not use this struct at all. + * + * Lock and ownership contract, by field group: + * + * - Identity and lifecycle (interp_id, is_subinterp, uses_worker_thread, + * uses_own_gil): written once by nif_context_create before the thread + * starts, read-only afterwards. destroyed, leaked, worker_running, + * shutdown_requested and init_error are atomics; any thread may read + * them, the writers are nif_context_destroy (destroyed, leaked), the + * shutdown helpers (shutdown_requested) and the context thread + * (worker_running, init_error). + * + * - Callback handler (has_callback_handler, callback_handler, + * callback_pipe): set by the owning Erlang process through + * nif_context_set_callback_handler before the first request; read by the + * context thread inside erlang_call_impl. The pipe is closed by + * nif_context_destroy only when the thread joined (`!leaked`): a stuck + * thread still reads the fds, and closing them would let the kernel hand + * the numbers to another file. + * + * - Request queue (queue_head, queue_tail, queue_not_empty): guarded by + * queue_mutex. Producers are NIFs on scheduler threads, the consumer is + * the context thread. Lock order: queue_mutex before req->mutex + * (ctx_queue_cancel_all takes both in that order); nothing takes + * queue_mutex while holding req->mutex, the GIL or interrupt_mutex. + * + * - msg_env: used only by the context thread, one message at a time + * (enif_clear_env, build, enif_send). Freed by the shutdown helper after + * the thread joined; never freed on the leak path. + * + * - Current request (shared_env, request_type, request_term, + * response_term, response_ok, reactor_buffer_ptr, local_env_ptr): a + * mirror of the ctx_request_t being executed, written and read by the + * context thread only, cleared after each request. No lock: no other + * thread may touch them. The execute functions take the context rather + * than the request, which is why the mirror exists. + * + * - Python state (globals, locals, module_cache, own_gil_tstate, + * own_gil_interp, thread_state, event_loop): created and destroyed on the + * context thread while it holds the GIL (the sub-interpreter's own GIL in + * owngil mode). Other threads may read own_gil_interp and event_loop as + * opaque pointers (nif_context_interrupt, nif_context_get_event_loop) + * but never dereference the Python objects. Refcounts belong to the + * context thread. + * + * - Interrupt (interrupt_mutex, exec_in_flight, exec_thread_id, + * interrupt_pending): see the invariant on interrupt_mutex below. It is + * the only lock a scheduler thread holds while acquiring a GIL, so it + * must never be taken by a thread that already holds one. + * + * Shutdown: nif_context_destroy marks destroyed, cancels the queue, wakes + * the thread and joins it with a timeout. If the thread does not exit the + * context is leaked on purpose (enif_keep_resource) rather than freed + * under a running pthread. * * @see nif_context_create - * @see nif_context_call - * @see subinterp_pool_alloc + * @see nif_context_call_async + * @see nif_context_interrupt + * @see nif_context_destroy */ struct py_context { /** @brief Unique interpreter ID for routing (0 = main, >0 = subinterp) */ @@ -683,7 +731,7 @@ struct py_context { /** @brief Pipe for callback responses [read, write] */ int callback_pipe[2]; - /* ========== Worker thread fields (used by both worker and owngil modes) ========== */ + /* ========== Context thread (worker and owngil modes) ========== */ /** @brief Dedicated pthread for this context */ pthread_t worker_thread; @@ -700,7 +748,7 @@ struct py_context { /** @brief True if thread initialization failed */ _Atomic bool init_error; - /* ========== Request queue (replaces single-slot pattern) ========== */ + /* ========== Request queue (queue_mutex) ========== */ /** @brief Mutex protecting the request queue */ pthread_mutex_t queue_mutex; @@ -717,10 +765,9 @@ struct py_context { /** @brief Environment for sending messages back to Erlang */ ErlNifEnv *msg_env; - /* ========== Legacy compatibility fields (populated from queue request) ========== */ - /* These fields are populated by the worker thread from the current request - * for compatibility with existing execute functions. They will be removed - * once all execute functions are refactored to use ctx_request_t directly. */ + /* ========== Current request (mirror of the ctx_request_t in flight) ========== */ + /* Written and cleared by the context thread around each request; the + * execute functions read the request from here. Context-thread only. */ /** @brief Shared env for current request (points to current req->request_env) */ ErlNifEnv *shared_env; diff --git a/src/py_buffer.erl b/src/py_buffer.erl index 829d703..17d7956 100644 --- a/src/py_buffer.erl +++ b/src/py_buffer.erl @@ -49,6 +49,10 @@ %%% process(line) %%% ''' %%% +%%% +%%% Owns: the native buffer resource (NIF) and the dispatch on handle shape. +%%% Talks to: `py_buffer.c' for native buffers, `py_shm' for `shared => true'. +%%% Never: reads on the Erlang side; Python is the only reader. %%% @end -module(py_buffer). diff --git a/src/py_callback.erl b/src/py_callback.erl index 0eed9ad..ce25fe2 100644 --- a/src/py_callback.erl +++ b/src/py_callback.erl @@ -18,6 +18,12 @@ %%% from Python code via the erlang.call() function. %%% %%% @private +%%% +%%% Owns: the ETS registry name to fun. +%%% Talks to: `py_context' and `py_thread_handler', which look functions up +%%% when Python calls `erlang.call'. +%%% Never: runs the function itself; the caller process does, with the context +%%% blocked on the callback pipe. -module(py_callback). -behaviour(gen_server). diff --git a/src/py_context.erl b/src/py_context.erl index d857b87..385dff4 100644 --- a/src/py_context.erl +++ b/src/py_context.erl @@ -12,23 +12,29 @@ %% See the License for the specific language governing permissions and %% limitations under the License. -%%% @doc Python context process. +%%% @doc The context process: one Python execution environment, one request +%%% at a time. %%% -%%% A py_context process owns a Python context (subinterpreter or worker). -%%% Each process has exclusive access to its context, eliminating mutex -%%% contention and enabling true N-way parallelism. +%%% Every mode goes through this module. In `worker' and `owngil' mode the +%%% process holds a NIF context resource, forwards each request to the +%%% context thread in C (`nif_context_call_async') and waits for its +%%% `{py_result, Ref, Result}'. In `isolated' mode `init/4' hands the +%%% process to `py_isolated', which speaks the same messages to a child OS +%%% process. Callers do not see the difference. %%% -%%% The context is created when the process starts and destroyed when it -%%% stops. All Python operations are serialized through message passing. +%%% == Callbacks == %%% -%%% == Callback Handling == +%%% When Python calls `erlang.call', the context thread blocks on the +%%% callback pipe and sends `{erlang_callback, Id, Fun, Args}' to this +%%% process, which runs the registered function and writes the reply frame +%%% back. Nested requests from the callback are served inline, so callbacks +%%% can call Python again to any depth. %%% -%%% When Python code calls `erlang.call()`, the NIF returns a `{suspended, ...}` -%%% tuple instead of blocking. The context process handles the callback inline -%%% using a recursive receive pattern, enabling arbitrarily deep callback nesting. -%%% -%%% This approach is inspired by PyO3's suspension mechanism and avoids the -%%% deadlock issues that occur with separate callback handler processes. +%%% Owns: the context resource, the request in flight, its timeout and +%%% the process-local envs (`py:call(Ctx, ...)'). +%%% Talks to: `py_nif' (context NIFs), `py_isolated', `py_callback', +%%% `py_context_sup'. +%%% Never: runs Python on a scheduler thread; the context thread does. %%% %%% @end -module(py_context). diff --git a/src/py_context_router.erl b/src/py_context_router.erl index 06e6e27..eb609e5 100644 --- a/src/py_context_router.erl +++ b/src/py_context_router.erl @@ -53,6 +53,10 @@ %%% ok = py_context_router:unbind_context(). %%% %%% +%%% +%%% Owns: the pool tables and the scheduler to context assignment. +%%% Talks to: `py_context' (creation, calls), `py_context_sup'. +%%% Never: executes Python; it picks a context and forwards. %%% @end -module(py_context_router). diff --git a/src/py_event_loop.erl b/src/py_event_loop.erl index 0121cff..c074e7f 100644 --- a/src/py_event_loop.erl +++ b/src/py_event_loop.erl @@ -19,6 +19,13 @@ %% and registers callback functions for Python to call. %% %% @private +%% +%% Owns: the lifecycle of main-interpreter loops and the `erlang.*' loop +%% callbacks Python needs. +%% Talks to: `py_event_worker' (one per loop), `py_event_loop_pool', the loop +%% NIFs. +%% Never: dispatches to owngil loops; those are reached through +%% `py_context:loop_ref/1'. -module(py_event_loop). -behaviour(gen_server). diff --git a/src/py_event_worker.erl b/src/py_event_worker.erl index da8b98e..20963d0 100644 --- a/src/py_event_worker.erl +++ b/src/py_event_worker.erl @@ -5,6 +5,13 @@ %% - Receives `{select, FdRes, Ref, ready_input|ready_output}' directly from enif_select %% - Handles `{timeout, TimerRef}' messages for timer dispatch %% - Manages timers via erlang:send_after to self() +%% +%% Owns: the readiness and timer messages of one loop, and its `task_ready' +%% coalescing. +%% Talks to: the `py_event_loop.c' NIFs (`process_ready_tasks', timers), +%% `py_event_worker_registry'. +%% Never: runs the loop itself in owngil mode (the context thread does) and +%% never blocks on Python. -module(py_event_worker). -behaviour(gen_server). diff --git a/src/py_isolated.erl b/src/py_isolated.erl index 4de2b7a..566c4f8 100644 --- a/src/py_isolated.erl +++ b/src/py_isolated.erl @@ -51,6 +51,12 @@ %%% Use `sys:get_state/1' to see the state and `sys:trace/2' for events. %%% %%% @private +%%% +%%% Owns: the child OS process, its Unix socket and the request in flight. +%%% Talks to: `py_context' (public API, the same messages as the embedded +%%% loop), `py_shm' (region handles crossing the socket), `py_callback' +%%% (registered functions the child calls). +%%% Never: runs Python in the VM, touches NIF resources other than `os_kill'. -module(py_isolated). -behaviour(gen_statem). diff --git a/src/py_shm.erl b/src/py_shm.erl index d14e86f..d9157e6 100644 --- a/src/py_shm.erl +++ b/src/py_shm.erl @@ -37,6 +37,14 @@ %%% a region used as a ring, with the write position and the closed flag in %%% a header page and flow control through the `_py_buffer_wait' and %%% `_py_buffer_consumed' callbacks the Python side calls. +%%% +%%% Owns: the region table (ETS), the backing files, and the ring state of +%%% shared buffers. +%%% Talks to: iommap (through `apply/3', optional dependency), `py_buffer' +%%% (shared variant), `py_callback' (registers `_py_buffer_wait', +%%% `_py_buffer_consumed', `_py_buffer_state'). +%%% Never: maps memory into Python itself; that is `_erlang_impl/_shm.py' in +%%% each interpreter. -module(py_shm). -behaviour(gen_server). diff --git a/src/py_thread_handler.erl b/src/py_thread_handler.erl index 3fe00a4..4455061 100644 --- a/src/py_thread_handler.erl +++ b/src/py_thread_handler.erl @@ -35,6 +35,13 @@ %%% 6. Python thread receives response and continues %%% %%% @private +%%% +%%% Owns: the coordinator process, one handler process and one pipe per Python +%%% thread. +%%% Talks to: `py_callback' (function lookup), the `py_thread_worker.c' side +%%% of the pipe. +%%% Never: touches a context: Python threads that call Erlang are not on a +%%% context thread. -module(py_thread_handler). -behaviour(gen_server).