Skip to content

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

Merged
Darksonn merged 1 commit into
tokio-rs:masterfrom
M4SS-Code:wakelist-fifo
May 1, 2024
Merged

util: make WakeList::wake_all use FIFO ordering#6521
Darksonn merged 1 commit into
tokio-rs:masterfrom
M4SS-Code:wakelist-fifo

Conversation

@paolobarbolini
Copy link
Copy Markdown
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.

Comment thread tokio/src/util/wake_list.rs Outdated
@Darksonn Darksonn added A-tokio Area: The main tokio crate M-sync Module: tokio/sync labels Apr 28, 2024
Copy link
Copy Markdown
Member

@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.

Comment thread tokio/src/util/wake_list.rs Outdated
Comment thread tokio/src/util/wake_list.rs
Comment thread tokio/src/util/wake_list.rs Outdated
Comment thread tokio/src/util/wake_list.rs Outdated
Comment thread tokio/src/util/wake_list.rs
Copy link
Copy Markdown
Member

@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
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