Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

runtime: fix double-increment of num_idle_threads when shutdown #6440

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

liuq19
Copy link

@liuq19 liuq19 commented Mar 29, 2024

Motivation

Fix #6439

Solution

Adjust the order of code and make the count of num_idle_threads correct

@mox692 mox692 added M-runtime Module: tokio/runtime M-metrics Module: tokio/runtime/metrics A-tokio Area: The main tokio crate labels Mar 29, 2024
Copy link
Contributor

@Darksonn Darksonn left a comment

Choose a reason for hiding this comment

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

I'm not completely sure what your code is doing. I wonder if it could be simplified.

Adding a test that would catch the issue would be great as well.

Comment on lines +530 to +533
// Work was produced, and we "took" it (by decrementing num_notify).
// This means that num_idle was decremented once for our wakeup.
// But, since we are exiting, we need to "undo" that, as we'll stay idle.
self.metrics.inc_num_idle_threads();
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment is confusing to me. Where did we decrement num_notify?

I guess what's happening here is that when waking is up, the counter is decremented, so this increment is there to counteract that.

But that's not what the comment says. Also, what if wait_timeout returned due to the timeout triggering, rather than due to us being woken by another thread?

@Darksonn
Copy link
Contributor

Any update on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate M-metrics Module: tokio/runtime/metrics M-runtime Module: tokio/runtime
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BlockingPool num_idle_threads is wrongly double-increased when shutting down
3 participants