Skip to content

Commit 3cd5715

Browse files
fix: hold a shared socket until the child adopts it, and free exec slots at the close
Windows CI, intermittently, on 3.14 and 3.15: a popen worker died before its handshake with WSAENOTSOCK, which reached the coordinator as a reset connection during makegateway. socket.share() hands the child a *blob*, and there is no socket on the other end until it calls fromshare() -- so closing our copy the moment the spawn returns is a race the child loses. The POSIX comment ("the child holds its own copy now") was true for pass_fds and quietly wrong for share. We now close after the handshake there, which is the point where the child has provably adopted. The server side has the same race and is not fixed: it cannot wait for a handshake that goes to the coordinator, and holding the socket instead would cost the coordinator its EOF when a worker dies. Recorded with the option it needs (a marker byte from the worker) in HANDOFF and the roadmap. Second, from a flake in the exec-capacity test under -n 12, which turned out to be the feature and not the test: the admission slot was released when the exec *task* unwound, a moment after executetask had already sent the channel close. That close is exactly what tells a coordinator at capacity it may send the next request, so waitclose() + remote_exec() could be refused for a slot that was already free. The release moved into _close_finished, ahead of the close, and is idempotent so the task's finally still covers whatever never got that far. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 28072d0 commit 3cd5715

7 files changed

Lines changed: 81 additions & 7 deletions

File tree

CHANGELOG.rst

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,10 @@ series, once the consumers that need them have released without them.
7676
one anyway -- capping exactly the concurrency the profile exists to
7777
provide. The wait is a ``trio.Event`` woken from the exec's own thread
7878
now; same for the main-thread exec under ``profile=thread``.
79+
* The exec slot is released just before the exec's channel close goes out,
80+
not after its task unwinds: that close is what tells a coordinator at
81+
capacity it may send the next request, so ``waitclose()`` followed by
82+
``remote_exec()`` must not be refused for a slot that is already free.
7983
* **An exec that finishes after its connection died no longer takes the
8084
worker down.** Closing the channel is how an exec reports it finished, and
8185
a connection that went away first makes that raise; the exception reached
@@ -107,6 +111,11 @@ series, once the consumers that need them have released without them.
107111
``EXECNET_IGNORE_VERSION_SKEW=1`` in the worker's environment (reachable
108112
as ``env:EXECNET_IGNORE_VERSION_SKEW=1`` in a spec) downgrades it to the
109113
old warning.
114+
* Windows: a ``popen`` coordinator kept its copy of the shared socket open
115+
until the worker's handshake. ``socket.share()`` hands over a blob, not a
116+
socket -- the child only has one once it calls ``fromshare()`` -- so
117+
closing at spawn time raced the child into ``WSAENOTSOCK`` and it died
118+
before handshaking, which the coordinator saw as a reset connection.
110119
* A worker that dies abruptly reports ``EOFError`` on every transport. A
111120
killed peer *resets* a socket -- Windows reports ``WSAECONNRESET`` --
112121
where a pipe would simply reach EOF, so the same event used to surface

HANDOFF.md

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,12 @@ built. Admission is **bounded** for the thread-shaped strategies
123123
(`exec_capacity()`, half the trio thread limiter) and a request over the
124124
line is refused on its channel, not queued — reported as
125125
`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.
126+
and cost no thread. Two ordering rules hold it together: nothing may wait
127+
for an exec by parking a pool thread (that spends the budget it is
128+
rationing), and the slot is released *before* the exec's channel close
129+
goes out (that close is what tells a coordinator at capacity to send the
130+
next request, so `waitclose(); remote_exec()` must not be refused for a
131+
slot already freed).
128132
`AsyncGroup.makegateway` defaults workers to `thread` — the coordinator's
129133
shape does not dictate the worker's.
130134

@@ -196,6 +200,20 @@ shape does not dictate the worker's.
196200
- **Hand a socket over as a socket, never as an fd.** Rebuilding one with
197201
`socket.socket(fileno=fd)` re-derives family/type/proto by querying the
198202
handle, which PyPy on Windows fails with `WinError 10014`.
203+
- **A `share()` blob is not a socket yet.** The child has one only once it
204+
calls `fromshare()`, so the sharer must keep its own copy open until then
205+
or the child gets `WSAENOTSOCK` (10038) and dies before the handshake —
206+
a rare, timing-dependent Windows startup failure that looks like a reset
207+
connection on the coordinator (seen on the 3.14/3.15 CI jobs, 2026-08-01).
208+
`connect_popen_worker` therefore closes its copy *after* the handshake on
209+
Windows and before the handshake everywhere else. **The server side has
210+
the same race and is not fixed**: `serve_socket_connection` closes the
211+
accepted socket as soon as the spawn returns, and it cannot see the
212+
handshake, which goes to the coordinator. Closing late instead is not
213+
the answer — a server holding the connection open means a dead worker
214+
stops being an EOF for the coordinator. It needs the worker to signal
215+
adoption (a marker byte on its stdout, which costs the socket worker its
216+
inherited stdout), so it is a design call rather than a patch.
199217
- Filling in a *missing* spec value is idempotent and fine; rewriting one
200218
the caller set is not. xdist reuses one spec object and re-reads it.
201219
- Worker teardown ends in `os._exit(0)` because trio's `to_thread` cache

ROADMAP-3.0.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,15 @@ protocol is the expensive version of this.
366366
a new reason.
367367
- **Unverified**: whether the socket/`installvia` path works on the Windows
368368
CI job at all. Assume any platform CI has not exercised is broken.
369+
- **The server-side `share()` handoff still races** (the popen one is
370+
fixed; see the invariant in `HANDOFF.md`). `serve_socket_connection`
371+
closes the accepted socket when the spawn returns, which can beat the
372+
worker's `fromshare()` — and it cannot wait for the handshake, which goes
373+
to the coordinator, nor simply close late, which would keep a dead
374+
worker's connection open and cost the coordinator its EOF. The fix is a
375+
marker byte from the worker once it has adopted; the price is that a
376+
socket worker's stdout becomes a pipe to its server rather than the
377+
user's. Decide before claiming Windows `socket=` works.
369378

370379
## Suggested order
371380

src/execnet/_gateway_base.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,14 @@ def _close_finished(self, channel: Channel, error: str | None = None) -> None:
270270
ordinary teardown race (a killed worker, a terminate that outran the
271271
exec), and there is no longer anyone to raise at. Letting the OSError
272272
out lands it in the exec task, whose nursery is the worker's root one.
273+
274+
The exec's admission slot goes back *first*: this close is also what
275+
tells a coordinator at capacity that it may send the next request,
276+
and it must not be able to arrive before the slot it frees.
273277
"""
278+
execpool = self._execpool
279+
if execpool is not None:
280+
execpool.release_slot(channel.id)
274281
try:
275282
channel.close(error)
276283
except OSError as exc:

src/execnet/_trio_gateway.py

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1124,15 +1124,22 @@ async def connect_popen_worker(spec: Any) -> tuple[ByteStream, trio.Process]:
11241124

11251125
import socket as _socket
11261126

1127+
# Windows hands the socket over by *sharing* it, and a share() blob is
1128+
# not a socket yet: the child only has one once it calls fromshare().
1129+
# Closing our copy before that races the child into WSAENOTSOCK, which
1130+
# it reports by dying without a handshake -- so hold it until the
1131+
# handshake byte says the child has adopted. POSIX inherited the fd
1132+
# itself and needs the opposite: close now, or the pair never ends.
1133+
shared = _provision.socket_share_required()
11271134
ours, theirs = _socket.socketpair()
11281135
try:
11291136
process = await _spawn_with_socket(spec, theirs)
11301137
except BaseException:
11311138
ours.close()
11321139
theirs.close()
11331140
raise
1134-
# The child holds its own copy now; ours would keep the pair open.
1135-
theirs.close()
1141+
if not shared:
1142+
theirs.close()
11361143
stream = trio.SocketStream(trio.socket.from_stdlib_socket(ours))
11371144
try:
11381145
await read_handshake_ack(stream, "bootstrap")
@@ -1143,6 +1150,9 @@ async def connect_popen_worker(spec: Any) -> tuple[ByteStream, trio.Process]:
11431150
await process.wait()
11441151
await stream.aclose()
11451152
raise
1153+
finally:
1154+
if shared:
1155+
theirs.close()
11461156
return stream, process
11471157

11481158

src/execnet/_trio_worker.py

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,9 @@ def __init__(
388388
#: admitted and not yet finished -- the number STATUS reports, and
389389
#: what admission is capped on
390390
self._running = 0
391+
#: channel ids currently holding one of those slots, so releasing is
392+
#: idempotent (it happens at the close, and again when the task ends)
393+
self._holding: set[int] = set()
391394
#: resolved on the loop at the first request (see exec_capacity)
392395
self._capacity: int | None = None
393396
self._shutting_down = False
@@ -423,8 +426,22 @@ def capacity(self) -> int | None:
423426
self._capacity = exec_capacity()
424427
return self._capacity
425428

426-
def _track_finish(self) -> None:
429+
def release_slot(self, channelid: int) -> None:
430+
"""Give back the admission slot ``channelid`` holds, once.
431+
432+
Called twice on the ordinary path, and the *early* call is the one
433+
that matters: from ``_close_finished``, just before the exec's
434+
channel close goes out. That close is how a coordinator learns it
435+
may send the next request, so it must not be able to arrive before
436+
the slot it frees -- otherwise ``waitclose(); remote_exec()`` on a
437+
worker at capacity is refused for a slot that was already gone.
438+
The task's own ``finally`` then covers everything that never got as
439+
far as closing.
440+
"""
427441
with self._lock:
442+
if channelid not in self._holding:
443+
return
444+
self._holding.discard(channelid)
428445
self._running -= 1
429446
if self._running == 0:
430447
self._idle.set()
@@ -445,6 +462,7 @@ def schedule(self, channel: Channel, sourcetask: bytes) -> None:
445462
full = True
446463
else:
447464
full = False
465+
self._holding.add(channel.id)
448466
self._running += 1
449467
self._idle.clear()
450468
if full:
@@ -490,7 +508,7 @@ async def _run_exec(self, channel: Channel, item: ExecItem) -> None:
490508
except BaseException as exc:
491509
trace(f"exec task for channel {channel.id} failed: {exc!r}")
492510
finally:
493-
self._track_finish()
511+
self.release_slot(channel.id)
494512

495513
def integrate_as_primary_thread(self) -> None:
496514
self.strategy.integrate_as_primary_thread()

testing/test_gateway.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -719,7 +719,9 @@ def test_one_exec_too_many_is_refused_not_hung(
719719
over = gw.remote_exec("channel.send('running')")
720720
with pytest.raises(RemoteError, match="concurrency limit"):
721721
over.receive(TESTTIMEOUT)
722-
# and the worker still works once a slot actually frees
722+
# and a slot is free the moment its close is observable: the
723+
# release happens before that close goes out, so this sequence
724+
# cannot be refused for a slot that is already gone
723725
freed = channels.pop()
724726
freed.send(None)
725727
freed.waitclose(TESTTIMEOUT)
@@ -762,6 +764,7 @@ async def main() -> int:
762764
gateway=None, # type: ignore[arg-type]
763765
strategy=BoomStrategy(),
764766
)
767+
pump._holding.add(DeadChannel.id)
765768
pump._running = 1
766769
pump._idle.clear()
767770
await pump._run_exec(DeadChannel(), ()) # type: ignore[arg-type]

0 commit comments

Comments
 (0)