mktemp, mktempdir: try harder to delete temp files before process exit

The main change here is that the non-higher-order versions of mktemp/dir will
now by default put the temp files/dirs that they create into a temp cleanup list
which will be deleted upon process exit. Instead of putting lots of atexit
handlers in the list, this uses a single handler 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.

If cleanup fails for the `mktemp/dir() do ... end` forms, they will also put
their temp files/dirs in the cleanup list but marked for "asap" deletion. Every
time the above mechanism for cleaning out the temp cleanup list scans for
already-deleted temp files, this will try again to delete these temp files.

NEWS.md

@@ -54,6 +54,7 @@ Standard library changes
 * `mod` now accepts a unit range as the second argument to easily perform offset modular arithmetic to ensure the result is inside the range ([#32628]).
 * `Sockets.recvfrom` now returns both host and port as an InetAddr ([#32729]).
 * `nothing` can now be `print`ed, and interplated into strings etc. as the string `"nothing"`. It is still not permitted to be interplated into Cmds (i.e. ``echo `$(nothing)` `` will still error without running anything.) ([#32148])
+* `mktemp` and `mktempdir` now try to remove temporary paths they create by default before the process exits ([#32851]).
 
 #### Libdl
 

base/file.jl

@@ -448,6 +448,31 @@ function tempdir()
     end
 end
 
+const TEMP_CLEANUP_MIN = Ref(1024)
+const TEMP_CLEANUP_MAX = Ref(1024)
+const TEMP_CLEANUP = Dict{String,Bool}()
+
+function temp_cleanup_later(path::AbstractString; asap::Bool=false)
+    TEMP_CLEANUP[path] = asap
+    length(TEMP_CLEANUP) ≤ TEMP_CLEANUP_MAX[] && return false
+    temp_cleanup_purge()
+    TEMP_CLEANUP_MAX[] = max(TEMP_CLEANUP_MIN[], 2*length(TEMP_CLEANUP))
+    return true
+end
+
+function temp_cleanup_purge()
+    for (path, asap) in TEMP_CLEANUP
+        asap && rm(path, recursive=true, force=true)
+        !ispath(path) && delete!(TEMP_CLEANUP, path)
+    end
+end
+
+function temp_cleanup_atexit()
+    for path in keys(TEMP_CLEANUP)
+        rm(path, recursive=true, force=true)
+    end
+end
+
 const temp_prefix = "jl_"
 
 if Sys.iswindows()
@@ -466,8 +491,9 @@ function _win_tempname(temppath::AbstractString, uunique::UInt32)
     return transcode(String, tname)
 end
 
-function mktemp(parent=tempdir())
+function mktemp(parent::AbstractString=tempdir(); cleanup::Bool=true)
     filename = _win_tempname(parent, UInt32(0))
+    cleanup && temp_cleanup_later(filename)
     return (filename, Base.open(filename, "r+"))
 end
 
@@ -498,10 +524,11 @@ function tempname()
 end
 
 # Create and return the name of a temporary file along with an IOStream
-function mktemp(parent=tempdir())
+function mktemp(parent::AbstractString=tempdir(); cleanup::Bool=true)
     b = joinpath(parent, temp_prefix * "XXXXXX")
     p = ccall(:mkstemp, Int32, (Cstring,), b) # modifies b
     systemerror(:mktemp, p == -1)
+    cleanup && temp_cleanup_later(b)
     return (b, fdio(p, true))
 end
 
@@ -524,22 +551,25 @@ created. The path is likely to be unique, but this cannot be guaranteed.
 tempname()
 
 """
-    mktemp(parent=tempdir())
+    mktemp(parent=tempdir(); cleanup=true) -> (path, io)
 
-Return `(path, io)`, where `path` is the path of a new temporary file in `parent` and `io`
-is an open file object for this path.
+Return `(path, io)`, where `path` is the path of a new temporary file in `parent`
+and `io` is an open file object for this path. The `cleanup` option controls whether
+the temporary file is automatically deleted when the process exits.
 """
 mktemp(parent)
 
 """
-    mktempdir(parent=tempdir(); prefix=$(repr(temp_prefix)))
+    mktempdir(parent=tempdir(); prefix=$(repr(temp_prefix)), cleanup=true) -> path
 
 Create a temporary directory in the `parent` directory with a name
 constructed from the given prefix and a random suffix, and return its path.
 Additionally, any trailing `X` characters may be replaced with random characters.
-If `parent` does not exist, throw an error.
+If `parent` does not exist, throw an error. The `cleanup` option controls whether
+the temporary directory is automatically deleted when the process exits.
 """
-function mktempdir(parent=tempdir(); prefix=temp_prefix)
+function mktempdir(parent::AbstractString=tempdir();
+    prefix::AbstractString=temp_prefix, cleanup::Bool=true)
     if isempty(parent) || occursin(path_separator_re, parent[end:end])
         # append a path_separator only if parent didn't already have one
         tpath = "$(parent)$(prefix)XXXXXX"
@@ -558,6 +588,7 @@ function mktempdir(parent=tempdir(); prefix=temp_prefix)
         end
         path = unsafe_string(ccall(:jl_uv_fs_t_path, Cstring, (Ptr{Cvoid},), req))
         ccall(:uv_fs_req_cleanup, Cvoid, (Ptr{Cvoid},), req)
+        cleanup && temp_cleanup_later(path)
         return path
     finally
         Libc.free(req)
@@ -571,8 +602,8 @@ end
 Apply the function `f` to the result of [`mktemp(parent)`](@ref) and remove the
 temporary file upon completion.
 """
-function mktemp(fn::Function, parent=tempdir())
-    (tmp_path, tmp_io) = mktemp(parent)
+function mktemp(fn::Function, parent::AbstractString=tempdir())
+    (tmp_path, tmp_io) = mktemp(parent, cleanup=false)
     try
         fn(tmp_path, tmp_io)
     finally
@@ -582,6 +613,8 @@ function mktemp(fn::Function, parent=tempdir())
             rm(tmp_path)
         catch ex
             @error "mktemp cleanup" _group=:file exception=(ex, catch_backtrace())
+            # might be possible to remove later
+            temp_cleanup_later(tmpdir, asap=true)
         end
     end
 end
@@ -592,8 +625,9 @@ end
 Apply the function `f` to the result of [`mktempdir(parent; prefix)`](@ref) and remove the
 temporary directory all of its contents upon completion.
 """
-function mktempdir(fn::Function, parent=tempdir(); prefix=temp_prefix)
-    tmpdir = mktempdir(parent; prefix=prefix)
+function mktempdir(fn::Function, parent::AbstractString=tempdir();
+    prefix::AbstractString=temp_prefix)
+    tmpdir = mktempdir(parent; prefix=prefix, cleanup=false)
     try
         fn(tmpdir)
     finally
@@ -602,6 +636,8 @@ function mktempdir(fn::Function, parent=tempdir(); prefix=temp_prefix)
             rm(tmpdir, recursive=true)
         catch ex
             @error "mktempdir cleanup" _group=:file exception=(ex, catch_backtrace())
+            # might be possible to remove later
+            temp_cleanup_later(tmpdir, asap=true)
         end
     end
 end

base/initdefs.jl

@@ -293,7 +293,7 @@ end
 
 ## atexit: register exit hooks ##
 
-const atexit_hooks = []
+const atexit_hooks = Callable[Filesystem.temp_cleanup_atexit]
 
 """
     atexit(f)

doc/src/base/file.md

@@ -29,10 +29,10 @@ Base.Filesystem.rm
 Base.Filesystem.touch
 Base.Filesystem.tempname
 Base.Filesystem.tempdir
-Base.Filesystem.mktemp(::Any)
-Base.Filesystem.mktemp(::Function, ::Any)
-Base.Filesystem.mktempdir(::Any)
-Base.Filesystem.mktempdir(::Function, ::Any)
+Base.Filesystem.mktemp(::AbstractString)
+Base.Filesystem.mktemp(::Function, ::AbstractString)
+Base.Filesystem.mktempdir(::AbstractString)
+Base.Filesystem.mktempdir(::Function, ::AbstractString)
 Base.Filesystem.isblockdev
 Base.Filesystem.ischardev
 Base.Filesystem.isdir

test/file.jl

@@ -48,6 +48,107 @@ if !Sys.iswindows()
     cd(pwd_)
 end
 
+child_eval(code::String) = eval(Meta.parse(readchomp(`$(Base.julia_cmd()) -E $code`)))
+
+@testset "mktemp/dir basic cleanup" begin
+    # mktemp without cleanup
+    t = child_eval("t = mktemp(cleanup=false)[1]; @assert isfile(t); t")
+    @test isfile(t)
+    rm(t, force=true)
+    @test !ispath(t)
+    # mktemp with cleanup
+    t = child_eval("t = mktemp()[1]; @assert isfile(t); t")
+    @test !ispath(t)
+    # mktempdir without cleanup
+    t = child_eval("t = mktempdir(cleanup=false); touch(\"\$t/file.txt\"); t")
+    @test isfile("$t/file.txt")
+    rm(t, recursive=true, force=true)
+    @test !ispath(t)
+    # mktempdir with cleanup
+    t = child_eval("t = mktempdir(); touch(\"\$t/file.txt\"); t")
+    @test !ispath(t)
+end
+
+import Base.Filesystem: TEMP_CLEANUP_MIN, TEMP_CLEANUP_MAX, TEMP_CLEANUP
+
+function with_temp_temp_cleanup(f::Function, n::Int)
+    SAVE_TEMP_CLEANUP_MIN = TEMP_CLEANUP_MIN[]
+    SAVE_TEMP_CLEANUP_MAX = TEMP_CLEANUP_MAX[]
+    SAVE_TEMP_CLEANUP = copy(TEMP_CLEANUP)
+    empty!(TEMP_CLEANUP)
+    TEMP_CLEANUP_MIN[] = n
+    TEMP_CLEANUP_MAX[] = n
+    try f()
+    finally
+        copy!(TEMP_CLEANUP, SAVE_TEMP_CLEANUP)
+        TEMP_CLEANUP_MAX[] = SAVE_TEMP_CLEANUP_MAX
+        TEMP_CLEANUP_MIN[] = SAVE_TEMP_CLEANUP_MIN
+    end
+end
+
+@testset "mktemp/dir cleanup list purging" begin
+    n = 12 # cleanup min & max
+    @assert n % 2 == n % 3 == 0 # otherwise tests won't work
+    with_temp_temp_cleanup(n) do
+        # for n mktemps, no purging is triggered
+        temps = String[]
+        for i = 1:n
+            t = i % 2 == 0 ? mktemp()[1] : mktempdir()
+            push!(temps, t)
+            @test ispath(t)
+            @test length(TEMP_CLEANUP) == i 
+            @test TEMP_CLEANUP_MAX[] == n
+            # delete 1/3 of the temp paths
+            i % 3 == 0 && rm(t, recursive=true, force=true)
+        end
+        # without cleanup no purge is triggered
+        t = mktempdir(cleanup=false)
+        @test isdir(t)
+        @test length(TEMP_CLEANUP) == n
+        @test TEMP_CLEANUP_MAX[] == n
+        rm(t, recursive=true, force=true)
+        # purge triggered by next mktemp with cleanup
+        t = mktemp()[1]
+        push!(temps, t)
+        n′ = 2n÷3 + 1
+        @test 2n′ > n
+        @test isfile(t)
+        @test length(TEMP_CLEANUP) == n′
+        @test TEMP_CLEANUP_MAX[] == 2n′
+        # remove all temp files
+        for t in temps
+            rm(t, recursive=true, force=true)
+        end
+        # for n′ mktemps, no purging is triggered
+        for i = 1:n′
+            t = i % 2 == 0 ? mktemp()[1] : mktempdir()
+            push!(temps, t)
+            @test ispath(t)
+            @test length(TEMP_CLEANUP) == n′ + i
+            @test TEMP_CLEANUP_MAX[] == 2n′
+            # delete 2/3 of the temp paths
+            i % 3 != 0 && rm(t, recursive=true, force=true)
+        end
+        # without cleanup no purge is triggered
+        t = mktemp(cleanup=false)[1]
+        @test isfile(t)
+        @test length(TEMP_CLEANUP) == 2n′
+        @test TEMP_CLEANUP_MAX[] == 2n′
+        rm(t, force=true)
+        # purge triggered by next mktemp
+        t = mktempdir()
+        push!(temps, t)
+        n′′ = n′÷3 + 1
+        @test 2n′′ < n
+        @test isdir(t)
+        @test length(TEMP_CLEANUP) == n′′
+        @test TEMP_CLEANUP_MAX[] == n
+        # remove all temp files
+        for t in temps
+            rm(t, recursive=true, force=true)
+        end
+    end
+end
 
 #######################################################################
 # This section tests some of the features of the stat-based file info #