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

util: make WakeList::wake_all use FIFO ordering #6521

Merged
merged 1 commit into from
May 1, 2024
Merged

util: make WakeList::wake_all use FIFO ordering #6521

merged 1 commit into from
May 1, 2024

Conversation

paolobarbolini
Copy link
Contributor

@paolobarbolini paolobarbolini commented Apr 28, 2024

Motivation

#6517 (comment) noted that the current implementation of WakeList::wake_all is LIFO, while the original pre-WakeList implementations were FIFO. The very first implementation of WakeList was FIFO, but it also was also unsound in the case the Waker panicked, so it was changed to work as it is currently implemented.

Solution

This changes WakeList to be FIFO by processing Wakers in order and using a DropGuard to handle panics.

@Darksonn Darksonn added A-tokio Area: The main tokio crate M-sync Module: tokio/sync labels Apr 28, 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.

Looks correct, but I have some comments on the safety comments.

tokio/src/util/wake_list.rs Outdated Show resolved Hide resolved
tokio/src/util/wake_list.rs Outdated Show resolved Hide resolved
tokio/src/util/wake_list.rs Outdated Show resolved Hide resolved
tokio/src/util/wake_list.rs Outdated Show resolved Hide resolved
tokio/src/util/wake_list.rs Show resolved Hide resolved
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.

Thanks.

@Darksonn Darksonn enabled auto-merge (squash) May 1, 2024 14:05
@Darksonn Darksonn merged commit e971a5e into tokio-rs:master May 1, 2024
76 checks passed
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-sync Module: tokio/sync
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants