Skip to content

Commit ca7f8a7

Browse files
authored
Wait on callback pipes with poll, not select (#78)
select() is undefined for a descriptor above FD_SETSIZE, so a VM with more than 1024 open files could not bring up a thread worker and every thread callback after that failed with "Failed to spawn thread handler". The ready-wait also releases the GIL, and the coordinator logs a failed ready signal instead of leaving Python to time out.
1 parent 6aafcf6 commit ca7f8a7

6 files changed

Lines changed: 114 additions & 23 deletions

File tree

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,11 @@
6666

6767
- `pthread_timedjoin_np` was called without `_GNU_SOURCE`, an implicit
6868
declaration on Linux that newer compilers reject.
69+
- Callback pipes waited with `select()`, which is undefined for a file
70+
descriptor above 1024: in a VM with many open files a thread callback
71+
could time out with "Failed to spawn thread handler". The waits use
72+
`poll()`, the handler ready-wait no longer holds the GIL, and
73+
`py_thread_handler` logs a failed ready signal.
6974

7075
## 4.1.0 (2026-08-15)
7176

c_src/py_nif.h

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@
6464
#define NEED_DLOPEN_GLOBAL 1
6565
#endif
6666

67-
#include <sys/select.h>
67+
#include <poll.h>
6868
/** @} */
6969

7070
/* ============================================================================
@@ -1598,13 +1598,10 @@ static ssize_t read_with_timeout(int fd, void *buf, size_t count, int timeout_ms
15981598
errno = ETIMEDOUT;
15991599
return (ssize_t)got;
16001600
}
1601-
struct timeval tv;
1602-
tv.tv_sec = remain_ms / 1000;
1603-
tv.tv_usec = (remain_ms % 1000) * 1000;
1604-
fd_set fds;
1605-
FD_ZERO(&fds);
1606-
FD_SET(fd, &fds);
1607-
int s = select(fd + 1, &fds, NULL, NULL, &tv);
1601+
/* poll, not select: select() is undefined for fd >= FD_SETSIZE
1602+
* (1024) and a VM with many open files gets pipe fds above it. */
1603+
struct pollfd pfd = { .fd = fd, .events = POLLIN, .revents = 0 };
1604+
int s = poll(&pfd, 1, (int)remain_ms);
16081605
if (s < 0) {
16091606
if (errno == EINTR) continue;
16101607
return -1;
@@ -1690,7 +1687,7 @@ typedef enum {
16901687
* @brief Write exactly @p count bytes to a (typically non-blocking) fd
16911688
* with a deadline.
16921689
*
1693-
* Loops on partial writes / EINTR / EAGAIN. On EAGAIN, uses select() for
1690+
* Loops on partial writes / EINTR / EAGAIN. On EAGAIN, uses poll() for
16941691
* write-readiness with the remaining deadline. Used by the thread-worker
16951692
* write path to avoid pinning a dirty I/O scheduler thread on a stalled
16961693
* Python reader.
@@ -1741,13 +1738,8 @@ static write_result_t write_all_with_deadline(int fd, const void *buf,
17411738
(deadline.tv_sec - now.tv_sec) * 1000L +
17421739
(deadline.tv_nsec - now.tv_nsec) / 1000000L;
17431740
if (remain_ms <= 0) return WRITE_TIMEOUT;
1744-
struct timeval tv;
1745-
tv.tv_sec = remain_ms / 1000;
1746-
tv.tv_usec = (remain_ms % 1000) * 1000;
1747-
fd_set fds;
1748-
FD_ZERO(&fds);
1749-
FD_SET(fd, &fds);
1750-
int s = select(fd + 1, NULL, &fds, NULL, &tv);
1741+
struct pollfd pfd = { .fd = fd, .events = POLLOUT, .revents = 0 };
1742+
int s = poll(&pfd, 1, (int)remain_ms);
17511743
if (s < 0) {
17521744
if (errno == EINTR) continue;
17531745
return WRITE_ERROR;

c_src/py_thread_worker.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -488,8 +488,12 @@ static int thread_worker_spawn_handler(thread_worker_t *tw) {
488488
* condition (Defect 5): both the byte count must match AND the
489489
* value must be zero. Short reads are not silently accepted. */
490490
uint32_t response_len = 0;
491-
ssize_t n = read_with_timeout(tw->response_pipe[0], &response_len,
492-
sizeof(response_len), 10000);
491+
ssize_t n;
492+
/* The coordinator answers without Python; do not hold the GIL for it. */
493+
Py_BEGIN_ALLOW_THREADS
494+
n = read_with_timeout(tw->response_pipe[0], &response_len,
495+
sizeof(response_len), 10000);
496+
Py_END_ALLOW_THREADS
493497
if (n != (ssize_t)sizeof(response_len) || response_len != 0) {
494498
return -1;
495499
}

src/py_thread_handler.erl

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -115,11 +115,17 @@ handle_info({thread_worker_spawn, WorkerId, WriteFd}, #state{handlers = Handlers
115115
HandlerPid = spawn_link(fun() -> handler_loop(WorkerId, WriteFd) end),
116116

117117
%% Signal readiness to Python (write 0 length to indicate success)
118-
py_nif:thread_worker_signal_ready(WriteFd),
119-
120-
%% Store handler mapping
121-
NewHandlers = Handlers#{WorkerId => {HandlerPid, WriteFd}},
122-
{noreply, State#state{handlers = NewHandlers}};
118+
case py_nif:thread_worker_signal_ready(WriteFd) of
119+
ok ->
120+
NewHandlers = Handlers#{WorkerId => {HandlerPid, WriteFd}},
121+
{noreply, State#state{handlers = NewHandlers}};
122+
{error, Reason} ->
123+
%% The Python side times out and reports it; say why here
124+
logger:error("py_thread_handler: ready signal for worker ~p (fd ~p) failed: ~p",
125+
[WorkerId, WriteFd, Reason]),
126+
HandlerPid ! shutdown,
127+
{noreply, State}
128+
end;
123129

124130
%% Handle callback request from Python thread
125131
handle_info({thread_callback, WorkerId, CallbackId, FuncName, Args},

test/py_reentrant_SUITE.erl

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
test_callback_with_complex_types/1,
2323
test_multiple_sequential_callbacks/1,
2424
test_call_from_non_worker_thread/1,
25+
test_thread_callback_fd_above_fd_setsize/1,
2526
test_callback_with_try_except/1,
2627
test_async_call/1,
2728
test_callback_name_registry/1,
@@ -38,6 +39,7 @@ all() ->
3839
test_callback_with_complex_types,
3940
test_multiple_sequential_callbacks,
4041
test_call_from_non_worker_thread,
42+
test_thread_callback_fd_above_fd_setsize,
4143
test_callback_with_try_except,
4244
test_async_call,
4345
test_callback_name_registry,
@@ -133,6 +135,12 @@ test_etf_decode_safe(_Config) ->
133135

134136
%% Negative: many DISTINCT brand-new atoms wrapped in marker-shaped binaries
135137
%% must all come back verbatim, never decoded into atoms.
138+
%% Warm up first so modules loaded on first use (base64, the callback
139+
%% path) do not count as atoms minted by the round trips below.
140+
Warm = etf_marker(novel_atom_etf("zzqx_etf_safe_warmup")),
141+
py:register_function(etf_probe_novel, fun(_) -> Warm end),
142+
{ok, Warm} = py:eval(<<"__import__('erlang').call('etf_probe_novel', [])">>),
143+
assert_atom_absent("zzqx_etf_safe_warmup"),
136144
Before = erlang:system_info(atom_count),
137145
N = 50,
138146
lists:foreach(
@@ -349,6 +357,32 @@ test_call_from_non_worker_thread(_Config) ->
349357
py:unregister_function(simple_add),
350358
ok.
351359

360+
%% @doc Thread callbacks must work when the response pipe lands on an fd
361+
%% above FD_SETSIZE: select() is undefined there and used to make the
362+
%% handler ready-wait time out after 10 s ("Failed to spawn thread handler").
363+
test_thread_callback_fd_above_fd_setsize(_Config) ->
364+
py:register_function(high_fd_add, fun([A, B]) -> A + B end),
365+
TestDir = filename:join(code:lib_dir(erlang_python), "test"),
366+
ok = py:exec(iolist_to_binary(io_lib:format(
367+
"import sys; sys.path.insert(0, '~s')", [TestDir]))),
368+
try
369+
case py:call(py_test_high_fds, prepare, [1200]) of
370+
{ok, Last} when Last >= 1200 ->
371+
ct:log("highest fd opened: ~p", [Last]),
372+
N = 8,
373+
{ok, Results} = py:call(py_test_high_fds, call_from_threads, [N]),
374+
Expected = [I + 1 || I <- lists:seq(0, N - 1)],
375+
Expected = Results;
376+
{ok, -1} ->
377+
{skip, "fd hard limit too low to open 1200 files"};
378+
Other ->
379+
ct:fail({prepare_failed, Other})
380+
end
381+
after
382+
_ = py:call(py_test_high_fds, cleanup, []),
383+
py:unregister_function(high_fd_add)
384+
end.
385+
352386
%% @doc Test that erlang.call() works even when wrapped in try/except blocks.
353387
%% This simulates ASGI/WSGI middleware that catches all exceptions.
354388
%% The flag-based detection should work even when the SuspensionRequired

test/py_test_high_fds.py

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
"""Thread callbacks with pipe fds above FD_SETSIZE.
2+
3+
Opens enough files to push every fd the runtime creates afterwards above
4+
1024, then has several new threads call Erlang at once so fresh thread
5+
workers (and their pipes) are created in that range. select() cannot
6+
watch such fds; poll() can.
7+
"""
8+
import concurrent.futures
9+
import os
10+
import resource
11+
12+
_kept = []
13+
14+
15+
def prepare(target=1200):
16+
"""Raise the fd soft limit and fill descriptors up to `target`.
17+
18+
Returns the highest fd opened, or -1 if the hard limit is too low.
19+
"""
20+
soft, hard = resource.getrlimit(resource.RLIMIT_NOFILE)
21+
want = target + 256
22+
if hard != resource.RLIM_INFINITY and hard < want:
23+
return -1
24+
if soft < want:
25+
resource.setrlimit(resource.RLIMIT_NOFILE, (want, hard))
26+
last = -1
27+
while last < target:
28+
fd = os.open(os.devnull, os.O_RDONLY)
29+
_kept.append(fd)
30+
last = fd
31+
return last
32+
33+
34+
def call_from_threads(n):
35+
import erlang
36+
with concurrent.futures.ThreadPoolExecutor(max_workers=n) as ex:
37+
futures = [ex.submit(erlang.call, 'high_fd_add', i, 1) for i in range(n)]
38+
results = []
39+
for f in futures:
40+
try:
41+
results.append(f.result())
42+
except Exception as exc:
43+
results.append('error: %s' % exc)
44+
return results
45+
46+
47+
def cleanup():
48+
while _kept:
49+
os.close(_kept.pop())
50+
return 'ok'

0 commit comments

Comments
 (0)