Skip to content

fix(executor): support destruction from worker threads - #233

Open
mrdrivingduck wants to merge 1 commit into
apache:mainfrom
mrdrivingduck:codex/fix_default_executor_shutdown
Open

fix(executor): support destruction from worker threads#233
mrdrivingduck wants to merge 1 commit into
apache:mainfrom
mrdrivingduck:codex/fix_default_executor_shutdown

Conversation

@mrdrivingduck

Copy link
Copy Markdown
Contributor

What does this PR do?

While testing the OSS filesystem asynchronous read path, we found a lifecycle issue in DefaultExecutor. The issue is not OSS-specific: S3 asynchronous reads use the same ownership pattern and can trigger it as well.

The sequence is:

  1. An asynchronous object-store request submits a task that captures a shared_ptr to its client.
  2. The caller releases its client reference.
  3. A worker completes the task and releases the final client reference.
  4. The client and its executor are then destroyed on that worker thread.
  5. Executor shutdown tries to join() the current worker thread.

Without this fix, joining the current thread throws std::system_error with Resource deadlock avoided. Since this happens during destruction, it can terminate the process.

This PR separates the executor's shared scheduling state from the executor object. Workers retain the shared state rather than accessing the executor through this. When destruction happens on a worker, shutdown stops the shared state, joins the other workers, and detaches the current one. The current worker then finishes its task and exits normally.

The PR adds a generic executor regression test and an S3 asynchronous range-read test covering this lifecycle.

@mrdrivingduck
mrdrivingduck force-pushed the codex/fix_default_executor_shutdown branch from 29b72fb to 9d7ba65 Compare August 21, 2026 06:04
@mrdrivingduck
mrdrivingduck marked this pull request as ready for review August 21, 2026 07:00
Decouple worker state from the executor object so that a task can
safely destroy its final executor owner from a worker thread.

Cover the lifecycle with executor and S3 asynchronous
range-read tests.

Co-authored-by: GPT-5.6 Terra <codex@users.noreply.github.com>
@mrdrivingduck
mrdrivingduck force-pushed the codex/fix_default_executor_shutdown branch from 9d7ba65 to c8ae8cb Compare August 22, 2026 04:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant