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

Problematic shutdown when using ActionController::Live #938

Closed
vandrijevik opened this issue Mar 30, 2016 · 7 comments
Closed

Problematic shutdown when using ActionController::Live #938

vandrijevik opened this issue Mar 30, 2016 · 7 comments

Comments

@vandrijevik
Copy link

Hello!

I am trying to use ActionController::Live in Rails to implement an application which uses server-sent events to send data to the browser. I find that doing so breaks the shutdown procedure for Puma (which never shuts down while there is a connection open from a browser), and I believe the problem is with Puma’s waiting on threads which serve connections to finish.

I have created a sample app, located at https://github.com/vandrijevik/sse_relay/ to demonstrate the issue, and compare it with using Rainbows!, which does what I would expect. The README in that repo contains instructions on how to replicate the problem, so I am not copying them here.

There are a few related issues in this repository:

and in particular, back in 2013 @evanphx asked:

Puma does wait for requests to end before quitting, so it's possible that the streaming requests are keeping it alive. Would you like puma to just quit and terminate the streams?

In this case yes, that is exactly what I expect to happen.

I opted to create a new issue since I could not clearly understand the specific problem from the above-mentioned ones, and none of them seemed to include code suitable for easy reproduction.

There is a potentially related issue in Rails as well, rails/rails#10989. That issue is from 2013, so it is quite stale by now, but since it is currently open, and no one answered @tenderlove’s ask for a sample application, I am cross-referencing it here in the hopes that this may be helpful.

For reference, I am using:

Version 3.2.0 (ruby 2.2.4-p230), codename: Spring Is A Heliocentric Viewpoint

Thanks for considering this!

@evanphx
Copy link
Member

evanphx commented Apr 5, 2016

Yes, this an existing issue that I haven't figured out the right way to fix. Puma wants to wait until all the requests are done before shutting down but the Live is just sitting there waiting and thus never shuts down. Part of the issue is that Puma doesn't know that request is Live-style connection.

@vandrijevik
Copy link
Author

Thanks for confirming @evanphx, I suspected as much!

Would you be open to making this behavior:

Puma wants to wait until all the requests are done before shutting down

configurable? Perhaps we could set a boolean like wait_on_threads_before_shutdown, or an integer like thread_wait_time_on_shutdown, which would let us get Rainbows!-like behavior, where a server shutdown doesn’t wait on its threads (or waits up to a configured value).

I think this would skirt the issue of Puma having to somehow detect that we expect long-lived requests (because we’d be responsible for setting the setting), but it would allow us to use Puma in these scenarios also.

@evanphx
Copy link
Member

evanphx commented Apr 5, 2016

I can certainly give you an option to perform an unclean shutdown, sure. Perhaps force_shutdown_after option that takes the number of seconds to wait for the threads to finish. -1 would be the current behavior, namely wait forever. 0 would mean "never wait, just exit" and any positive number would just start a time and exit! when done.

Would that work for you?

@vandrijevik
Copy link
Author

@evanphx I think that would work well! However, would it be possible to just kill the threads (thus unblocking the shutdown procedure which waits on them, and allowing it to continue) instead of calling exit!?

I think that would cause less compatibility trouble (if someone has exit handlers they could still run, just the HTTP-serving threads would get killed abruptly), but I am not very familiar with Puma’s code in this area, so sorry if this is a dumb question!

@evanphx
Copy link
Member

evanphx commented Apr 7, 2016

@vandrijevik I can have it try to kill the threads, though I still consider that a dicey move because it introduces the equivalent of raising an exception in a random place, and it doesn't work if the code is blocked in some C code (sometimes).

@vandrijevik
Copy link
Author

@evanphx Ah, good point! I was just wondering how other servers do this, and from this it seems that Rainbows! (which I’ve been using to compare), does exit! just like you proposed above.

So that would be great after force_shutdown_after without any thread killing business! :)

@evanphx evanphx closed this as completed in 448c022 Apr 7, 2016
@vandrijevik
Copy link
Author

Thank you @evanphx! 😄

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

No branches or pull requests

2 participants