Skip to content

Commit

Permalink
[mktempdir()]: reset permissions before attempting to delete (#37206)
Browse files Browse the repository at this point in the history
  • Loading branch information
staticfloat authored Sep 1, 2020
1 parent 3285ee0 commit a3430e7
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 1 deletion.
32 changes: 31 additions & 1 deletion base/file.jl
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,32 @@ function tempdir()
end
end

"""
prepare_for_deletion(path::AbstractString)
Prepares the given `path` for deletion by ensuring that all directories within that
`path` have write permissions, so that files can be removed from them. This is
automatically invoked by methods such as `mktempdir()` to ensure that no matter what
weird permissions a user may have created directories with within the temporary prefix,
it will always be deleted.
"""
function prepare_for_deletion(path::AbstractString)
# Nothing to do for non-directories
if !isdir(path)
return
end

try chmod(path, filemode(path) | 0o333)
catch; end
for (root, dirs, files) in walkdir(path)
for dir in dirs
dpath = joinpath(root, dir)
try chmod(dpath, filemode(dpath) | 0o333)
catch; end
end
end
end

const TEMP_CLEANUP_MIN = Ref(1024)
const TEMP_CLEANUP_MAX = Ref(1024)
const TEMP_CLEANUP = Dict{String,Bool}()
Expand All @@ -484,6 +510,7 @@ function temp_cleanup_purge(; force::Bool=false)
if (force || asap) && ispath(path)
need_gc && GC.gc(true)
need_gc = false
prepare_for_deletion(path)
rm(path, recursive=true, force=true)
end
!ispath(path) && delete!(TEMP_CLEANUP, path)
Expand Down Expand Up @@ -682,7 +709,10 @@ function mktempdir(fn::Function, parent::AbstractString=tempdir();
fn(tmpdir)
finally
try
ispath(tmpdir) && rm(tmpdir, recursive=true)
if ispath(tmpdir)
prepare_for_deletion(tmpdir)
rm(tmpdir, recursive=true)
end
catch ex
@error "mktempdir cleanup" _group=:file exception=(ex, catch_backtrace())
# might be possible to remove later
Expand Down
14 changes: 14 additions & 0 deletions test/file.jl
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,20 @@ no_error_logging(f::Function) =
end
end

@testset "mktempdir() permissions correction" begin
# Test that `mktempdir()` with children with weird permissions get cleared
# before we would delete the directory
local temp_dir_path
mktempdir() do dir
temp_dir_path = dir
mkdir(joinpath(dir, "foo"))
touch(joinpath(dir, "foo", "bar"))
chmod(joinpath(dir, "foo"), 0o444)
@test isdir(temp_dir_path)
end
@test !isdir(temp_dir_path)
end

#######################################################################
# This section tests some of the features of the stat-based file info #
#######################################################################
Expand Down

2 comments on commit a3430e7

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily package evaluation, I will reply here when finished:

@nanosoldier runtests(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your package evaluation job has completed - possible issues were detected. A full report can be found here. cc @maleadt

Please sign in to comment.