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

Memory leak with InfiniteChannel #27

Open
tyranron opened this issue Apr 10, 2018 · 7 comments
Open

Memory leak with InfiniteChannel #27

tyranron opened this issue Apr 10, 2018 · 7 comments
Labels

Comments

@tyranron
Copy link

tyranron commented Apr 10, 2018

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.

@eapache
Copy link
Owner

eapache commented Apr 10, 2018

You are not wrong, this is another caveat. In general this is true of all of the buffered channel implementations here (RingChannel, InfiniteChannel, etc):

If the channel passes out of scope without being explicitly closed, it will leak a goroutine plus whatever values were stored in the channel at the time it went out of scope.

How does that sound?

@tyranron
Copy link
Author

@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.

@eapache
Copy link
Owner

eapache commented Apr 10, 2018

Right, good point.

@michilu
Copy link

michilu commented Nov 1, 2018

How do you think about providing another way such as supports the cancel event of context.Context for return from goroutines?

@tyranron
Copy link
Author

tyranron commented Nov 1, 2018

@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?

@michilu
Copy link

michilu commented Nov 2, 2018

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.
I want do not closing edge channels of input/output each time because keep the channels owned by the worker func.
But It does not work with the Multiplex/Tee funcs.

channels:
edge inputs => Multiplex -> WeakTee => edge outputs

Maybe the WeakTee works well, so it has nothing to do to edge channels of output if closed the input channel.
I must be close all input channels if stop the Multiplex/WeakMultiplex goroutine, otherwise leak goroutines of the Multiplex func.

I think to it useful to supports context.Context for this countermeasure.

signature:
Multiplex(ctx context.Context, output SimpleInChannel, inputs ...SimpleOutChannel)

ctx, cancel := context.WithCancel(context.Background())
defer cancel()
Multiplex(ctx, output, inputs)

@tyranron
Copy link
Author

tyranron commented Nov 2, 2018

@michilu yup, now I see... polluting channels with context.Context will give an ability to cancel inner runners of such channels. This definitely should work.

@eapache eapache added the bug label Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants