channel: don't leak go routine on close #130
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
uhhggg, that lint means well but.. |
|
heh, sorry for uh... shall we say... aggressive linters :) its to keep me from getting off the rails! thanks for opening this and the context! looks good and sounds good and will check it out this weekend when I get a sec to process (seems totally reasonable just wanna make sure I have time to fully grok it!)! (and I can appease the silly linter when I get to it this weekend!) |
|
dunno whats going on with actions, but thats a problem for later :) thanks again for this! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The channel close has some timeout logic for block reads. If the timeout is hit (the `<-time.After(c.ReadDelay * (c.ReadDelay / readDelayDivisor) branch) then the go routine will block forever.
This changes the channel send to a
closewhich will not block. Closed go routines will always send the zero value on any receivers which will do the right thing in both cases.I am pretty sure there are other goroutine leaks in closing connections as well.
c.doneis used to signal closing of the channel but on theread()method there is a good chance the connection is blocking on c.Read() and soc.doneis probably not going to be read:func (c *Channel) read() {.
transport.Read()probably won't not block until closed, so the order of operation is backwards, but I haven't had a chance to dig into it and probably wont' be but something to look/test for.