-
Notifications
You must be signed in to change notification settings - Fork 68
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
Memory leak with InfiniteChannel #27
Comments
You are not wrong, this is another caveat. In general this is true of all of the buffered channel implementations here (RingChannel, InfiniteChannel, etc):
How does that sound? |
@eapache sounds OK, but sometimes (as in my example above) just closing channel is not enough, we also need to read all the values from internal buffer. |
Right, good point. |
How do you think about providing another way such as supports the cancel event of |
@michilu I can barely imagine how that can be applied in this situation. Would you be so kind to provide a sort of sketch/pseudocode to illustrate your idea? |
The inner goroutine returns if a case of closing all input channels. This is other use cases, I try implements the Multiple-Input/Multiple-Output now. channels: Maybe the WeakTee works well, so it has nothing to do to edge channels of output if closed the input channel. I think to it useful to supports signature: ctx, cancel := context.WithCancel(context.Background())
defer cancel()
Multiplex(ctx, output, inputs) |
@michilu yup, now I see... polluting channels with |
If I understand correctly, the goroutine
ch.infiniteBuffer()
persists in memory until all the values from inner buffer are drained.So, in situation where there are many writes, but only few reads, and then both writer/reader are done with a channel (channel goes out of their scopes), the channel won't be garbage collected as the goroutine
ch.infiniteBuffer()
still runs (and we have no runtime support for our lib as for native Go channels). Which in certain circumstances (and naive usage) may lead to memory leaks.@eapache please, correct me if I'm wrong. If not, maybe this fact needs additional mention in docs?
There is already related one, but it points to another caveat.
The text was updated successfully, but these errors were encountered: