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

Finalizers are not called on thread exit #12192

Open
endragor opened this issue Sep 15, 2019 · 5 comments
Open

Finalizers are not called on thread exit #12192

endragor opened this issue Sep 15, 2019 · 5 comments

Comments

@endragor
Copy link
Contributor

When a thread exits, Nim simply deallocates GC heap without running finalizers. This results in resource leaking if the finalizers are supposed to free some foreign resources.

Finalizers must be called when:

  1. A thread exits.
  2. tearDownForeignThreadGc is called.
@Araq
Copy link
Member

Araq commented Jun 7, 2020

This is possible via deallocHeap.

@Araq Araq closed this as completed Jun 7, 2020
@timotheecour
Copy link
Member

timotheecour commented Jun 8, 2020

@Araq wait but why wouldn't it be by default called automatically, eg when thread exits?

@Araq
Copy link
Member

Araq commented Jun 8, 2020

Days have only 24 hours and finalizers are "old runtime" cruft, my focus is on ARC that doesn't have this problem.

@endragor
Copy link
Contributor Author

endragor commented Jun 8, 2020

I guess the question was: isn't fixing this a matter of replacing deallocOsPages with deallocHeap(allowGcAfterwards = false) within the threads module?

This bug causes leaks for those who use threads, and it's not obvious you should call deallocHeap on your own. Even if you don't use finalizers, you may use a library that implements them, which makes the issue even more subtle.

@Araq
Copy link
Member

Araq commented Jun 8, 2020

Fine...

@Araq Araq reopened this Jun 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants