Skip to content

Commit 4dfe23f

Browse files
benoitcbenoitc
committed
Add OWN_GIL subinterpreter support for true Python parallelism
Subinterpreters with PyInterpreterConfig_OWN_GIL run in dedicated threads, each with its own GIL, enabling true parallel Python execution on Python 3.12+. Key changes: - Thread pool manages subinterpreter lifecycle and context switching - Atomic state machine for thread-safe subinterpreter state management - Support blocking callbacks in thread-model subinterpreters - ProcessError exception class lookup for correct identity in subinterpreters - Test adjustments for subinterpreter path isolation and error messages
1 parent 3cb5854 commit 4dfe23f

31 files changed

+5041
-1204
lines changed

‎.github/workflows/ci.yml‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,87 @@ jobs:
186186
'
187187
continue-on-error: true # Free-threading is experimental
188188

189+
# Sanitizer builds for detecting memory issues and race conditions
190+
test-sanitizers:
191+
name: ${{ matrix.sanitizer }} / Python ${{ matrix.python }}
192+
runs-on: ubuntu-24.04
193+
194+
strategy:
195+
fail-fast: false
196+
matrix:
197+
include:
198+
# ASan + UBSan with Python 3.12
199+
- sanitizer: "ASan+UBSan"
200+
python: "3.12"
201+
cmake_flags: "-DENABLE_ASAN=ON -DENABLE_UBSAN=ON"
202+
env_vars: "ASAN_OPTIONS=detect_leaks=1:abort_on_error=1"
203+
# ASan + UBSan with Python 3.13
204+
- sanitizer: "ASan+UBSan"
205+
python: "3.13"
206+
cmake_flags: "-DENABLE_ASAN=ON -DENABLE_UBSAN=ON"
207+
env_vars: "ASAN_OPTIONS=detect_leaks=1:abort_on_error=1"
208+
# TSan with Python 3.12 (separate because incompatible with ASan)
209+
- sanitizer: "TSan"
210+
python: "3.12"
211+
cmake_flags: "-DENABLE_TSAN=ON"
212+
env_vars: "TSAN_OPTIONS=second_deadlock_stack=1"
213+
214+
steps:
215+
- name: Checkout
216+
uses: actions/checkout@v4
217+
218+
- name: Set up Python
219+
uses: actions/setup-python@v5
220+
with:
221+
python-version: ${{ matrix.python }}
222+
223+
- name: Set up Erlang
224+
uses: erlef/setup-beam@v1
225+
with:
226+
otp-version: "27.0"
227+
rebar3-version: "3.24"
228+
229+
- name: Install dependencies
230+
run: |
231+
sudo apt-get update
232+
sudo apt-get install -y cmake
233+
234+
- name: Set Python library path
235+
run: |
236+
PYTHON_LIB=$(python3 -c "import sysconfig; print(sysconfig.get_config_var('LIBDIR'))")
237+
echo "LD_LIBRARY_PATH=${PYTHON_LIB}:${LD_LIBRARY_PATH}" >> $GITHUB_ENV
238+
239+
- name: Clean and compile with sanitizers
240+
run: |
241+
rm -rf _build/cmake
242+
mkdir -p _build/cmake
243+
cd _build/cmake
244+
cmake ../../c_src ${{ matrix.cmake_flags }}
245+
cmake --build . -- -j $(nproc)
246+
cd ../..
247+
rebar3 compile
248+
249+
- name: Run tests with sanitizers
250+
env:
251+
ASAN_OPTIONS: ${{ contains(matrix.env_vars, 'ASAN_OPTIONS') && 'detect_leaks=1:abort_on_error=1' || '' }}
252+
TSAN_OPTIONS: ${{ contains(matrix.env_vars, 'TSAN_OPTIONS') && 'second_deadlock_stack=1' || '' }}
253+
run: |
254+
# For ASan/UBSan, we need to preload the runtime library
255+
if [[ "${{ matrix.sanitizer }}" == "ASan+UBSan" ]]; then
256+
ASAN_LIB=$(gcc -print-file-name=libasan.so)
257+
export LD_PRELOAD="$ASAN_LIB"
258+
fi
259+
rebar3 ct --readable=compact
260+
261+
- name: Check debug counters
262+
run: |
263+
erl -pa _build/default/lib/erlang_python/ebin -noshell -eval '
264+
application:ensure_all_started(erlang_python),
265+
Counters = py_nif:get_debug_counters(),
266+
io:format("Debug counters: ~p~n", [Counters]),
267+
halt().
268+
'
269+
189270
lint:
190271
name: Lint
191272
runs-on: ubuntu-24.04

‎PLAN.md‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
# Plan: Fix Remaining Test Failures
2+
3+
## Current Status
4+
5+
| Python | Passed | Failed | Skipped | Notes |
6+
|--------|--------|--------|---------|-------|
7+
| 3.9 | 203 | 0 | 12 | All pass (subinterp tests skipped) |
8+
| 3.13 | 224 | 3 | 2 | py_pid_send_SUITE failures |
9+
| 3.14 | 219 | 8 | 2 | py_pid_send_SUITE failures |
10+
11+
## Root Cause
12+
13+
The `py_pid_send_SUITE` tests fail on Python 3.12+ with subinterpreter mode because:
14+
15+
1. `init_per_suite` sets `sys.path` via `py:exec()` on the main context
16+
2. When tests run, they use context router which routes to subinterpreter contexts
17+
3. Subinterpreters have isolated `sys.path` - the path modification doesn't propagate
18+
4. Result: `ModuleNotFoundError: No module named 'py_test_pid_send'`
19+
20+
## Fix Options
21+
22+
### Option 1: Set sys.path per-context (Recommended)
23+
24+
Modify `py_context:init/1` to accept an optional `sys_path` list and set it when creating the context. This ensures each subinterpreter has the correct path.
25+
26+
```erlang
27+
%% In py_context.erl
28+
init(#{sys_path := Paths} = Opts) ->
29+
%% After context creation, set sys.path
30+
lists:foreach(fun(P) ->
31+
py_nif:context_exec(Ctx, <<"import sys; sys.path.insert(0, '", P/binary, "')">>)
32+
end, Paths),
33+
...
34+
```
35+
36+
### Option 2: Fix test setup to use context-aware path setting
37+
38+
Modify `py_pid_send_SUITE:init_per_suite/1` to set the path on all contexts:
39+
40+
```erlang
41+
init_per_suite(Config) ->
42+
{ok, _} = application:ensure_all_started(erlang_python),
43+
TestDir = list_to_binary(code:lib_dir(erlang_python, test)),
44+
%% Set path on all contexts
45+
NumContexts = py_context_router:num_contexts(),
46+
[begin
47+
Ctx = py_context_router:get_context(I),
48+
py_context:exec(Ctx, <<"import sys; sys.path.insert(0, '", TestDir/binary, "')">>)
49+
end || I <- lists:seq(1, NumContexts)],
50+
Config.
51+
```
52+
53+
### Option 3: Use absolute imports in tests
54+
55+
Modify tests to use `importlib` with absolute file paths instead of relying on sys.path.
56+
57+
## Implementation Plan
58+
59+
1. **Fix py_pid_send_SUITE init_per_suite** (Option 2)
60+
- Modify to set sys.path on all contexts, not just main
61+
- This is the minimal fix that doesn't require API changes
62+
63+
2. **Add sys_path option to py:context/1** (Option 1 for future)
64+
- Add `sys_path` option to context creation
65+
- Apply to both worker and subinterpreter modes
66+
- Document in API
67+
68+
3. **Test validation**
69+
- Run full test suite on Python 3.9, 3.13, 3.14
70+
- Ensure all tests pass
71+
72+
## Remaining Issue
73+
74+
### test_send_dead_process_raises_process_error (1 failure on 3.13/3.14)
75+
76+
**Problem:** The test calls `erlang.send(dead_pid, msg)` which raises `ProcessError`.
77+
The Python code tries to catch `erlang.ProcessError` but it's not caught.
78+
79+
**Root cause:** In subinterpreter mode, the `erlang` module is created separately
80+
in each subinterpreter. The `ProcessError` exception class created by the NIF
81+
when raising the error is from a different module instance than the one the
82+
Python code imports. This is a class identity issue:
83+
84+
```python
85+
# In subinterpreter
86+
import erlang # Gets subinterpreter's erlang module
87+
try:
88+
erlang.send(dead_pid, msg) # Raises ProcessError from NIF's erlang module
89+
except erlang.ProcessError: # This is a different class!
90+
return True # Never reached
91+
```
92+
93+
**Fix options:**
94+
1. Store exception classes globally and share across subinterpreters
95+
2. Use string-based exception matching in tests
96+
3. Ensure the NIF uses the same exception class as the subinterpreter's erlang module
97+
98+
## Timeline
99+
100+
1. ~~Fix py_pid_send_SUITE init_per_suite~~ - DONE
101+
2. ~~Validate all Python versions pass~~ - DONE (203/226/226 passed)
102+
3. Fix ProcessError class identity issue - follow-up PR
103+
4. Add sys_path option to context API - follow-up PR

‎c_src/CMakeLists.txt‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,33 @@ if(ASGI_PROFILING)
5757
add_definitions(-DASGI_PROFILING)
5858
endif()
5959

60+
# Sanitizer options for debugging race conditions and memory issues
61+
option(ENABLE_ASAN "Enable AddressSanitizer" OFF)
62+
option(ENABLE_TSAN "Enable ThreadSanitizer" OFF)
63+
option(ENABLE_UBSAN "Enable UndefinedBehaviorSanitizer" OFF)
64+
65+
if(ENABLE_ASAN)
66+
message(STATUS "AddressSanitizer enabled")
67+
add_compile_options(-fsanitize=address -fno-omit-frame-pointer -g -O1)
68+
add_link_options(-fsanitize=address)
69+
# ASan is incompatible with TSan
70+
if(ENABLE_TSAN)
71+
message(FATAL_ERROR "ASan and TSan cannot be used together")
72+
endif()
73+
endif()
74+
75+
if(ENABLE_TSAN)
76+
message(STATUS "ThreadSanitizer enabled")
77+
add_compile_options(-fsanitize=thread -fno-omit-frame-pointer -g -O1)
78+
add_link_options(-fsanitize=thread)
79+
endif()
80+
81+
if(ENABLE_UBSAN)
82+
message(STATUS "UndefinedBehaviorSanitizer enabled")
83+
add_compile_options(-fsanitize=undefined -fno-omit-frame-pointer -g -O1)
84+
add_link_options(-fsanitize=undefined)
85+
endif()
86+
6087
if(PERF_BUILD)
6188
message(STATUS "Performance build enabled - using aggressive optimizations")
6289
# Override compiler flags for maximum performance

0 commit comments

Comments
 (0)