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

Apparent memory leak of spawned tasks #38

Closed
jebrosen opened this issue Apr 23, 2020 · 6 comments
Closed

Apparent memory leak of spawned tasks #38

jebrosen opened this issue Apr 23, 2020 · 6 comments

Comments

@jebrosen
Copy link

I believe there is some form of memory leak, and this was discussed some in the Discord channel #tokio-users recently. This graph should illustrate it pretty well:

Memory leak graph

The orange 6.2MB are a single allocation by tracing-subscriber, the green are RawTasks that are allocated via spawn, and the blue are something in broadcast. The workload is just running while true; do target/release/cli get k; done for a while (perhaps after setting a value for k, but I believe it doesn't actually matter).

As far as I can tell, the spawned tasks have actually completed. A different implementation (using watch instead of broadcast) fixes it. Another project using broadcast does not have a similar issue. My hunch is that there is some kind of reference cycle, maybe in part because the shutdown coordination is bidirectional, but I have not looked more closely yet.

@Darksonn
Copy link
Collaborator

If you modify Tokio to print when Arc<WaitNode> is created and dropped inside broadcast, you will see that it creates one for every connection, but none of them are dropped until the application closes.

@carllerche
Copy link
Member

Thanks for the report. I will try to look later today. My guess, w/o looking, is that it is a similar problem as the old semaphore had. If that is a case, it will need a similar rework as tokio-rs/tokio#2325

@hawkw
Copy link
Member

hawkw commented Apr 23, 2020

@carllerche do you think broadcast can easily be modified to use the semaphore?

@carllerche
Copy link
Member

Open PR here: tokio-rs/tokio#2509

@carllerche
Copy link
Member

I have been running the reproduction using the PR linked above and memory is stable for me.

@carllerche
Copy link
Member

Fixed w/ the tokio release.

Darksonn added a commit that referenced this issue May 15, 2020
As Tokio version 0.2.21 contains a fix in the broadcast channel that
removes a memory leak in mini-redis, I don't think 0.2.20 should be
considered the minimum supported version, even if the codebase compiles
with that version.

Refs: #38
carllerche pushed a commit that referenced this issue May 15, 2020
As Tokio version 0.2.21 contains a fix in the broadcast channel that
removes a memory leak in mini-redis, I don't think 0.2.20 should be
considered the minimum supported version, even if the codebase compiles
with that version.

Refs: #38
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

No branches or pull requests

4 participants