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 worker are notified to stop with non_graceful shutdown #333

Merged
merged 6 commits into from
Apr 15, 2021

Conversation

fakeshadow
Copy link
Contributor

@fakeshadow fakeshadow commented Apr 15, 2021

PR Type

Bug Fix

PR Checklist

Check your PR fulfills the following:

  • A changelog entry has been made for the appropriate packages.
  • Format code with the latest stable rustfmt

Overview

With non graceful shutdown there is no signal for notify worker threads to exit. This could result in leak of worker threads.

This PR changes to notify worker threads to shutdown on all cases. But only in graceful shutdown case server builer wait for worker shutdown response.

@fakeshadow
Copy link
Contributor Author

This changes the behavior of server as force shutdown was not exiting worker threads. There could be people using it as a feature rather than bug.


if exit {
sleep(Duration::from_millis(300)).await;
System::current().stop();
Copy link
Member

Choose a reason for hiding this comment

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

should we code this as try_current ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right now we still expect system when running a server so I think it's fine here.
We can change when actix-rt is made optional.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The thing I have no idea is why there is 300 ms delay.

@robjtede robjtede merged commit aeb81ad into master Apr 15, 2021
@robjtede robjtede deleted the fix/non_graceful_shutdown branch April 15, 2021 23:54
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.

2 participants