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

NatsConnection.close unnecessarily sleeps for one second #310

Closed
brettkail-wk opened this issue May 11, 2020 · 5 comments
Closed

NatsConnection.close unnecessarily sleeps for one second #310

brettkail-wk opened this issue May 11, 2020 · 5 comments
Assignees

Comments

@brettkail-wk
Copy link

brettkail-wk commented May 11, 2020

This call to NatsConnectionReader.stop simply sets a flag, but it does not actually interrupt the blocking read, which means that this call to await the stop always times out. Commit bb8968b recently removed the logging that happens when the await fails, but the underlying issue is still present. It seems like these calls to .get should be removed, and these these calls should be updated to use readStop.get and writeStop.get.

@sasbury
Copy link
Contributor

sasbury commented May 29, 2020

Haven't dug into logic, but wanted to make a quick note, in case someone else digs in, but we shouldnt' use get(), there should always be a timeout on these. That said, if we are always reaching the timeout, that is bad.

@brettkail-wk
Copy link
Author

@sasbury The code does use get-with-timeout (twice), but yes, the readStop.get(1, TimeUnit.SECONDS) always times out. Sorry for confusion; I've adjusted the description slightly.

@jglink
Copy link

jglink commented Feb 22, 2021

Any news on this issue?
Is there an estimation on when this will be fixed?
Thanks

@scottf scottf self-assigned this Mar 8, 2021
@scottf
Copy link
Contributor

scottf commented Mar 13, 2021

The read call is blocking. We can actually interrupt the read by telling the underlying socket to shutdown. Is this the desired behavior to basically forcibly stop the read? Here is a PR to review: #427

@scottf
Copy link
Contributor

scottf commented Mar 30, 2021

Closed in #427

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

No branches or pull requests

5 participants