mktemp/dir: delete temp files on exit by default

[StefanKarpinski]

An alternative to #32335. Still needs tests and NEWS, which can maybe be taken from there. Sorry to snipe your PR, @DilumAluthge 😬

Instead of putting lots of atexit handlers in the list, this uses a single one and keeps a list of temporary paths to cleanup. If the list gets long (≥ 1024) it will be scanned for already-deleted paths when adding a new path to the cleanup list. To avoid running too frequently, it doubles the "too many paths" threshold after this.

Needs the generate_precompile improvements in #32850 which in turn needs the count(regex, string) method added in #32849. A yak shave in chained pull request form.

[DilumAluthge]

I like this a lot!

[StefanKarpinski]

Tests passed for all but last commit, now running those tests.

[StefanKarpinski]

Win32 failure is the usual FileWatching failure.

[StefanKarpinski]

Ok, I'd say this is ready to go. There is 3x as much test code added as implementation 😂. But the tests did catch several bonehead mistakes I'd made, so yay.

[StefanKarpinski]

So, on Windows, of course, problem child that it is, you cannot remove a file while anyone has it open. So when we open a temporary file in the child and then forget about it and let the exit handler clean it up that causes all sorts of problems since the "resource is busy". One option would be to call GC.gc() in the temp_cleanup_atexit handler if there are any files that need deleting.

[DilumAluthge]

Classic Windows

[StefanKarpinski]

So it seems that on Windows doing chmod(0o400) on the parent directory of a file doesn't make it impossible to delete the file. Any ideas on how to prevent deletion of a temp file on Windows?

[DilumAluthge]

I wonder if it’s related to this: https://support.microsoft.com/en-hk/help/320246/inherited-permissions-are-not-automatically-updated-when-you-move-fold

Apparently depending on what API you use, permissions don’t always inherit.

You said that chmod 000 didn’t work on Windows, right?

Is this just for the tests, or for the implementation?

[StefanKarpinski]

Just the tests. Since we know Windows can't delete open files, I decided to use that approach.

[StefanKarpinski]

All green! Have squashed into one commit and changed GC.gc() calls to GC.gc(true) calls. This is ready to go now. Asked Jeff, Jameson and Elliot what they thought about this and all seemed in favor. This is effectively a simple garbage collector for temp paths without needing to make a special (mutable) type for temp paths that the normal GC can finalize.

[ararslan]

I'd be interested to know what @fredrikekre's objection is.

[StefanKarpinski]

My guess is that it's arguably overly complex and seems not to be something most runtimes do.

[fredrikekre]

What Stefan said, but more importantly that I find these functions very useful to create random directories/filenames in a user created non-temp directory. I use this quite often and I want to inspect the files after Julia exits. With this implementation it does not seem to be a way to opt out either.
I think mktempdir(...) do d; ... ; end, together with the opt-out from #32355 is sufficient alternatives for users that want to cleanup.

[StefanKarpinski]

With this implementation it does not seem to be a way to opt out either.

Well, mktemp(cleanup=false) and mktempdir(cleanup=false) in this PR lets you opt out.

I find these functions very useful to create random directories/filenames in a user created non-temp directory. I use this quite often and I want to inspect the files after Julia exits.

I think this use case is unusual enough that it doesn't really warrant being the default. You can also always do path = joinpath(dir, randstring()) if you want to create a random path that you want to keep around after your process has exited.

[StefanKarpinski]

Regarding complexity, note that the implementation of this is only 25 lines long. The tests, on the other hand... are almost 200 lines and kind of nasty, but I wanted to make sure it was correctly implemented and it's incidentally testing a fair amount of other mktemp/dir functionality that wasn't very well tested previously, so I don't think that should be held against this change.

[vtjnash]

Regarding complexity

Not that I'd ever use more tests as a bad thing, but I think lots of kind of nasty edge cases (and as a weak proxy, number of tests) is the sort of the thing that makes something complex—much more so than SLOC of the code itself.

[StefanKarpinski]

The tests aren't complex because there are lots of nasty edge cases, they're complex because it's testing something that inherently has to do with external mutable state (temp files and directories) and because I'm a thorough motherfucker and wanted to get full coverage of all code paths (and found several bonehead bugs because of it).

[StefanKarpinski]

I'm going to go ahead with this given that it has relatively broad support, limited opposition and on the premise that most people want their temporary files and directories to be temporary.

[vtjnash]

Doesn’t this need a try/catch? There’s a somewhat complex edge cases on Windows (deleting an open file will essentially just set the delete-on-close bit and cause some future operations, including rm, to error with file-not-found)

[StefanKarpinski]

Does it? I thought that force=true made it never fail.