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

Mark all functions that use wait as noasync #3168

Merged
merged 26 commits into from May 10, 2024
Merged

Mark all functions that use wait as noasync #3168

merged 26 commits into from May 10, 2024

Conversation

0xTim
Copy link
Member

@0xTim 0xTim commented Apr 4, 2024

These changes are now available in 4.98.0

⚠️ WARNING: If you have strict concurrency checking enabled you should migrate to the async Application.make()

NIO's EventLoopFuture.wait() is marked as noasync because is can cause issues when used in a concurrency context. All places where we call .wait() should also be marked as noasync to avoid this issue.

This adds async alternatives for those functions and adds noasync annotations where appropriate.

Also adds an async Application.make to replace the old initialiser that is now noasync

@0xTim
Copy link
Member Author

0xTim commented Apr 4, 2024

This is reliant on #3167

Todo:

  • Migrate the commands to AsyncCommands
  • Set up tests to ensure we don't add any more back
  • Make NIO the global executor (this may need to be done in the template, still to investigate)

@0xTim 0xTim added the semver-minor Contains new API label Apr 4, 2024
@0xTim 0xTim marked this pull request as ready for review April 24, 2024 01:09
@0xTim 0xTim requested a review from gwynne as a code owner April 24, 2024 01:09
@0xTim
Copy link
Member Author

0xTim commented Apr 29, 2024

Test failures are expected due to the use of strict concurrency

@0xTim
Copy link
Member Author

0xTim commented May 9, 2024

@gwynne I think this is ready for review now, should be non-breaking

@0xTim 0xTim merged commit d411635 into main May 10, 2024
14 of 16 checks passed
@0xTim 0xTim deleted the noasync-waits branch May 10, 2024 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-minor Contains new API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash after installing posix ELG as concurrency executor
2 participants