Skip to content

honor cancel() issued before spin_until_future_complete().#3174

Open
fujitatomoya wants to merge 1 commit into
rollingfrom
issues/3172
Open

honor cancel() issued before spin_until_future_complete().#3174
fujitatomoya wants to merge 1 commit into
rollingfrom
issues/3172

Conversation

@fujitatomoya

Copy link
Copy Markdown
Collaborator

Description

Fixes #3172

Is this user-facing behavior change?

Yes, but it only gets better.

Did you use Generative AI?

Yes, Claude Opus 4.7 partially, especially on test cases.

Additional Information

Signed-off-by: Tomoya Fujita <tomoya.fujita825@gmail.com>
@fujitatomoya fujitatomoya requested a review from skyegalaxy June 11, 2026 05:18
@fujitatomoya fujitatomoya self-assigned this Jun 11, 2026

@jmachowinski jmachowinski left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also change the while loops in the spins, to only react to the cancel_request

// bail out early instead of re-arming spinning and blocking forever in
// wait_for_work().
cancel_requested_.store(true);
spinning.store(false);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is now wrong.
The cancel should not touch the spinning at all, it shall be only controlled by the spin functions.

This would also fix a bug, that is_spinning would return false, directly even after the cancel, even though the executor might still be doing stuff.

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.

Race condition in executor when canceling before spin

2 participants