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

Connection is not released after calling close() #86

Closed
wants to merge 1 commit into from
Closed

Connection is not released after calling close() #86

wants to merge 1 commit into from

Conversation

caucik
Copy link

@caucik caucik commented Dec 27, 2018

Watching the connection using netstat after calling close() doesn't release the connection but keeps it in ESTABLISHED state. Adding [$this->_stream->close();] fixes the issue.

Watching the connection using netstat after calling close() doesn't release the connection but keeps it in ESTABLISHED state. Adding [$this->_stream->close();] fixes the issue.
@mbonneau
Copy link
Member

Calling close will prevent the close message from being sent to the server.

end SHOULD close the connection once the buffer has been flushed.

Would it be possible to create an example WebSocket server and client that demonstrates this issue?

@caucik
Copy link
Author

caucik commented Dec 29, 2018

It seems that the connection can't be closed if the loop is no longer running. I am appending 2 files. In the file close_while_loop_running.php all seems OK. In the file close_after_loop_stopped.php the connections keep piling up. Running the second file with the WebSocket.php modified as suggested fixes this issue.

close.zip

@mbonneau
Copy link
Member

mbonneau commented Jan 1, 2019

@caucik Thank you for providing the example. This did help to demonstrate the issue you are encountering. The proper way to handle this would be to only stop the loop after the connection is closed which only happens after the buffer is flushed.

I do realize that this could cause the connection to hang if the buffer never flushes, but this should be handled with a timeout that forces a close. (see https://github.com/reactphp/stream/blob/50426855f7a77ddf43b9266c22320df5bf6c6ce6/src/WritableStreamInterface.php#L332-L335)

@mbonneau mbonneau closed this Jan 1, 2019
@caucik
Copy link
Author

caucik commented Jan 1, 2019

Thanks. It's obvious that the loop should be closed only after all the connections are closed. But loop is also stopped after the exception is thrown therefore I run $loop->run() inside try block. It would be fine to have option to close the connections in a clean way in the case of exception thrown.

@mbonneau mbonneau mentioned this pull request Jan 1, 2019
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.

None yet

2 participants