From 6da6acf51c50f1195cc4ed8210f83edbcd07d020 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Wed, 25 Sep 2024 11:22:23 -0400 Subject: [PATCH] [FileWatching] fix PollingFileWatcher design and add workaround for a stat bug What started as an innocent fix for a stat bug on Apple (#48667) turned into a full blown investigation into the design problems with the libuv backend for PollingFileWatcher, and writing my own implementation of it instead which could avoid those singled-threaded concurrency bugs. --- base/libuv.jl | 8 +- base/reflection.jl | 3 +- base/stat.jl | 111 +++++------ src/sys.c | 1 - stdlib/FileWatching/src/FileWatching.jl | 240 ++++++++++++++---------- stdlib/FileWatching/test/runtests.jl | 9 +- test/file.jl | 10 + 7 files changed, 215 insertions(+), 167 deletions(-) diff --git a/base/libuv.jl b/base/libuv.jl index 143201598fde0..17d75b254ed98 100644 --- a/base/libuv.jl +++ b/base/libuv.jl @@ -26,10 +26,10 @@ for r in uv_req_types @eval const $(Symbol("_sizeof_", lowercase(string(r)))) = uv_sizeof_req($r) end -uv_handle_data(handle) = ccall(:jl_uv_handle_data, Ptr{Cvoid}, (Ptr{Cvoid},), handle) -uv_req_data(handle) = ccall(:jl_uv_req_data, Ptr{Cvoid}, (Ptr{Cvoid},), handle) -uv_req_set_data(req, data) = ccall(:jl_uv_req_set_data, Cvoid, (Ptr{Cvoid}, Any), req, data) -uv_req_set_data(req, data::Ptr{Cvoid}) = ccall(:jl_uv_req_set_data, Cvoid, (Ptr{Cvoid}, Ptr{Cvoid}), req, data) +uv_handle_data(handle) = ccall(:uv_handle_get_data, Ptr{Cvoid}, (Ptr{Cvoid},), handle) +uv_req_data(handle) = ccall(:uv_req_get_data, Ptr{Cvoid}, (Ptr{Cvoid},), handle) +uv_req_set_data(req, data) = ccall(:uv_req_set_data, Cvoid, (Ptr{Cvoid}, Any), req, data) +uv_req_set_data(req, data::Ptr{Cvoid}) = ccall(:uv_handle_set_data, Cvoid, (Ptr{Cvoid}, Ptr{Cvoid}), req, data) macro handle_as(hand, typ) return quote diff --git a/base/reflection.jl b/base/reflection.jl index 5b395efc58190..cfeea1be5ff62 100644 --- a/base/reflection.jl +++ b/base/reflection.jl @@ -964,7 +964,7 @@ use it in the following manner to summarize information about a struct: julia> structinfo(T) = [(fieldoffset(T,i), fieldname(T,i), fieldtype(T,i)) for i = 1:fieldcount(T)]; julia> structinfo(Base.Filesystem.StatStruct) -13-element Vector{Tuple{UInt64, Symbol, Type}}: +14-element Vector{Tuple{UInt64, Symbol, Type}}: (0x0000000000000000, :desc, Union{RawFD, String}) (0x0000000000000008, :device, UInt64) (0x0000000000000010, :inode, UInt64) @@ -978,6 +978,7 @@ julia> structinfo(Base.Filesystem.StatStruct) (0x0000000000000050, :blocks, Int64) (0x0000000000000058, :mtime, Float64) (0x0000000000000060, :ctime, Float64) + (0x0000000000000068, :ioerrno, Int32) ``` """ fieldoffset(x::DataType, idx::Integer) = (@_foldable_meta; ccall(:jl_get_field_offset, Csize_t, (Any, Cint), x, idx)) diff --git a/base/stat.jl b/base/stat.jl index 506b5644dccbc..c6fb239a96404 100644 --- a/base/stat.jl +++ b/base/stat.jl @@ -63,6 +63,7 @@ struct StatStruct blocks :: Int64 mtime :: Float64 ctime :: Float64 + ioerrno :: Int32 end @eval function Base.:(==)(x::StatStruct, y::StatStruct) # do not include `desc` in equality or hash @@ -80,22 +81,23 @@ end end) end -StatStruct() = StatStruct("", 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0) -StatStruct(buf::Union{Vector{UInt8},Ptr{UInt8}}) = StatStruct("", buf) -StatStruct(desc::Union{AbstractString, OS_HANDLE}, buf::Union{Vector{UInt8},Ptr{UInt8}}) = StatStruct( +StatStruct() = StatStruct("", 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, Base.UV_ENOENT) +StatStruct(buf::Union{Memory{UInt8},Vector{UInt8},Ptr{UInt8}}, ioerrno::Int32) = StatStruct("", buf, ioerrno) +StatStruct(desc::Union{AbstractString, OS_HANDLE}, buf::Union{Memory{UInt8},Vector{UInt8},Ptr{UInt8}}, ioerrno::Int32) = StatStruct( desc isa OS_HANDLE ? desc : String(desc), - ccall(:jl_stat_dev, UInt32, (Ptr{UInt8},), buf), - ccall(:jl_stat_ino, UInt32, (Ptr{UInt8},), buf), - ccall(:jl_stat_mode, UInt32, (Ptr{UInt8},), buf), - ccall(:jl_stat_nlink, UInt32, (Ptr{UInt8},), buf), - ccall(:jl_stat_uid, UInt32, (Ptr{UInt8},), buf), - ccall(:jl_stat_gid, UInt32, (Ptr{UInt8},), buf), - ccall(:jl_stat_rdev, UInt32, (Ptr{UInt8},), buf), - ccall(:jl_stat_size, UInt64, (Ptr{UInt8},), buf), - ccall(:jl_stat_blksize, UInt64, (Ptr{UInt8},), buf), - ccall(:jl_stat_blocks, UInt64, (Ptr{UInt8},), buf), - ccall(:jl_stat_mtime, Float64, (Ptr{UInt8},), buf), - ccall(:jl_stat_ctime, Float64, (Ptr{UInt8},), buf), + ioerrno != 0 ? zero(UInt32) : ccall(:jl_stat_dev, UInt32, (Ptr{UInt8},), buf), + ioerrno != 0 ? zero(UInt32) : ccall(:jl_stat_ino, UInt32, (Ptr{UInt8},), buf), + ioerrno != 0 ? zero(UInt32) : ccall(:jl_stat_mode, UInt32, (Ptr{UInt8},), buf), + ioerrno != 0 ? zero(UInt32) : ccall(:jl_stat_nlink, UInt32, (Ptr{UInt8},), buf), + ioerrno != 0 ? zero(UInt32) : ccall(:jl_stat_uid, UInt32, (Ptr{UInt8},), buf), + ioerrno != 0 ? zero(UInt32) : ccall(:jl_stat_gid, UInt32, (Ptr{UInt8},), buf), + ioerrno != 0 ? zero(UInt32) : ccall(:jl_stat_rdev, UInt32, (Ptr{UInt8},), buf), + ioerrno != 0 ? zero(UInt64) : ccall(:jl_stat_size, UInt64, (Ptr{UInt8},), buf), + ioerrno != 0 ? zero(UInt64) : ccall(:jl_stat_blksize, UInt64, (Ptr{UInt8},), buf), + ioerrno != 0 ? zero(UInt64) : ccall(:jl_stat_blocks, UInt64, (Ptr{UInt8},), buf), + ioerrno != 0 ? zero(Float64) : ccall(:jl_stat_mtime, Float64, (Ptr{UInt8},), buf), + ioerrno != 0 ? zero(Float64) : ccall(:jl_stat_ctime, Float64, (Ptr{UInt8},), buf), + ioerrno ) function iso_datetime_with_relative(t, tnow) @@ -130,35 +132,41 @@ end function show_statstruct(io::IO, st::StatStruct, oneline::Bool) print(io, oneline ? "StatStruct(" : "StatStruct for ") show(io, st.desc) - oneline || print(io, "\n ") - print(io, " size: ", st.size, " bytes") - oneline || print(io, "\n") - print(io, " device: ", st.device) - oneline || print(io, "\n ") - print(io, " inode: ", st.inode) - oneline || print(io, "\n ") - print(io, " mode: 0o", string(filemode(st), base = 8, pad = 6), " (", filemode_string(st), ")") - oneline || print(io, "\n ") - print(io, " nlink: ", st.nlink) - oneline || print(io, "\n ") - print(io, " uid: $(st.uid)") - username = getusername(st.uid) - username === nothing || print(io, " (", username, ")") - oneline || print(io, "\n ") - print(io, " gid: ", st.gid) - groupname = getgroupname(st.gid) - groupname === nothing || print(io, " (", groupname, ")") - oneline || print(io, "\n ") - print(io, " rdev: ", st.rdev) - oneline || print(io, "\n ") - print(io, " blksz: ", st.blksize) - oneline || print(io, "\n") - print(io, " blocks: ", st.blocks) - tnow = round(UInt, time()) - oneline || print(io, "\n ") - print(io, " mtime: ", iso_datetime_with_relative(st.mtime, tnow)) - oneline || print(io, "\n ") - print(io, " ctime: ", iso_datetime_with_relative(st.ctime, tnow)) + code = st.ioerrno + if code != 0 + print(io, oneline ? " " : "\n ") + print(io, Base.uverrorname(code), ": ", Base.struverror(code)) + else + oneline || print(io, "\n ") + print(io, " size: ", st.size, " bytes") + oneline || print(io, "\n") + print(io, " device: ", st.device) + oneline || print(io, "\n ") + print(io, " inode: ", st.inode) + oneline || print(io, "\n ") + print(io, " mode: 0o", string(filemode(st), base = 8, pad = 6), " (", filemode_string(st), ")") + oneline || print(io, "\n ") + print(io, " nlink: ", st.nlink) + oneline || print(io, "\n ") + print(io, " uid: $(st.uid)") + username = getusername(st.uid) + username === nothing || print(io, " (", username, ")") + oneline || print(io, "\n ") + print(io, " gid: ", st.gid) + groupname = getgroupname(st.gid) + groupname === nothing || print(io, " (", groupname, ")") + oneline || print(io, "\n ") + print(io, " rdev: ", st.rdev) + oneline || print(io, "\n ") + print(io, " blksz: ", st.blksize) + oneline || print(io, "\n") + print(io, " blocks: ", st.blocks) + tnow = round(UInt, time()) + oneline || print(io, "\n ") + print(io, " mtime: ", iso_datetime_with_relative(st.mtime, tnow)) + oneline || print(io, "\n ") + print(io, " ctime: ", iso_datetime_with_relative(st.ctime, tnow)) + end oneline && print(io, ")") return nothing end @@ -168,18 +176,13 @@ show(io::IO, ::MIME"text/plain", st::StatStruct) = show_statstruct(io, st, false # stat & lstat functions +checkstat(s::StatStruct) = Int(s.ioerrno) in (0, Base.UV_ENOENT, Base.UV_ENOTDIR, Base.UV_EINVAL) ? s : uv_error(string("stat(", repr(s.desc), ")"), s.ioerrno) + macro stat_call(sym, arg1type, arg) return quote - stat_buf = zeros(UInt8, Int(ccall(:jl_sizeof_stat, Int32, ()))) + stat_buf = fill!(Memory{UInt8}(undef, Int(ccall(:jl_sizeof_stat, Int32, ()))), 0x00) r = ccall($(Expr(:quote, sym)), Int32, ($(esc(arg1type)), Ptr{UInt8}), $(esc(arg)), stat_buf) - if !(r in (0, Base.UV_ENOENT, Base.UV_ENOTDIR, Base.UV_EINVAL)) - uv_error(string("stat(", repr($(esc(arg))), ")"), r) - end - st = StatStruct($(esc(arg)), stat_buf) - if ispath(st) != (r == 0) - error("stat returned zero type for a valid path") - end - return st + return checkstat(StatStruct($(esc(arg)), stat_buf, r)) end end @@ -334,7 +337,7 @@ Return `true` if a valid filesystem entity exists at `path`, otherwise returns `false`. This is the generalization of [`isfile`](@ref), [`isdir`](@ref) etc. """ -ispath(st::StatStruct) = filemode(st) & 0xf000 != 0x0000 +ispath(st::StatStruct) = st.ioerrno == 0 function ispath(path::String) # We use `access()` and `F_OK` to determine if a given path exists. `F_OK` comes from `unistd.h`. F_OK = 0x00 diff --git a/src/sys.c b/src/sys.c index b54edc32b32b6..fa9054bb93e9a 100644 --- a/src/sys.c +++ b/src/sys.c @@ -102,7 +102,6 @@ JL_DLLEXPORT int32_t jl_nb_available(ios_t *s) // --- dir/file stuff --- -JL_DLLEXPORT int jl_sizeof_uv_fs_t(void) { return sizeof(uv_fs_t); } JL_DLLEXPORT char *jl_uv_fs_t_ptr(uv_fs_t *req) { return (char*)req->ptr; } JL_DLLEXPORT char *jl_uv_fs_t_path(uv_fs_t *req) { return (char*)req->path; } diff --git a/stdlib/FileWatching/src/FileWatching.jl b/stdlib/FileWatching/src/FileWatching.jl index 0c987ad01c828..4ea6fcedd59bb 100644 --- a/stdlib/FileWatching/src/FileWatching.jl +++ b/stdlib/FileWatching/src/FileWatching.jl @@ -22,11 +22,11 @@ export trymkpidlock import Base: @handle_as, wait, close, eventloop, notify_error, IOError, - _sizeof_uv_poll, _sizeof_uv_fs_poll, _sizeof_uv_fs_event, _uv_hook_close, uv_error, _UVError, - iolock_begin, iolock_end, associate_julia_struct, disassociate_julia_struct, - preserve_handle, unpreserve_handle, isreadable, iswritable, isopen, - |, getproperty, propertynames -import Base.Filesystem.StatStruct + uv_req_data, uv_req_set_data, associate_julia_struct, disassociate_julia_struct, + _sizeof_uv_poll, _sizeof_uv_fs, _sizeof_uv_fs_event, _uv_hook_close, uv_error, _UVError, + iolock_begin, iolock_end, preserve_handle, unpreserve_handle, + isreadable, iswritable, isopen, |, getproperty, propertynames +import Base.Filesystem: StatStruct, uv_fs_req_cleanup if Sys.iswindows() import Base.WindowsRawSocket end @@ -126,31 +126,30 @@ mutable struct FolderMonitor end end +# this is similar to uv_fs_poll, but strives to avoid the design mistakes that make it unsuitable for any usable purpose +# https://github.com/libuv/libuv/issues/4543 mutable struct PollingFileWatcher - @atomic handle::Ptr{Cvoid} file::String - interval::UInt32 - notify::Base.ThreadSynchronizer - active::Bool - curr_error::Int32 - curr_stat::StatStruct + interval::Float64 + const notify::Base.ThreadSynchronizer # lock protects all fields which can be changed (including interval and file, if you really must) + timer::Union{Nothing,Timer} + const stat_req::Memory{UInt8} + active::Bool # whether there is already an uv_fspollcb in-flight, so to speak + closed::Bool # whether the user has explicitly destroyed this + ioerrno::Int32 # the stat errno as of the last result + prev_stat::StatStruct # the stat as of the last successful result PollingFileWatcher(file::AbstractString, interval::Float64=5.007) = PollingFileWatcher(String(file), interval) function PollingFileWatcher(file::String, interval::Float64=5.007) # same default as nodejs - handle = Libc.malloc(_sizeof_uv_fs_poll) - this = new(handle, file, round(UInt32, interval * 1000), Base.ThreadSynchronizer(), false, 0, StatStruct()) - associate_julia_struct(handle, this) - iolock_begin() - err = ccall(:uv_fs_poll_init, Int32, (Ptr{Cvoid}, Ptr{Cvoid}), eventloop(), handle) - if err != 0 - Libc.free(handle) - throw(_UVError("PollingFileWatcher", err)) - end - finalizer(uvfinalize, this) - iolock_end() + stat_req = Memory{UInt8}(undef, Int(_sizeof_uv_fs)) + this = new(file, interval, Base.ThreadSynchronizer(), nothing, stat_req, false, false, 0, StatStruct()) + uv_req_set_data(stat_req, this) + wait(this) # initialize with the current stat before return return this end end +Base.stat(pfw::PollingFileWatcher) = Base.checkstat(@lock pfw.notify pfw.prev_stat) + mutable struct _FDWatcher @atomic handle::Ptr{Cvoid} fdnum::Int # this is NOT the file descriptor @@ -327,7 +326,7 @@ function close(t::FDWatcher) close(t.watcher, mask) end -function uvfinalize(uv::Union{FileMonitor, FolderMonitor, PollingFileWatcher}) +function uvfinalize(uv::Union{FileMonitor, FolderMonitor}) iolock_begin() if uv.handle != C_NULL disassociate_julia_struct(uv) # close (and free) without notify @@ -336,7 +335,7 @@ function uvfinalize(uv::Union{FileMonitor, FolderMonitor, PollingFileWatcher}) iolock_end() end -function close(t::Union{FileMonitor, FolderMonitor, PollingFileWatcher}) +function close(t::Union{FileMonitor, FolderMonitor}) iolock_begin() if t.handle != C_NULL ccall(:jl_close_uv, Cvoid, (Ptr{Cvoid},), t.handle) @@ -344,6 +343,21 @@ function close(t::Union{FileMonitor, FolderMonitor, PollingFileWatcher}) iolock_end() end +function close(pfw::PollingFileWatcher) + timer = nothing + lock(pfw.notify) + try + pfw.closed = true + notify(pfw.notify, false) + timer = pfw.timer + pfw.timer = nothing + finally + unlock(pfw.notify) + end + timer === nothing || close(timer) + nothing +end + function _uv_hook_close(uv::_FDWatcher) # fyi: jl_atexit_hook can cause this to get called too Libc.free(@atomicswap :monotonic uv.handle = C_NULL) @@ -351,18 +365,6 @@ function _uv_hook_close(uv::_FDWatcher) nothing end -function _uv_hook_close(uv::PollingFileWatcher) - lock(uv.notify) - try - uv.active = false - Libc.free(@atomicswap :monotonic uv.handle = C_NULL) - notify(uv.notify, StatStruct()) - finally - unlock(uv.notify) - end - nothing -end - function _uv_hook_close(uv::FileMonitor) lock(uv.notify) try @@ -388,7 +390,7 @@ end isopen(fm::FileMonitor) = fm.handle != C_NULL isopen(fm::FolderMonitor) = fm.handle != C_NULL -isopen(pfw::PollingFileWatcher) = pfw.handle != C_NULL +isopen(pfw::PollingFileWatcher) = !pfw.closed isopen(pfw::_FDWatcher) = pfw.refcount != (0, 0) isopen(pfw::FDWatcher) = !pfw.mask.timedout @@ -449,21 +451,50 @@ function uv_pollcb(handle::Ptr{Cvoid}, status::Int32, events::Int32) nothing end -function uv_fspollcb(handle::Ptr{Cvoid}, status::Int32, prev::Ptr, curr::Ptr) - t = @handle_as handle PollingFileWatcher - old_status = t.curr_error - t.curr_error = status - if status == 0 - t.curr_stat = StatStruct(convert(Ptr{UInt8}, curr)) - end - if status == 0 || status != old_status - prev_stat = StatStruct(convert(Ptr{UInt8}, prev)) - lock(t.notify) - try - notify(t.notify, prev_stat) - finally - unlock(t.notify) +function uv_fspollcb(req::Ptr{Cvoid}) + pfw = unsafe_pointer_to_objref(uv_req_data(req))::PollingFileWatcher + pfw.active = false + unpreserve_handle(pfw) + @assert pointer(pfw.stat_req) == req + r = Int32(ccall(:uv_fs_get_result, Cssize_t, (Ptr{Cvoid},), req)) + statbuf = ccall(:uv_fs_get_statbuf, Ptr{UInt8}, (Ptr{Cvoid},), req) + curr_stat = StatStruct(pfw.file, statbuf, r) + uv_fs_req_cleanup(req) + lock(pfw.notify) + try + if !isempty(pfw.notify) # discard the update if nobody watching + if pfw.ioerrno != r || (r == 0 && pfw.prev_stat != curr_stat) + if r == 0 + pfw.prev_stat = curr_stat + end + pfw.ioerrno = r + notify(pfw.notify, true) + end + pfw.timer = Timer(pfw.interval) do t + # async task + iolock_begin() + lock(pfw.notify) + try + if pfw.timer === t # use identity check to test if this callback is stale by the time we got the lock + pfw.timer = nothing + @assert !pfw.active + if isopen(pfw) && !isempty(pfw.notify) + preserve_handle(pfw) + err = ccall(:uv_fs_stat, Cint, (Ptr{Cvoid}, Ptr{Cvoid}, Cstring, Ptr{Cvoid}), + eventloop(), pfw.stat_req, pfw.file, uv_jl_fspollcb) + err == 0 || notify(pfw.notify, _UVError("PollingFileWatcher (start)", err), error=true) # likely just ENOMEM + pfw.active = true + end + end + finally + unlock(pfw.notify) + end + iolock_end() + nothing + end end + finally + unlock(pfw.notify) end nothing end @@ -475,7 +506,7 @@ global uv_jl_fseventscb_folder::Ptr{Cvoid} function __init__() global uv_jl_pollcb = @cfunction(uv_pollcb, Cvoid, (Ptr{Cvoid}, Cint, Cint)) - global uv_jl_fspollcb = @cfunction(uv_fspollcb, Cvoid, (Ptr{Cvoid}, Cint, Ptr{Cvoid}, Ptr{Cvoid})) + global uv_jl_fspollcb = @cfunction(uv_fspollcb, Cvoid, (Ptr{Cvoid},)) global uv_jl_fseventscb_file = @cfunction(uv_fseventscb_file, Cvoid, (Ptr{Cvoid}, Ptr{Int8}, Int32, Int32)) global uv_jl_fseventscb_folder = @cfunction(uv_fseventscb_folder, Cvoid, (Ptr{Cvoid}, Ptr{Int8}, Int32, Int32)) @@ -504,35 +535,6 @@ function start_watching(t::_FDWatcher) nothing end -function start_watching(t::PollingFileWatcher) - iolock_begin() - t.handle == C_NULL && throw(ArgumentError("PollingFileWatcher is closed")) - if !t.active - uv_error("PollingFileWatcher (start)", - ccall(:uv_fs_poll_start, Int32, (Ptr{Cvoid}, Ptr{Cvoid}, Cstring, UInt32), - t.handle, uv_jl_fspollcb::Ptr{Cvoid}, t.file, t.interval)) - t.active = true - end - iolock_end() - nothing -end - -function stop_watching(t::PollingFileWatcher) - iolock_begin() - lock(t.notify) - try - if t.active && isempty(t.notify) - t.active = false - uv_error("PollingFileWatcher (stop)", - ccall(:uv_fs_poll_stop, Int32, (Ptr{Cvoid},), t.handle)) - end - finally - unlock(t.notify) - end - iolock_end() - nothing -end - function start_watching(t::FileMonitor) iolock_begin() t.handle == C_NULL && throw(ArgumentError("FileMonitor is closed")) @@ -640,28 +642,65 @@ end function wait(pfw::PollingFileWatcher) iolock_begin() - preserve_handle(pfw) lock(pfw.notify) - local prevstat + prevstat = pfw.prev_stat + havechange = false + timer = nothing try - start_watching(pfw) + # we aren't too strict about the first interval after `wait`, but rather always + # check right away to see if it had immediately changed again, and then repeatedly + # after interval again until success + pfw.closed && throw(ArgumentError("PollingFileWatcher is closed")) + timer = pfw.timer + pfw.timer = nothing # disable Timer callback + # start_watching + if !pfw.active + preserve_handle(pfw) + err = ccall(:uv_fs_stat, Cint, (Ptr{Cvoid}, Ptr{Cvoid}, Cstring, Ptr{Cvoid}), + eventloop(), pfw.stat_req, pfw.file, uv_jl_fspollcb) + err == 0 || uv_error("PollingFileWatcher (start)", err) # likely just ENOMEM + pfw.active = true + end iolock_end() - prevstat = wait(pfw.notify)::StatStruct + havechange = wait(pfw.notify)::Bool unlock(pfw.notify) iolock_begin() - lock(pfw.notify) - finally - unlock(pfw.notify) - unpreserve_handle(pfw) + catch + # stop_watching: cleanup any timers from before or after starting this wait before it failed, if there are no other watchers + latetimer = nothing + try + if isempty(pfw.notify) + latetimer = pfw.timer + pfw.timer = nothing + end + finally + unlock(pfw.notify) + end + if timer !== nothing || latetimer !== nothing + iolock_end() + timer === nothing || close(timer) + latetimer === nothing || close(latetimer) + iolock_begin() + end + rethrow() end - stop_watching(pfw) iolock_end() - if pfw.handle == C_NULL + timer === nothing || close(timer) # cleanup resources so we don't hang on exit + if !havechange # user canceled by calling close return prevstat, EOFError() - elseif pfw.curr_error != 0 - return prevstat, _UVError("PollingFileWatcher", pfw.curr_error) + end + # grab the most up-to-date stat result as of this time, even if it was a bit newer than the notify call + lock(pfw.notify) + currstat = pfw.prev_stat + ioerrno = pfw.ioerrno + unlock(pfw.notify) + if ioerrno == 0 + @assert currstat.ioerrno == 0 + return prevstat, currstat + elseif ioerrno in (Base.UV_ENOENT, Base.UV_ENOTDIR, Base.UV_EINVAL) + return prevstat, StatStruct(pfw.file, Ptr{UInt8}(0), ioerrno) else - return prevstat, pfw.curr_stat + return prevstat, _UVError("PollingFileWatcher", ioerrno) end end @@ -880,9 +919,9 @@ The `previous` status is always a `StatStruct`, but it may have all of the field The `current` status object may be a `StatStruct`, an `EOFError` (indicating the timeout elapsed), or some other `Exception` subtype (if the `stat` operation failed - for example, if the path does not exist). -To determine when a file was modified, compare `current isa StatStruct && mtime(prev) != mtime(current)` to detect -notification of changes. However, using [`watch_file`](@ref) for this operation is preferred, since -it is more reliable and efficient, although in some situations it may not be available. +To determine when a file was modified, compare `!(current isa StatStruct && prev == current)` to detect +notification of changes to the mtime or inode. However, using [`watch_file`](@ref) for this operation +is preferred, since it is more reliable and efficient, although in some situations it may not be available. """ function poll_file(s::AbstractString, interval_seconds::Real=5.007, timeout_s::Real=-1) pfw = PollingFileWatcher(s, Float64(interval_seconds)) @@ -893,12 +932,7 @@ function poll_file(s::AbstractString, interval_seconds::Real=5.007, timeout_s::R close(pfw) end end - statdiff = wait(pfw) - if isa(statdiff[2], IOError) - # file didn't initially exist, continue watching for it to be created (or the error to change) - statdiff = wait(pfw) - end - return statdiff + return wait(pfw) finally close(pfw) @isdefined(timer) && close(timer) diff --git a/stdlib/FileWatching/test/runtests.jl b/stdlib/FileWatching/test/runtests.jl index 2592aea024386..c9d7a4317fd08 100644 --- a/stdlib/FileWatching/test/runtests.jl +++ b/stdlib/FileWatching/test/runtests.jl @@ -2,6 +2,7 @@ using Test, FileWatching using Base: uv_error, Experimental +using Base.Filesystem: StatStruct @testset "FileWatching" begin @@ -218,7 +219,7 @@ function test_timeout(tval) @async test_file_poll(channel, 10, tval) tr = take!(channel) end - @test tr[1] === Base.Filesystem.StatStruct() && tr[2] === EOFError() + @test ispath(tr[1]::StatStruct) && tr[2] === EOFError() @test tval <= t_elapsed end @@ -231,7 +232,7 @@ function test_touch(slval) write(f, "Hello World\n") close(f) tr = take!(channel) - @test ispath(tr[1]) && ispath(tr[2]) + @test ispath(tr[1]::StatStruct) && ispath(tr[2]::StatStruct) fetch(t) end @@ -435,8 +436,8 @@ end @test_throws(Base._UVError("FolderMonitor (start)", Base.UV_ENOENT), watch_folder("____nonexistent_file", 10)) @test(@elapsed( - @test(poll_file("____nonexistent_file", 1, 3.1) === - (Base.Filesystem.StatStruct(), EOFError()))) > 3) + @test(poll_file("____nonexistent_file", 1, 3.1) == + (StatStruct(), EOFError()))) > 3) unwatch_folder(dir) @test isempty(FileWatching.watched_folders) diff --git a/test/file.jl b/test/file.jl index de258c92e02bc..a4262c4eaaa21 100644 --- a/test/file.jl +++ b/test/file.jl @@ -2128,6 +2128,16 @@ Base.joinpath(x::URI50890) = URI50890(x.f) @test !isnothing(Base.Filesystem.getusername(s.uid)) @test !isnothing(Base.Filesystem.getgroupname(s.gid)) end + s = Base.Filesystem.StatStruct() + stat_show_str = sprint(show, s) + stat_show_str_multi = sprint(show, MIME("text/plain"), s) + @test startswith(stat_show_str, "StatStruct(\"\" ENOENT: ") && endswith(stat_show_str, ")") + @test startswith(stat_show_str_multi, "StatStruct for \"\"\n ENOENT: ") && !endswith(stat_show_str_multi, r"\s") + s = Base.Filesystem.StatStruct("my/test", Ptr{UInt8}(0), Int32(Base.UV_ENOTDIR)) + stat_show_str = sprint(show, s) + stat_show_str_multi = sprint(show, MIME("text/plain"), s) + @test startswith(stat_show_str, "StatStruct(\"my/test\" ENOTDIR: ") && endswith(stat_show_str, ")") + @test startswith(stat_show_str_multi, "StatStruct for \"my/test\"\n ENOTDIR: ") && !endswith(stat_show_str_multi, r"\s") end @testset "diskstat() works" begin