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

Wait for stream writes to stop before closing channels #42

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

pboyd
Copy link

@pboyd pboyd commented Jun 25, 2019

I was getting some test failures under the race detector after calling stream.Close. The goroutine that processes events was writing to the channel that Close closed. It looks like the same problem as #39, so at the very least here's a test case to reproduce that.

I attempted to fix it by using a sync.WaitGroup to keep stream.Close from closing the channel until the other goroutine stopped (stream()). But by itself that just caused a deadlock, because nothing triggered stream() to stop. So I also had to cancel the outbound request.

The non-deprecated way to cancel an HTTP request is by canceling the context, so that's what I did. That was added in Go 1.7, which is perhaps a problem. I can rewrite it to use Request.Cancel if that's more palatable.

pboyd added 2 commits June 24, 2019 22:04
It was sleeping for the retry time after `Close()` was called.
@pboyd
Copy link
Author

pboyd commented Jun 28, 2019

I made one small tweak to this. It was calling stream.retryRestartStream after Close was called. retryRestartStream sleeps before it checks if the connection was closed, so that, combined with the wait in Close, had the effect of making Close take 3s (or whatever stream.retry happens to be).

@perpetual-hydrofoil
Copy link

Bump..

@floodkoff
Copy link

Ping...

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.

3 participants