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

Fix Eventually and EventuallyWithT to always run condition #1653

Closed

Conversation

brackendawson
Copy link
Collaborator

Summary

If Eventually was called with a tick larger than waitFor, or a tick very slightly smaller than waitFor, or very small tick and waitFor values, then a timing race can occur where the select statement falls into the waitFor case first and the condition function is never run.

Changes

  • Add a timeout to the unit test so that a deadlock is found after 1 second rather than 600 seconds.
  • Swap the order that the ticker and timer are created so that in the case that tick is very slightly smaller than waitFor, then the timer will never yield before the ticker.
  • Run condition once before the loop and initialise tick in the for loop to (<-chan time.Time)(nil) so that we always run condition statically, then check its result, then run condition based on ticks.

Motivation

Having a timing race where it is possible for condition to never be run causes our CI tests to flake, and is not how I think the user would expect Eventually can behave.

Related issues

Closes #1652

Regardless of how small tick is compared to waitFor.
@brackendawson
Copy link
Collaborator Author

This might be a dupe of #1427

@brackendawson brackendawson marked this pull request as draft October 4, 2024 17:20
@brackendawson
Copy link
Collaborator Author

The same fix is needed in Never.

@brackendawson
Copy link
Collaborator Author

#1657 is a far better approach I think.

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

Successfully merging this pull request may close these issues.

Eventually can fail having never run condition
1 participant