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
Unblock the accept thread on shutdown #120
Conversation
This sounds good to me! Looks like there are merge conflicts though |
6992c74
to
a7a8c6f
Compare
// Connect briefly to ourselves to unblock the accept thread | ||
let maybe_stream = TcpStream::connect(self.listening_addr); | ||
if let Ok(stream) = maybe_stream { | ||
stream.shutdown(Shutdown::Both); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be let _ = stream.shutdown(Shutdown::Both);
, then we can get rid of #[allow(unused_must_use)]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah! right, thanks, much better. Done.
a7a8c6f
to
cde7981
Compare
@frewsxcv ok rebased with master and addressed your comment, let me know if there's anything else to be done. Thanks. |
Thanks! @aelita-mergebot r+ |
😱 Internal error: no commit found for PR |
@aelita-mergebot r+ |
Merge #120 a=@moises-silva r=@frewsxcv ________________________________________________________________________ I'd like to find a better way, but, this works and is not completely awful, is it? :) If this is not acceptable I'd like to initiate a discussion on ideas on how to handle shutdown correctly. Currently if tiny-http is used in a loadable module (I am using it inside a project that can load/unload modules from an interactive command line a system administrator controls), when the server goes out of scope the socket lingers because the thread never terminates unless there's a new tcp connection received (accept is a blocking call). An alternative to this would be to put the socket in non-blocking mode and then somehow poll() or select() on it with a timeout to check every sec for the shutdown flag (close trigger bool). I may be wrong, but I don't see many downsides to this implementation. The Drop() call *could* hang for a bit on unusual situations like the server being inaccessible to itself (e.g 127 -> 127 firewall blocking or whatever), but the connection will timeout and move on. Most cases are better off with this as far as I can tell. Thoughts?
I'd like to find a better way, but, this works and is not completely awful, is it? :)
If this is not acceptable I'd like to initiate a discussion on ideas on how to handle shutdown correctly. Currently if tiny-http is used in a loadable module (I am using it inside a project that can load/unload modules from an interactive command line a system administrator controls), when the server goes out of scope the socket lingers because the thread never terminates unless there's a new tcp connection received (accept is a blocking call).
An alternative to this would be to put the socket in non-blocking mode and then somehow poll() or select() on it with a timeout to check every sec for the shutdown flag (close trigger bool).
I may be wrong, but I don't see many downsides to this implementation. The Drop() call could hang for a bit on unusual situations like the server being inaccessible to itself (e.g 127 -> 127 firewall blocking or whatever), but the connection will timeout and move on. Most cases are better off with this as far as I can tell.
Thoughts?