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 for resource leak that occurs when parent scope is closed during resource acquisition #2074

Merged
merged 2 commits into from
Oct 9, 2020

Conversation

mpilquist
Copy link
Member

This needs a lot more work but committing what I've found so far. When the resource acquisition completed in CompileScope#acquireResource, the wrapper ScopedResource instance was getting registered on the containing scope, but the containing scope's state was already closed. Not sure why the test I added yesterday passed though.

@mpilquist
Copy link
Member Author

Okay, figured it out. The fix from yesterday was only partial. When the resource acquisition completed, the associated ScopedResource was registered with the current scope, which was already closed. However, that scope was still the current scope at stream termination and hence was closed again, freeing the resource. Adding a subsequent bracket resulted in changing the current scope, and hence there was no double closure.

In this PR, we correctly prevent any resources from being registered on a closed scope. This causes the original test to fail as well as the new variant. We fix by invoking the finalizer if registration fails.

I'd like to change these internals a bit -- e.g., why does a closed scope even have the ability to reference resources -- but I'll save the more drastic changes for develop branch.

@SystemFw
Copy link
Collaborator

SystemFw commented Oct 9, 2020

nice work!

I'd like to change these internals a bit -- e.g., why does a closed scope even have the ability to reference resources -- but I'll save the more drastic changes for develop branch.

👍 👍

@mpilquist mpilquist changed the title Initial sketch of a fix for resource leak that occurs when parent scope is closed during resource acquisition Fix for resource leak that occurs when parent scope is closed during resource acquisition Oct 9, 2020
@mpilquist mpilquist merged commit cecaf46 into typelevel:main Oct 9, 2020
@mpilquist mpilquist deleted the bugfix/2072-take-2 branch February 18, 2024 13:35
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

Successfully merging this pull request may close these issues.

2 participants