RFC: Add remove_atexit keyword argument to mktempdir(parent)

[DilumAluthge]

Description

This pull request adds a remove_atexit keyword argument to the mktempdir(parent) method.

Old method signature: mktempdir(parent=tempdir(); prefix=$(repr(temp_prefix)))

New method signature: mktempdir(parent=tempdir(); prefix=$(repr(temp_prefix)), remove_atexit=false)

If remove_atexit is true, then we add an atexit exit hook that will remove the temporary directory when Julia exits.

If remove_atexit is false (default), then the current behavior is preserved. Therefore, this change is backwards compatible.

Rationale

Currently, you can use the mktempdir(fn::Function, parent=tempdir(); prefix=temp_prefix) method to create a temporary directory, apply the function fn, and then remove the temporary directory. However, this requires that you have a function fn that contains all of the code that you want to run while the temporary directory is available. You may instead want to create a temporary directory, keep it available for the entire duration of the Julia program, and only delete it when Julia exits. Currently, if you want this functionality, you have to create the atexit exit hook yourself. This pull request makes this easier by creating the exit hook for you.

[DilumAluthge]

Done!

[ararslan]

Seems okay to me in general, but I think a stronger test would be nice. Something like generating a temp directory name in the main process, spinning up a child process that does mkdir(thing_from_parent, remove_on_exit=true), then @testing in the main process that the directory doesn't exist.

[DilumAluthge]

Done. I run a child process that (1) creates a temp directory, (2) tests that it exists (throwing an error if it does not), and (3) prints the name of the temp directory to STDOUT. The parent process runs the child process, reads the temp directory from STDOUT, and then tests that it no longer exists.

[vtjnash]

Neat, I like this idea!

Maybe in a followup-PR we should do it for open and mktemp also (which could try to map to FILE_FLAG_DELETE_ON_CLOSE / O_TMPFILE, where applicable)?

[kshyatt]

Can this be merged?

[JeffBezanson]

Note from triage: would be nice to synchronize terminology more and not use both "at exit" and "on exit". The argument could be called atexit = true or remove_atexit = true ?

[DilumAluthge]

Done. I've renamed remove_on_exit to remove_atexit, and I've put the description of the remove_atexit keyword argument under a !!! compat "Julia 1.3" heading.

[StefanKarpinski]

Triage is kind of stuck on this one on account of two things:

1. This is such a nasty keyword argument name: it mixes using an underscore for a space with not using an underscore for a space in the same name; also, it's just long and kind of awkward.

2. That might be fine, but as has been noted, we should have similar functionality on a number of other functions that create temporary files or directories that might be removed.

These two things seem to combine to lead to a near future where we have a proliferation of functions with one of the most unfortunate keyword argument names I've ever found. Triage spent a significant time trying to come up with something better but we couldn't come up with anything.

[ararslan]

Related to an idea Stefan had, we could make the keyword just called atexit, which would default to nothing (no action at exit) but could alternativly be a function, e.g. atexit=rm or atexit=x->rm(x, force=true, recursive=true) or atexit=print("hi"), that would be run at exit.

[StefanKarpinski]

And if we had rmrf as an abbreviation for p -> rm(p, recursive=true, force=true) then this could be written as mktempdir(..., atexit=rmrf) which is not too bad. Jeff's objection was "what else would you pass as an atexit value besides nothing or rmrf?

[DilumAluthge]

How about something short like cleanup? The default value is cleanup = false. If cleanup = false, do nothing. If cleanup = true, then delete the directory on exit.

[DilumAluthge]

Although, if #32831 gets merged, then atexit = rmrf is pretty nice too!

[DilumAluthge]

Jeff's objection was "what else would you pass as an atexit value besides nothing or rmrf?

That is a pretty good point.

[StefanKarpinski]

I would be ok with cleanup=true as well. That's a pretty clean name (see what I did there?).

[ararslan]

My only issue with cleanup is that it doesn't really tell you what it does or when it happens.

[StefanKarpinski]

True. At least with mktempdir(path, atexit=rmrf) you can guess what will happen—rmrf(path)—and when—atexit.

[DilumAluthge]

My only issue with cleanup is that it doesn't really tell you what it does or when it happens.

True. And frankly, mktempdir(path, cleanup=true) and mktempdir(path, atexit=rmrf) are basically the same length. So we might as well use the latter, since it is more informative.

[DilumAluthge]

Let me know if/when you want to go with the mktempdir(path, atexit=rmrf) approach, and I'll make the changes to the code!

Specifically:

• If atexit is nothing, we'll do nothing.
• Else, we'll add an exit hook that calls atexit(path)

[JeffBezanson]

atexit=rmrf does read nicely. It's just so flexible... atexit = println ∘ powerset ∘ readdir 😂 I don't know, I'm trying to think of the most absurd possible function to pass there. Any other contestants?

[StefanKarpinski]

f(path) = atexit(()->f(path))
mktempdir(path, atexit=f)

[StefanKarpinski]

Here's a radical idea: always do this, unconditionally. I.e. don't provide an option, just always set an atexit cleanup handler any time we create a temp file or directory and the closure form isn't used.

[StefanKarpinski]

I realize that might be a slight performance issue since adding a lot of atexit hooks could be problematic, but we could instead implement this by keeping a list of temporary paths to ensure the deletion of temp paths and using a single atexit handler to clean up all of them.

Then, of course, I start to think about "what if the list of temp paths gets really huge?" at which point I think about scanning it before the end of the process if it's too long when adding a new path to it and clearing out any paths that are already deleted so we don't have to remember them.

[ararslan]

Here's a radical idea: always do this, unconditionally.

That's an interesting idea, I think that would make a lot of sense. If you're doing mktempdir and you want the directory to persist after Julia exits, you should be using mkdir anyway.

[DilumAluthge]

Here's a radical idea: always do this, unconditionally.

That's an interesting idea, I think that would make a lot of sense. If you're doing mktempdir and you want the directory to persist after Julia exits, you should be using mkdir anyway.

That is not a crazy idea at all!

Then, of course, I start to think about "what if the list of temp paths gets really huge?" at which point I think about scanning it before the end of the process if it's too long when adding a new path to it and clearing out any paths that are already deleted so we don't have to remember them.

This would be easy to implement. We have some length cutoff N, and if the length of paths to delete is longer than N, we iterate through and delete the paths that no longer exist on disk.

[StefanKarpinski]

Yeah, I've already got a PR worked up for this. I'm stuck on a yak shave because it broke the precompile script and to fix the precompile script I need a count(regex, string) method, which doesn't exist, so I had to implement that first.

[StefanKarpinski]

Superseded by #32851. Thanks for the impetus, @DilumAluthge!