Skip to content

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