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

Fix: Cleanup nodes in Thread::LinkedList(T)#delete #15295

Merged

Conversation

ysbaddaden
Copy link
Contributor

@ysbaddaden ysbaddaden commented Dec 20, 2024

We internally use Thread::LinkedList(T) to keep a list of all threads & fibers, so the GC won't collect them and know all the stacks to scan during a GC collection. I noticed that #delete doesn't cleanup the @previous and @next links of the T objects.

When we remove the thread or fiber from the list, it means they have terminated and the objects should get collected at some point, so I guess cleanup isn't very important. On the other hand, there isn't much harm to cleanup a couple pointers (quite the opposite).

In fact, while investigating a memory leak with PerfTools::MemProf.pretty_log_object_graph(io, Fiber) I noticed a number of Fiber objects kept references to the same set of other fibers, while with proper cleanup, the graph of objects remained clean & stable, maybe because the Fiber objects disappear much more quickly.

@straight-shoota straight-shoota added this to the 1.15.0 milestone Dec 20, 2024
@straight-shoota straight-shoota added kind:bug A bug in the code. Does not apply to documentation, specs, etc. and removed kind:refactor labels Dec 23, 2024
@straight-shoota straight-shoota merged commit 47b948f into crystal-lang:master Dec 23, 2024
69 checks passed
@straight-shoota straight-shoota changed the title Fix: cleanup T on Thread::LinkedList(T)#delete Fix: Cleanup nodes in Thread::LinkedList(T)#delete Dec 23, 2024
@ysbaddaden ysbaddaden deleted the fix/thread-linked-list-delete branch December 23, 2024 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:runtime
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants