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

krane tempfiles are not properly cleaned up #417

Open
cbgbt opened this issue Dec 12, 2024 · 0 comments · May be fixed by #423 or #420
Open

krane tempfiles are not properly cleaned up #417

cbgbt opened this issue Dec 12, 2024 · 0 comments · May be fixed by #423 or #420
Assignees

Comments

@cbgbt
Copy link
Contributor

cbgbt commented Dec 12, 2024

Issue Description

When using twoliter, my root volume became full due to /tmp usage. It turns out that there were many .tmp directories, each with their own unpacked copy of krane. It seems the twoliter is not properly cleaning up its krane binaries on exit.

After investigating, I believe it's due to the fact that we have a public static reference to a singular Krane struct that we use in twoliter (See here).

The Krane struct uses TempDir to store its binary; however, the tempfile documentation calls out that this will result in resource leaking -- Rust must not be calling the Drop implementation for this static.

tempfile will (almost) never fail to cleanup temporary resources. However TempDir and NamedTempFile will fail if their destructors don’t run. This is because tempfile relies on the OS to cleanup the underlying file, while TempDir and NamedTempFile rely on rust destructors to do so. Destructors may fail to run if the process exits through an unhandled signal interrupt (like SIGINT), or if the instance is declared statically (like with lazy_static), among other possible reasons.

That documentation also says:

When choosing between the temporary file variants, prefer tempfile unless you either need to know the file’s path or to be able to persist it.

We do need to know the filepath to execute the binary that we write and thus must use one of the named options.

So rather than relying on a KRANE static, I think we need to rework the krane-bundle crate and users to create their own instance of Krane that they are guaranteed to drop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants