Skip to content

Commit 28072d0

Browse files
fix: a test that only held locally, and a cap gevent should never have had
CI caught the first one on every Linux runner: the skewed-version test spelled an "impossible" coordinator version, 0.1.2, and a wheel built from a checkout without tags is 0.1.dev1 -- which is exactly what CI installs. The versions matched, the worker started and served, and the test blocked on its output until it timed out. The skew is derived from the real version now, the way the unit tests around it already did. The second was mine too, in the message I had just written: it points at profile=gevent as the way past the exec cap, and profile=gevent was capped right along with thread. Waiting for an exec that runs elsewhere -- the main thread, a greenlet -- parked a pool thread on a threading.Event, so execs that cost no thread each held one anyway, and a gevent worker was rationed to 20 concurrent greenlets. That is a cap on the one thing the profile is for. The wait is a trio.Event woken from the exec's own thread now, and capacity is the strategy's to declare: None where execs are tasks or greenlets, half the thread budget where they are threads. Also drops the last of the review's stale wording: Channel.send has not blocked on a full queue since 2.1's write lock, and says what it does do. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 21b6072 commit 28072d0

7 files changed

Lines changed: 127 additions & 34 deletions

File tree

CHANGELOG.rst

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,15 @@ series, once the consumers that need them have released without them.
6767
Admitting it instead made request 41 wait for a slot only a finishing exec
6868
could free, which from the coordinator is indistinguishable from a hung
6969
``remote_exec``. ``remote_status()`` gained ``execcapacity`` (``None``
70-
under ``profile=trio``, whose execs are tasks and are not bounded this
71-
way), and ``numexecuting`` now counts what is really running.
70+
under ``profile=trio`` and ``profile=gevent``, whose execs are tasks and
71+
greenlets and are not bounded this way), and ``numexecuting`` now counts
72+
what is really running.
73+
* **A ``profile=gevent`` worker is no longer limited to as many concurrent
74+
execs as it has threads.** Waiting for a greenlet to finish parked a pool
75+
thread on a ``threading.Event``, so execs that cost no thread each held
76+
one anyway -- capping exactly the concurrency the profile exists to
77+
provide. The wait is a ``trio.Event`` woken from the exec's own thread
78+
now; same for the main-thread exec under ``profile=thread``.
7279
* **An exec that finishes after its connection died no longer takes the
7380
worker down.** Closing the channel is how an exec reports it finished, and
7481
a connection that went away first makes that raise; the exception reached

HANDOFF.md

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ This is the doc to read first. Two companions:
1111
## How to work here
1212

1313
```
14-
uv run pytest testing/ # 582 passed, 66 skipped
14+
uv run pytest testing/ # 583 passed, 66 skipped
1515
uv run pytest testing/ -n 12 # must stay green (~8s)
1616
uv run pre-commit run -a # never grep-filter its output
1717
uv run tox -e docs # sphinx -W, then doctests all of doc/
@@ -119,9 +119,12 @@ request rather than letting a gateway hang.
119119

120120
`TrioWorkerExec` is a FIFO admission pump delegating to strategy objects
121121
(`WORKER_EXEC_STRATEGIES`); subinterpreters are a future strategy slot, not
122-
built. Admission is **bounded** (`exec_capacity()`, half the trio thread
123-
limiter) and a request over the line is refused on its channel, not
124-
queued — reported as `remote_status().execcapacity`.
122+
built. Admission is **bounded** for the thread-shaped strategies
123+
(`exec_capacity()`, half the trio thread limiter) and a request over the
124+
line is refused on its channel, not queued — reported as
125+
`remote_status().execcapacity`, `None` where execs are tasks or greenlets
126+
and cost no thread. Nothing may wait for an exec by parking a pool
127+
thread: that spends the budget it is rationing.
125128
`AsyncGroup.makegateway` defaults workers to `thread` — the coordinator's
126129
shape does not dictate the worker's.
127130

doc/basics.rst

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -301,18 +301,19 @@ How many at once
301301

302302
.. versionadded:: 3.0
303303

304-
``thread`` and ``gevent`` execs each need a thread of the worker's thread
305-
budget, which also has to serve channel callbacks and the worker's own
306-
protocol work -- so a worker admits **half that budget** in concurrent
304+
Under ``thread`` each exec needs a thread of the worker's thread budget,
305+
which also has to serve channel callbacks and the worker's own protocol
306+
work -- so a worker admits **half that budget** in concurrent
307307
``remote_exec`` calls (20, unless the worker changed trio's default
308308
limiter) and *refuses* the one after that with a ``RemoteError`` naming the
309309
limit. ``remote_status().execcapacity`` reports the number.
310310

311311
Refusing rather than queueing is deliberate: a request waiting for a thread
312312
that only a finishing exec can free is indistinguishable, from the
313313
coordinator, from an exec that hung. For genuine fan-out use more gateways
314-
-- that is what a ``Group`` is for -- or ``profile=trio``, whose execs are
315-
tasks on the worker's loop and are not bounded this way.
314+
-- that is what a ``Group`` is for -- or a profile whose execs are not
315+
threads. ``trio`` and ``gevent`` are unbounded here (``execcapacity`` is
316+
``None``): their execs are tasks and greenlets, and spend no thread.
316317

317318

318319
Transports

src/execnet/_channel.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -327,12 +327,17 @@ def waitclose(self, timeout: float | None = None) -> None:
327327
raise error
328328

329329
def send(self, item: object) -> None:
330-
"""Sends the given item to the other side of the channel,
331-
possibly blocking if the sender queue is full.
330+
"""Sends the given item to the other side of the channel.
332331
333332
The item must be a simple Python type and will be
334333
copied to the other side by value.
335334
335+
Returns once the data has reached the OS write, which is not the
336+
same as the peer having read it: there is no flow control, so a
337+
peer that never receives buffers everything sent to it rather than
338+
pushing back. Sending unboundedly to one is a memory leak in *its*
339+
process.
340+
336341
OSError is raised if the write pipe was prematurely closed.
337342
"""
338343
# before the state check: an unusable gateway (inherited by a fork,

src/execnet/_trio_worker.py

Lines changed: 58 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ class PoolExec:
4747

4848
#: whether _run_worker must hand this strategy the process main thread
4949
needs_primary_thread = False
50+
#: whether each concurrent exec occupies a thread of the worker's budget
51+
exec_costs_a_thread = True
5052

5153
def __init__(self, gateway: WorkerGateway) -> None:
5254
self.gateway = gateway
@@ -69,6 +71,28 @@ def trigger_shutdown(self) -> None:
6971
pass
7072

7173

74+
def _thread_signal() -> tuple[trio.Event, Callable[[], None]]:
75+
"""A loop-side event plus the callable that sets it from a foreign thread.
76+
77+
Waiting for an exec that runs *elsewhere* -- the main thread, a greenlet
78+
-- must not park a pool thread on a ``threading.Event``: that thread is
79+
part of the budget exec placement is rationed against, so an exec that
80+
costs no thread would still spend one waiting for itself.
81+
82+
Must be built on the loop (it captures the trio token).
83+
"""
84+
done = trio.Event()
85+
token = trio.lowlevel.current_trio_token()
86+
87+
def signal() -> None:
88+
# posted callbacks must not raise, and a loop that ended while the
89+
# exec ran leaves nobody to wake
90+
with suppress(trio.RunFinishedError):
91+
token.run_sync_soon(done.set)
92+
93+
return done, signal
94+
95+
7296
class PrimaryThreadPump:
7397
"""Runs exec requests handed to it on the process main thread.
7498
@@ -79,10 +103,14 @@ class PrimaryThreadPump:
79103

80104
#: whether _run_worker must hand this strategy the process main thread
81105
needs_primary_thread = True
106+
#: whether each concurrent exec occupies a thread of the worker's budget
107+
#: (see TrioWorkerExec.capacity): the primary one does not, but the
108+
#: overflow HybridExec sends to the pool does
109+
exec_costs_a_thread = True
82110

83111
def __init__(self, gateway: WorkerGateway) -> None:
84112
self.gateway = gateway
85-
self._primary: Mailbox[tuple[Channel, ExecItem, threading.Event] | None] = (
113+
self._primary: Mailbox[tuple[Channel, ExecItem, Callable[[], None]] | None] = (
86114
Mailbox()
87115
)
88116

@@ -91,9 +119,9 @@ async def admit(self, channel: Channel, item: ExecItem) -> bool:
91119
return True
92120

93121
async def run(self, channel: Channel, item: ExecItem) -> None:
94-
done = threading.Event()
95-
self._primary.put((channel, item, done))
96-
await trio.to_thread.run_sync(done.wait, abandon_on_cancel=True)
122+
done, signal = _thread_signal()
123+
self._primary.put((channel, item, signal))
124+
await done.wait()
97125

98126
def released(self) -> None:
99127
"""Hook: the main thread is free again (called on it, before done)."""
@@ -104,14 +132,14 @@ def integrate_as_primary_thread(self) -> None:
104132
task = self._primary.get()
105133
if task is None:
106134
break
107-
channel, item, done = task
135+
channel, item, signal = task
108136
try:
109137
self.gateway.executetask((channel, item))
110138
finally:
111139
# Release before signalling: the next request should see the
112140
# main thread free as early as we can make it.
113141
self.released()
114-
done.set()
142+
signal()
115143

116144
def trigger_shutdown(self) -> None:
117145
self._primary.put(None)
@@ -186,34 +214,39 @@ class GreenletExec:
186214
"""
187215

188216
needs_primary_thread = True
217+
#: greenlets, not threads: concurrent execs here cost the thread budget
218+
#: nothing, so they are not rationed against it
219+
exec_costs_a_thread = False
189220

190221
def __init__(self, gateway: WorkerGateway) -> None:
191222
from ._boundary import make_wakener
192223

193224
self.gateway = gateway
194225
# The integrate loop blocks in get() on the hub thread: a gevent
195226
# wakener parks only its root greenlet, letting exec greenlets run.
196-
self._primary: Mailbox[tuple[Channel, ExecItem, threading.Event] | None] = (
227+
self._primary: Mailbox[tuple[Channel, ExecItem, Callable[[], None]] | None] = (
197228
Mailbox(make_wakener("gevent"))
198229
)
199230

200231
async def admit(self, channel: Channel, item: ExecItem) -> bool:
201232
return True
202233

203234
async def run(self, channel: Channel, item: ExecItem) -> None:
204-
done = threading.Event()
205-
self._primary.put((channel, item, done))
206-
await trio.to_thread.run_sync(done.wait, abandon_on_cancel=True)
235+
done, signal = _thread_signal()
236+
self._primary.put((channel, item, signal))
237+
await done.wait()
207238

208239
def integrate_as_primary_thread(self) -> None:
209240
"""Run the hub on the main thread, spawning a greenlet per exec."""
210241
import gevent
211242

212-
def run_exec(channel: Channel, item: ExecItem, done: threading.Event) -> None:
243+
def run_exec(
244+
channel: Channel, item: ExecItem, signal: Callable[[], None]
245+
) -> None:
213246
try:
214247
self.gateway.executetask((channel, item))
215248
finally:
216-
done.set()
249+
signal()
217250

218251
while True:
219252
task = self._primary.get()
@@ -373,14 +406,19 @@ def active_count(self) -> int:
373406
with self._lock:
374407
return self._running
375408

376-
def capacity(self) -> int:
377-
"""Concurrent execs this worker admits (loop thread only).
409+
def capacity(self) -> int | None:
410+
"""Concurrent execs this worker admits, or None for unbounded.
378411
379-
Resolved on first use rather than in ``__init__``, which runs before
380-
there is a loop to read the thread limiter from. Reported by
381-
``remote_status()`` as ``execcapacity``, because a coordinator that
382-
just had a request refused wants the number it hit.
412+
Only the thread-shaped strategies are rationed: a greenlet exec
413+
spends no thread, so bounding it would cap the very thing
414+
``profile=gevent`` exists to provide. Resolved on first use rather
415+
than in ``__init__``, which runs before there is a loop to read the
416+
thread limiter from. Reported by ``remote_status()`` as
417+
``execcapacity``, because a coordinator that just had a request
418+
refused wants the number it hit.
383419
"""
420+
if not self.strategy.exec_costs_a_thread:
421+
return None
384422
if self._capacity is None:
385423
self._capacity = exec_capacity()
386424
return self._capacity
@@ -403,7 +441,7 @@ def schedule(self, channel: Channel, sourcetask: bytes) -> None:
403441
if self._shutting_down:
404442
channel.close("execution disallowed")
405443
return
406-
if self._running >= capacity:
444+
if capacity is not None and self._running >= capacity:
407445
full = True
408446
else:
409447
full = False
@@ -416,7 +454,7 @@ def schedule(self, channel: Channel, sourcetask: bytes) -> None:
416454
" concurrency limit (half its thread budget; the rest serves"
417455
" channel callbacks and protocol work). Use more gateways, or"
418456
" a profile whose execs are not threads (profile=trio,"
419-
" profile=gevent)."
457+
" profile=gevent), which are not bounded this way."
420458
)
421459
return
422460
# Already on the Trio host thread (Message handler).

testing/test_cli.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,15 @@ def test_the_override_also_comes_from_the_config_env(self, capfd) -> None:
132132

133133
@posix_only
134134
def test_a_skewed_worker_exits_with_a_reason(self) -> None:
135+
from execnet import _trio_worker
136+
137+
# derived, never spelled out: a literal "impossible" version is only
138+
# impossible until an environment has it. A wheel built from a
139+
# checkout without tags is 0.1.dev1, which is what CI installs -- so
140+
# a hardcoded 0.1.2 matched there, the worker started, and this
141+
# blocked on its output until the test timed out.
142+
major, _ = _trio_worker._rough_version(execnet.__version__)
143+
skewed = f"{major + 1}.0.0"
135144
ours, theirs = socket.socketpair()
136145
out = subprocess.run(
137146
[
@@ -142,7 +151,7 @@ def test_a_skewed_worker_exits_with_a_reason(self) -> None:
142151
"--protocol-fd",
143152
str(theirs.fileno()),
144153
"--config",
145-
worker_config(coordinator_version="0.1.2"),
154+
worker_config(coordinator_version=skewed),
146155
],
147156
pass_fds=(theirs.fileno(),),
148157
capture_output=True,

testing/test_gevent.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
from __future__ import annotations
1212

1313
import threading
14+
from contextlib import suppress
1415

1516
import pytest
1617

@@ -179,6 +180,35 @@ def test_execs_cooperate_via_gevent(self, worker_gw) -> None:
179180
def test_status_reports_gevent(self, worker_gw) -> None:
180181
assert worker_gw.remote_status().execmodel == "gevent"
181182

183+
def test_execs_are_not_rationed_against_the_thread_budget(self, worker_gw) -> None:
184+
"""Greenlets cost no thread, so nothing caps them at the thread limit.
185+
186+
The exec-admission bound exists because a thread-shaped exec spends
187+
a thread the callbacks and protocol work also need. A greenlet
188+
spends none -- but waiting for one used to park a pool thread, so a
189+
gevent worker was rationed to 20 concurrent execs, which is a cap on
190+
exactly what the profile is for.
191+
"""
192+
import trio
193+
194+
from execnet import _trio_worker
195+
196+
async def thread_bound_capacity() -> int:
197+
return _trio_worker.exec_capacity()
198+
199+
assert worker_gw.remote_status().execcapacity is None
200+
wanted = trio.run(thread_bound_capacity) + 5
201+
channels = [
202+
worker_gw.remote_exec("channel.send('go'); channel.receive()")
203+
for _ in range(wanted)
204+
]
205+
try:
206+
assert [ch.receive(TESTTIMEOUT) for ch in channels] == ["go"] * wanted
207+
finally:
208+
for ch in channels:
209+
with suppress(OSError):
210+
ch.send(None)
211+
182212

183213
def test_provisioning_adds_gevent_requirement() -> None:
184214
from execnet import XSpec

0 commit comments

Comments
 (0)