[laughingman7743]

WHAT

Files opened in a forked child process no longer hang, through either filesystem:

• S3ThreadPoolExecutor runs its calls in an S3AioThreadPool, which creates new threads in a forked child, instead of a ThreadPoolExecutor, whose copy in the child counts the parent's threads as idle and starts none of its own. Work submitted in the child to an executor that had run a call in the parent, such as the range reads of a file that crossed the fork, now runs.
• AioS3FileSystem._create_executor() reads the event loop through fsspec's loop property, so open() of a filesystem with an event loop of its own (not asynchronous=True) created in the parent raises RuntimeError("This class is not fork-safe") in the child, as fsspec's synchronous methods do. It used to return a file whose reads waited forever for the parent's loop, whose copy reports running but never runs.
• A register_at_fork hook detaches each S3AioExecutor in a forked child from the parent's loop and replaces its lock, as S3AioThreadPool already resets itself. submit() then raises RuntimeError, and shutdown() only shuts down its pool. Both used to wait forever, so closing an AioS3File that crossed the fork hung.
• docs/aio.md describes the behavior after a fork.
• The fork-and-wait code of the existing S3AioThreadPool fork test moves into a test helper, run_in_forked_child(), which prints the traceback of a failed check in the child; a forking decorator replaces the repeated skip and warning marks.

A filesystem created in the child works with both filesystems; fsspec's instance cache creates a new instance there. An AioS3FileSystem created with asynchronous=True has no loop, so its files use S3ThreadPoolExecutor and work after a fork.

Release note:

• In a forked child process, AioS3FileSystem.open() of an instance created in the parent raises RuntimeError instead of returning a file that hangs, and so does S3AioExecutor.submit().

WHY

Closes #1336 (defects 12 and 13 of the #1327 audit).

This is step 1 of the order decided in #1332 (comment 6598062349), before the executors are consolidated (step 2) and the batch operations of #1322 are rebuilt on them (#1344).
S3ThreadPoolExecutor and S3AioThreadPool merge into one pool type in step 2; here, S3ThreadPoolExecutor delegates to S3AioThreadPool, which already resets after a fork, so its interface and FIFO start order are unchanged.
S3AioExecutor raises instead of hopping to another loop: #1332 keeps its interface until step 2, which decides on files inherited across a fork (rejected rather than supported).

Out of scope, for step 2, which rejects files inherited across a fork:

• A file whose requests were in flight when the process forked still waits for futures that only the parent resolves. Failing them in the at-fork hook could deadlock the fork on a future's lock held by a thread of the parent.
• As for S3AioThreadPool before this change, the child keeps the copy of each pool's ThreadPoolExecutor (with any queued calls, such as part bodies) until it exits, since collecting it could take a queue lock that a thread of the parent held. This now also applies to the executors of sync files open at the fork.

Known dependency limit: fsspec 2026.9.0's at-fork hook resets a misnamed global instead of the lock of its IO loop, so a child forked while another thread of the parent was creating that loop deadlocks when it creates an AioS3FileSystem. fsspec/filesystem_spec#2242 fixed this upstream, after the latest release.

TEST

Tested commit: e2d2496 (the full suites at the commits named below; the commits after f192c0f change only the two executor fork tests, which wait for the loop to run, hold the lock until released, and bound their cleanup; the fork tests pass at e2d2496, and removing the lock reset of the pool or of the executor still fails its test).

• Reproduced offline at 4a0cadb (each killed by a 3 s alarm in the child), and fixed at the tested commit:
  • submit to an S3ThreadPoolExecutor that ran a call before the fork: hung → result returned;
  • AioS3FileSystem._create_executor() of an instance created before the fork: hung → RuntimeError;
  • submit() and shutdown(wait=True) of an S3AioExecutor created before the fork: hung → RuntimeError and return;
  • a new AioS3FileSystem in the child: its executor runs calls.
• just lint: passed.
• just docs lint (0 errors) and just docs build: passed.
• uv run --env-file .env pytest -n 8 tests/pyathena/filesystem/, including the AWS integration tests: 1423 passed, 6 skipped at 2169c92 and at f192c0f. One earlier run at f192c0f failed test_fsspec_conformance.py::TestS3FileSystemCopy::test_copy_directory_without_files_with_same_name_prefix (failure and teardown error); its output was not kept, so the cause is unknown. The test does not fork, and it passed in three isolated runs and in the full rerun above.
• just benchmark test: 336 passed, 2 skipped, 3 failed in tests/test_fleet.py::TestWork (test_losing_a_slot_does_not_stop_the_job and the two throttled-quiescing tests). The same three fail on the unmodified master checkout locally (also with AWS_EC2_METADATA_DISABLED=true, as CI sets it), each run taking about 9 minutes; the cause is not identified, and this change does not touch the fleet harness. The Benchmark tooling workflow, which runs just benchmark test, passed on this PR at e2d2496.
• The offline reproduction gives the same results at f192c0f, and an AioS3FileSystem(asynchronous=True) created before the fork runs calls in the child through S3ThreadPoolExecutor, both from a new executor and from one created before the fork.
• New tests: TestS3ThreadPoolExecutor::test_fork, TestS3AioExecutor::test_fork (with a parent thread holding the executor's lock across the fork), and TestAioS3FileSystem::test_open_in_forked_child.
• Mutation check, reverting each fix alone, each fails exactly one of the new tests: self._loop in _create_executor, a plain ThreadPoolExecutor (at 2169c92), and, in the at-fork hook of S3AioExecutor, keeping the loop, keeping the lock, or not marking it forked. The S3AioThreadPool fork test, moved onto the helper, still fails without the lock reset of the pool.

Not run yet: the AWS CI jobs, which start when the PR is marked Ready.

🤖 Generated with Claude Code

[laughingman7743]

Self-review round 1 (implementation behavior): FINDINGS, repaired in f192c0f.

Scope: git diff 4a0cadb2eabc0b0831b0e0f5c01932661495ebd8..2169c92d63aa8316f5d060c9504dc9086cd21048 (base = merge-base with master), every changed file: s3_executor.py, s3_async.py, docs/aio.md, and the tests in test_s3_executor.py, test_s3_async.py, util.py. Perspectives: behavior and failure paths (fork, shutdown, close of a file that crossed the fork, asynchronous=True instances), resource boundaries (threads, locks, retained executors after fork), simplicity and test quality (/code-review high and /simplify with four angles).

Repaired here: S3AioExecutor compared PIDs in both submit() and shutdown(), and the child path of shutdown() duplicated the shutdown and stepped around a lock that a thread of the parent may hold. It now resets in a register_at_fork hook, as S3AioThreadPool does (loop detached, lock replaced, marked forked), and shutdown() takes its existing path for an executor without a loop. The old comment there also claimed that no function had been dispatched to the pool in the child, which is false for calls in flight at the fork; the comment is gone with the branch. Mutation check: keeping the loop, the lock, or the unforked mark in the hook each fails TestS3AioExecutor::test_fork.

[laughingman7743]

Self-review round 1: FINDING, repaired in f192c0f. The docs and the Raises section of _create_executor() said that an AioS3FileSystem created in the parent raises from open() in a child. An instance created with asynchronous=True has _loop None, gets an S3ThreadPoolExecutor, and works after the fork (now fork-safe). Both now name only instances with an event loop of their own.

[laughingman7743]

Self-review round 1: deferrals and skipped suggestions.

• Deferred to step 2 of Consolidate the filesystem executors before parallel batch operations #1332 (files inherited across a fork are rejected): the child keeps each pool's copied ThreadPoolExecutor in S3AioThreadPool._forked_executors until it exits, now also for the executors of sync files open at the fork, with any queued part bodies; collecting it could take a queue lock held by a parent thread. Calls in flight at the fork still leave their futures unresolved; failing them in the hook could deadlock the fork on a future's lock. Both are stated in the PR body.
• Skipped: subclassing or aliasing S3AioThreadPool instead of delegating (it would expose run()/release(), and step 2 merges the types); the per-call bookkeeping of S3AioThreadPool (about 1-3 µs per call against 10-100 ms requests); removing the S3ThreadPoolExecutor cases of the parametrized TestS3Executor tests, which protect that class's public context and argument-freeing contract independent of its implementation.

[laughingman7743]

Self-review round 1: repaired in f192c0f. A failed check in the child exited 1 with no diagnostics; it now prints the traceback, which pytest shows in the captured output. The repeated skipif/filterwarnings pair on the four fork tests is the forking decorator above.

[laughingman7743]

Self-review round 2 (claims, callers, evidence): CLEAN after corrections to the PR body; no code change.

Scope: git diff 4a0cadb2eabc0b0831b0e0f5c01932661495ebd8..f192c0fedfa38b37be0ef3c56b2278d084b78922, the PR body, docstrings, comments, docs/aio.md, and both commit messages.

Claims checked:

• A copied ThreadPoolExecutor starts no thread in the child: _adjust_thread_count returns once _idle_semaphore.acquire(timeout=0) succeeds, and the stdlib's at-fork hooks only reset _global_shutdown_lock and clear _threads_queues (CPython 3.12 and 3.13 sources; 3.11 and 3.14 are not installed locally, and CI covers them).
• The interface and FIFO order of S3ThreadPoolExecutor are unchanged: the constructor still takes max_workers, a non-positive value still raises ValueError, and the calls still go through one ThreadPoolExecutor queue. Only the private _executor attribute is gone; no caller in pyathena/, tests/, or benchmarks/ uses it.
• fsspec's loop raises RuntimeError("This class is not fork-safe") after a fork, and _Cached.__call__ clears the instance cache when the PID changes, so a filesystem created in the child is new (fsspec of the lock file; the property predates the fsspec>=2026.9.0 floor).
• Closing an AioS3File that crossed the fork hung: S3File.close() calls self._executor.shutdown() with wait=True (s3.py:2983), the hang reproduced at the merge base.
• asynchronous=True: verified at f192c0f in a child (new and pre-fork S3ThreadPoolExecutor both run calls).
• The docs outside the diff (benchmarks/README.md, the benchmark history) describe S3ThreadPoolExecutor only as where sync files run parallel requests, which still holds.

Corrections: the PR body's TEST now separates the runs at each commit, and records an unexplained failure of TestS3FileSystemCopy::test_copy_directory_without_files_with_same_name_prefix in one full run at f192c0f, whose output was not kept. That test does not fork, and it passed three times alone and in a full rerun (1423 passed, 6 skipped). The refactored S3AioThreadPool fork test was checked to still fail without the pool's lock reset.

No AWS operator effect: no request, retry, or quota path changes.

[laughingman7743]

Independent review (relayed): Codex CLI 0.166.0, model gpt-6-astra, codex exec -s read-only (session 01a1a19a-c316-7261-b2ce-54676f9e9e11), static review of a detached snapshot of f192c0fedfa38b37be0ef3c56b2278d084b78922 against merge base 4a0cadb2eabc0b0831b0e0f5c01932661495ebd8. The prompt held the diff and the repository conventions, without the PR text or the self-review conclusions; no edits, tests, or network. Both worktrees were unchanged afterwards. Covered: fork hooks, inherited locks and loops, new child objects, both filesystems and asynchronous=True; constructor, submit/shutdown, context, ordering, GC, retention, interpreter exit; all added or rewritten tests, docstrings, comments, and docs. Verdict: FINDINGS (three).

Finding 3 (P2): the new TestS3AioExecutor::test_fork races the start of the event loop. thread.start() can return before run_forever() marks the loop running, and the first submit() then raises RuntimeError before the fork is exercised.

Verified: with the loop's start delayed by 0.2 s, submit() raises "requires a running event loop". Repaired in 7fafe8d: the test waits for a callback scheduled on the loop before it submits (fork tests: 4 passed).

[laughingman7743]

Repair and independent follow-ups (relayed).

Self-review of the 7fafe8d repair, both perspectives: the readiness wait runs inside try, so the loop is still stopped if it times out; the commit message is accurate. The PR body names 7fafe8d and the fsspec limit.

Follow-up 1 (Codex gpt-6-astra, read-only, session 01a1a1c1-5c53-7091-a97c-fb728f3731a1, static, f192c0fe..7fafe8d2 in the context of the whole test): confirmed that the startup race is gone; FINDINGS on the test's robustness: (P2) the cleanup could wait forever in executor.shutdown() before loop.stop(), and the unchecked join let loop.close() raise on a running loop; (P2) the holder released the lock after 10 s, so a slow parent could fork with the lock free and pass without the lock reset (same pattern in TestS3AioThreadPool::test_fork); (P3) loop.stop() could leave a dispatch task pending that close() discards.

Repaired in e2d2496: holders keep the lock until the test releases it and run as daemons (both fork tests); the executor is shut down with shutdown_bounded(), the loop thread is a daemon and is checked to have ended, and the pending tasks are run before close(). Fork tests: 4 passed; removing the lock reset of the pool or of the executor still fails its test.

Follow-up 2 (Codex gpt-6-astra, read-only, session 01a1a1c7-39f9-7f71-b6cd-a6a6de452835, static, 7fafe8d2..e2d2496b with both whole tests, shutdown_bounded, run_in_forked_child, and the executor): CLEAN: all three resolved, no new failure or hang path.

[laughingman7743]

Independent review (relayed), finding 2 (P2, pre-existing, dependency): in fsspec 2026.9.0, fsspec.asyn.reset_after_fork() sets a nonexistent global lock instead of _lock, so a child whose parent forked while another thread held _lock during the first get_loop() deadlocks when it constructs a default AioS3FileSystem.

Verified in the installed fsspec 2026.9.0 (asyn.py:256-260 vs get_lock() at asyn.py:30-38). Already fixed upstream in fsspec/filesystem_spec@cca6715b8e (#2242, 2026-10-07: reset_after_fork() calls reset_lock()), which is not in a release yet (latest tag 2026.9.0). Deferred: no workaround here; the fix arrives with the next fsspec release. The race needs a fork during the first creation of fsspec's IO loop in another thread.

[laughingman7743]

Independent review (relayed), finding 1 (P1, pre-existing): pyathena/filesystem/s3_writer.py:920 — a multipart write session inherited across a fork with a part request in flight keeps that unfinished future in _requests, so commit()/close() and discard() in the child wait forever, through either filesystem; the executor resets neither invalidate the session nor reset its locks.

Verified by reading _S3WriteSession.commit() (waits while self._requests is non-empty). Not introduced here: it is the in-flight case listed as out of scope in the PR body. Deferred to step 2 of #1332, which rejects files inherited across a fork (decision 6); the reviewer's suggestion, rejecting inherited write sessions before taking locks or waiting, including in cleanup, belongs to that design.