-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
sync: add tokio_util::sync::TaskTracker
#6033
Conversation
This was a hack for something I removed.
tokio-util/src/sync/task_tracker.rs
Outdated
pub fn new() -> Self { | ||
Self { | ||
inner: Arc::new(TaskTrackerInner::new()), | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A thought I had: I could rename this to new_open
and add a new_closed
. This might help prevent mistakes where you forget to call close
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall, this looks very nice! we've previously written a somewhat similar utility in the drain
crate, which essentially provides a combination TaskTracker
and CancellationToken
, so it's nice to have this as part of tokio-util
.
i commented on a few minor nits and suggestions, but overall it looks great!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! I left a few minor docs suggestions, but I feel good about the implementation overall.
@hawkw Thoughts about the terminology discussed in #6033 (comment)? |
error: this function has an empty `#[must_use]` attribute, but returns a type already marked as `#[must_use]` 60 --> tokio-util/src/task/task_tracker.rs:591:5 61 | 62 591 | fn clone(&self) -> TaskTrackerToken { 63 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 64 | 65
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
take it or leave it, but i removed some usage of the second-person perspective. personally, i prefer to avoid the second-person perspective in rustdocs, but it's your call.
The page will be updated to use `TaskTracker` after this is released.
On our topic page for graceful shutdown, we currently recommend using the close flag of an mpsc channel to wait for things to shut down. That's pretty terrible. Let's add a dedicated utility for this.
Rendered docs
Closes: #5585