Fix leaking main thread held memory #1857
Conversation
- Release such memory explicitly by `destroy`ing
the main thread on threading module termination
- Remove explicit _tlsRanges.popBack call as now _tlsRanges will have already been released in that case via
core.thread.thread_term ->
~Thread on the main thread ->
rt.tlsgc.destroy ->
rt.sections.finiTLSRanges
|
Thanks for your pull request, @Calrama! We are looking forward to reviewing it, and you should be hearing from a maintainer soon. Some tips to help speed things up:
Bear in mind that large or tricky changes may require multiple rounds of review and revision. Please see CONTRIBUTING.md for more information. Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. |
| @@ -461,8 +461,6 @@ extern(C) void _d_dso_registry(CompilerDSOData* data) | |||
| else | |||
| { | |||
| // static DSOs are unloaded in reverse order | |||
| assert(pdso._tlsSize == _tlsRanges.back.length); | |||
| _tlsRanges.popBack(); | |||
There was a problem hiding this comment.
Just to check - this is called once per the executable, not once per thread, right? Meaning, _tlsRanges will have just one element.
There was a problem hiding this comment.
Before this PR, yes. With this PR _tlsRanges will have no elements at that time (and the .popBack will thus result in an AssertError, which is why I removed it), because it was already .cleared via thread_term (as per PR description).
There was a problem hiding this comment.
Yes, just wanted to confirm that!
|
Just FYI, since I saw this mentioned on the newsgroup :). I'm back from a holiday next week, and will give this a more detailed look then, if nobody does by then. It looked fine to me last time I checked. |
|
@Burgos, do you have time to give this PR a more detailed look? |
Yes, indeeed. LGTM! |
ghost
mentioned this pull request
Solves the problem described in #1557 (which got stalled)
destroyingthe main thread on threading module termination
rt.sections_elf_shared.finiTLSRangesalready contains the necessary logic toreset_tlsRangesto zero length in the notSharedversion, it just isn't used; instead the_tlsRanges(which only have one element) are cleared by calling_tlsRanges.popBackin_d_dso_registry.This PR removes that
popBackcall and instead uses the preexisting logic infiniTLSRangesviacore.thread.thread_term -> ~Thread on the main thread -> rt.tlsgc.destroy -> rt.sections.finiTLSRanges