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

Process timers set for exactly now #73

Merged
merged 1 commit into from
Nov 21, 2021
Merged

Conversation

sporksmith
Copy link
Contributor

@sporksmith sporksmith commented Nov 19, 2021

This fixes a bug wherein the first timer set for exactly now ends up
in the pending list instead of the ready list, eventually resulting
in polling with a timeout of 0.

Under normal circumstances this bug would trigger very rarely, and when
it did would only result in one spurious "loop", since the next time the
timers are checked, time will have advanced.

However, this bug can cause misbehavior and deadlock in emulated
environments. e.g., in the Shadow emulator, time only moves forward when
a blocking syscall is performed, so this bug causes deadlock:
https://gitlab.torproject.org/tpo/core/arti/-/issues/174#note_2762399

This fixes a bug wherein the first timer set for exactly `now` ends up
in the `pending` list instead of the `ready` list, eventually resulting
in polling with a timeout of 0.

Under normal circumstances this bug would trigger very rarely, and when
it did would only result in one spurious "loop", since the next time the
timers are checked, time will have advanced.

However, this bug can cause misbehavior and deadlock in emulated
environments. e.g., in the Shadow emulator, time only moves forward when
a blocking syscall is performed, so this bug causes deadlock:
https://gitlab.torproject.org/tpo/core/arti/-/issues/174#note_2762399
Copy link
Collaborator

@taiki-e taiki-e left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@taiki-e taiki-e merged commit aa0d0c8 into smol-rs:master Nov 21, 2021
@sporksmith
Copy link
Contributor Author

Thanks for the quick review and merge!

@sporksmith
Copy link
Contributor Author

Hi, is it possible to get a release, so that we can avoid a git-based dependency?

@taiki-e
Copy link
Collaborator

taiki-e commented May 26, 2022

Published in 1.7.0.
Thanks for the ping on this, I had forgotten that there's an unreleased change.

@sporksmith
Copy link
Contributor Author

Published in 1.7.0.
Thanks for the ping on this, I had forgotten that there's an unreleased change.

Great, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants