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

missing disarm timeout monitor after connection refused #8

Closed
wants to merge 1 commit into from

Conversation

bixuehujin
Copy link

This issue will cause the 'Connection timed out' error will also be raised even though a 'Connection refused' error raised before.

@nrk
Copy link
Owner

nrk commented Sep 2, 2013

Thanks @bixuehujin,

the bug is indeed there but a more proper fix would consist in not invoking StreamConnection::disconnect() here since disconnection is handled by StreamConnection::onError(). Invoking StreamConnection::disconnect() two times in a row actually triggers a new connection attempt on the second call (this should be also fixed since it's not really a good idea, but will go with that for now) thus creating a loop.

Do you prefer to rebase your pull request or create a new one with this change, or should I just commit the proper fix myself?

@bixuehujin
Copy link
Author

Thinks, just commit your proper fix.

nrk added a commit that referenced this pull request Sep 2, 2013
Invoking "disconnect()" two times in a row triggers a new connection
attempt on the second call but this will end up in a connection loop.

We should also fix "disconnect()" to make it so that it doesn't try
to fetch a new socket resource when the connection is not actually
active, but we'll leave this to a different commit.

Thanks to @bixuehujin for reporting the issue (see #8).
@nrk
Copy link
Owner

nrk commented Sep 2, 2013

Alright fixed, thanks!

@nrk nrk closed this Sep 2, 2013
@bixuehujin bixuehujin deleted the timeout_monitor_issue branch September 3, 2013 02:39
nrk added a commit that referenced this pull request Sep 4, 2013
This change prevents the underlying connection from creating and opening
a new socket when StreamConnection::disconnect() is invoked on a closed
connection. Partially related to #8.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants