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

[FIXED] Repeated timeout of Flush/FlushTimeout and inability to dispatch #322

Merged
merged 1 commit into from
Oct 20, 2017

Conversation

kozlovic
Copy link
Member

If when the client issues a Flush() or FlushTimeout(), the client
library times out when at the same time it receives the PONG, there
is a possibility that the processing of the PONG gets the channel
it needs to notify from the array while the routine waiting on
that notification is done waiting but has not yet removed the
channel from the array. This will lead to processPong() to be
stuck in a chan send, which is blocking the readLopp, which will
ultimately lead to server closing this connection.

The solution is to create a buffererd channel (of size 1). We
should not simply do a select when trying to notify because we
don't want to miss the notification if the routine sending the PING
is not yet in the select to consume the notification.

Resolves #321

If when the client issues a Flush() or FlushTimeout(), the client
library times out when at the same time it receives the PONG, there
is a possibility that the processing of the PONG gets the channel
it needs to notify from the array while the routine waiting on
that notification is done waiting but has not yet removed the
channel from the array. This will lead to processPong() to be
stuck in a chan send, which is blocking the readLopp, which will
ultimately lead to server closing this connection.

The solution is to create a buffererd channel (of size 1). We
should not simply do a `select` when trying to notify because we
don't want to miss the notification if the routine sending the PING
is not yet in the select to consume the notification.

Resolves #321
@coveralls
Copy link

Coverage Status

Coverage increased (+0.006%) to 94.518% when pulling 7239a8e on fix_pong into 4b47946 on master.

@tylertreat
Copy link
Contributor

lgtm

@kozlovic kozlovic requested a review from wallyqs October 17, 2017 15:51
@kozlovic kozlovic merged commit 6f06d34 into master Oct 20, 2017
@kozlovic kozlovic deleted the fix_pong branch October 20, 2017 23:19
TheGhoul21 pushed a commit to TheGhoul21/nats.go that referenced this pull request Jun 20, 2024
Previously, it would attempt to use the host from the url, which
includes the `[]` which `to_socket_addrs()` didn't know how to deal
with. This change strips the `[]`s.

Fixes: nats-io#322

Signed-off-by: Jesse Szwedko <[email protected]>

Co-authored-by: Tomasz Pietrek <[email protected]>
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.

4 participants