Detecting client disconnection #1373
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When the stream is used for SSE server which works via chunked content provider it's not possible to check connectivity over writability because it's wrapped by DataSink. It could make the server stalled, when the provider wants to only keep connection without send data and certain amount of clients consumes entire threadpool.
SocketStream::is_writable() should return false if peer is disconnected.
DataSink could be unwritable if it's backed by connection based. Because it could be disconnected by client right after enter centent provider.
gh4ck3r
changed the title
|
@gh4ck3r, thanks for the fine contribution! |
ExclusiveOrange
pushed a commit
to ExclusiveOrange/cpp-httplib-exor
that referenced
this pull request
May 2, 2023
* SocketStream need to check connectivity for writability. When the stream is used for SSE server which works via chunked content provider it's not possible to check connectivity over writability because it's wrapped by DataSink. It could make the server stalled, when the provider wants to only keep connection without send data and certain amount of clients consumes entire threadpool. * add unittest for SocketStream::is_writable() SocketStream::is_writable() should return false if peer is disconnected. * revise broken unittest ServerTest.ClientStop DataSink could be unwritable if it's backed by connection based. Because it could be disconnected by client right after enter centent provider. Co-authored-by: Changbin Park <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As described in #1370, Server requires to detect client disconnection if it offers Server-Sent-Event. Unless, it possibly stall the server in certain circumstance. Because if "chunked content provider" which is a component to offer SSE wants to keep connection without sending data, it'll hold given thread as long as given
DataSinkis "writable". TheDataSink, wrapper ofSocketStream, uses underlyingis_writable()to check it's "writable" of not. But,SocketStream::is_writable()returns true even after disconnection of peer. That means "chunked content provider" can't aware disconnection of client without writing anything.In this PR,
is_writable()ofSocketStreamandSSLSocketStreamreturns false if socket is disconnected.I'll be better to detect half-close status. But, it's not covered here.
This is worked from @AhnLab-OSSG