Martin Tournoij [Sat, 30 Jul 2022 10:54:27 +0000 (12:54 +0200)]
macos: retry if open() returns EINTR (#475)
Retry the unix.Open() if the error returned is EINTR; looking around the
web it seems many systems handle it like this. This is also what
os.Open() does:
https://github.com/golang/go/commit/50d0ee0c98ea21f818d2daa9bc21ef51861a2ef9
Martin Tournoij [Fri, 29 Jul 2022 21:00:44 +0000 (23:00 +0200)]
Remove AUTHORS file
This goes back to when this library was part of golang.org/x/...; the "Google lawyers" said that an AUTHORS file was needed, but seems they changed their minds and it got removed from Go last week and decided that the git log is enough:
https://github.com/golang/go/commit/774fa58d1d3c9926709e108afdf83af7010bde72
I don't think there's much value keeping it here, and it's one more thing that can be forgotten (it's already out of date). The authorship is preserved in the git log, and if that's good enough for Google's lawyers then it's good enough for me.
Martin Tournoij [Fri, 29 Jul 2022 18:31:09 +0000 (20:31 +0200)]
inotify: don't ignore events for files that don't exist (#470)
This is quite an odd check, leading to an inconsistent event stream,
which also doesn't match what other platforms do. If you do a "CREATE +
MODIFY + REMOVE" in quick succession then you probably *want* all three
events. If you don't want to operate on non-existing files, then you can
check this in your application code.
You need to do that already, since this check is far from reliable. In
the time between this check and the application code doing something
with an event the file may have been deleted already.
I looked a bit at the history of this, and looks like it was added in
2013 with cc2c34e; issue 36 refers to this issue on the old repo, which
mentions it fixes a memory leak: https://github.com/howeyc/fsnotify/issues/36
I can't reproduce that at all; using the CLI from #463 modified to print
the memory and running:
for i in $(seq 0 10000); { touch $i; rm $i }
Memory stays at about 100/110K in both the current main branch and this.
It's mostly the same as the example in the README, except that it takes
paths from the commandline and aligns things a bit nicer.
This is useful for documentation and experimentation. For example, while
reviewing some PRs I found it useful to quickly get an overview of "what
does fsnotify do now, and what does it do with this patch?"
Martin Tournoij [Fri, 29 Jul 2022 18:28:22 +0000 (20:28 +0200)]
kqueue: better error if watching a file fails (#471)
kqueue requires opening a file descriptor for every file; so we read the
directory and do that. If this failed the error was quite unclear; for
example:
Martin Tournoij [Sun, 24 Jul 2022 09:22:52 +0000 (11:22 +0200)]
Test some more things in CI (#469)
* Test some more things in CI
- Test all GOOS/GOARCH combinations by looping over "go tool dist list";
this just tests if it compiles.
- Add a Vagrant box to test Debian 6 / Linux 2.6.32; this was adapted
from @horahoradev's patch at #434.
- Update the minimum version requirements: we test Linux 2.6.32 now and
turns out that's also the minimum version [Go supports] in recent
releases, so just set it to that.
Need Go 1.16 for retract in go.mod. I don't know, maybe we can just
remove the retract? Latest Debian ships with Go 1.15.
- Test both Go 1.16 and 1.18 (the lowest supported version and newest
version). I guess we could also test 1.17, but the CI already takes
somewhat long and I can't recall ever having a situation where an
intermediate version failed.
- Test macOS 11 and 12; macOS 10.15 will (probably) end support in
November, so probably not worth supporting this. GitHub will [remove]
support for this at the end of August.
- Actually run tests for all systems Go supports. Bit pointless right
now as many of these don't do anything. Currently untested are
Solaris, illumios, plan9, AIX, Android, iOS, DragonflyBSD, WASM.
Some of these might be difficult (AIX, iOS, Android), but haven't
looked in to it. I tried setting up Solaris with the
vmactions/solaris, but it doesn't seem to have an easy way to install
Go.
- GitHub only [supports] Windows Server 2019 and 2022; probabably also
want to test Server 2016, but GitHub dropped support for this. Can
maybe use AppVeyor(?)
Martin Tournoij [Fri, 22 Jul 2022 05:22:49 +0000 (07:22 +0200)]
inotify: fix race in Close() (#465)
Would sometimes fail with:
panic: close of closed channel
goroutine 204 [running]:
github.com/fsnotify/fsnotify.(*Watcher).Close(0xc0003e6410)
/home/martin/code/Golib/fsnotify/inotify.go:82 +0x66
created by github.com/fsnotify/fsnotify.TestCloseRace
/home/martin/code/Golib/fsnotify/integration_test.go:1256 +0x1a5
exit status 2
Because isClosed() might return "false" for two goroutines, and then
they will both try to close it, resulting in the panic.
Kir Kolyshkin [Thu, 21 Jul 2022 11:32:11 +0000 (04:32 -0700)]
Do not suppress Chmod on non-existent file (#260)
Currently fsnotify suppresses a Chmod event if the file does not exist
when the event is received.
This makes it impossible to use fsnotify to detect when an opened file
is removed. In such case the Linux kernel sends IN_ATTRIB event,
as described in inotify(7) man page:
> IN_ATTRIB (*)
> Metadata changed—for example, permissions (e.g., chmod(2)),
> timestamps (e.g., utimensat(2)), extended attributes (setx‐
> attr(2)), link count (since Linux 2.6.25; e.g., for the tar‐
> get of link(2) and for unlink(2)), and user/group ID (e.g.,
> chown(2)).
(to clarify, in this very case it's link count that changes).
To fix:
* Modify the code to only suppress MODIFY and CREATE events.
* Add a test case to verify Chmod event is delivered.
While at it, fix the comment in ignoreLinux() to use the up-to-date
terminology (event ops).
A test case is added.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Nick Santos [Thu, 21 Jul 2022 10:20:14 +0000 (11:20 +0100)]
kqueue: Make watcher.Close() O(n) instead of O(n^2) (#233)
In the old implementation watcher.Close() would clone the list of
watches and then run Remove() on that, and every Remove() call would
iterate over the full list of watches.
This stores the watches more efficiently so that Remove() doesn't need
to iterate over the full list: instead of merely a path → fd map, it
also stores a parent-path → list-of-files.
No functional changes, just a performance improvement.
Martin Tournoij [Thu, 21 Jul 2022 03:06:57 +0000 (05:06 +0200)]
Use common error when removing an unwatched file (#460)
The errors returned by the various implementations of the watcher are all different
which makes handling them difficult. This PR follows the suggestion in:
https://github.com/fsnotify/fsnotify/pull/455#issuecomment-1146037157 by @mattn
to create a common error which is wrapped by the implementations.
Replaces: https://github.com/fsnotify/fsnotify/pull/455 Signed-off-by: Andrew Thornton <art27@cantab.net> Co-authored-by: Andrew Thornton <art27@cantab.net>
Robert Fratto [Thu, 21 Jul 2022 03:06:40 +0000 (23:06 -0400)]
windows: protect access to isClosed with mutex (#454)
isClosed may be read and modified concurrently if the Windows Watcher is
closed at the same time as a new entry being added. This change protects
isClosed to prevent race conditions.
The order of fields for the Windows Watcher have been rearranged to
group fields protected by the mutex together.
Tobias Klauser [Tue, 25 Jan 2022 02:13:06 +0000 (03:13 +0100)]
Allow build on unsupported GOOS (#424)
In cases where fsnotify is a transitive dependency of a package, it
might need to be built on platforms not supported (yet), e.g. aix.
Provide an empty implemention for these cases, so depending packages
don't need to add workarounds.
Tobias Klauser [Wed, 19 Jan 2022 14:22:52 +0000 (15:22 +0100)]
Don't set poller.fd twice in newFdPoller (#406)
In newEmptyPoller when creating the poller using newEmptyPoller(fd), the
fd field of the poller instance is already set. There is no need to set
it again.
Aaron L [Wed, 29 Mar 2017 04:21:07 +0000 (21:21 -0700)]
Fix deadlock in Remove (linux/inotify)
Several people have reported this issue where if you are using a
single goroutine to watch for fs events and you call Remove in
that goroutine it can deadlock. The cause for this is that the Remove
was made synchronous by PR #73. The reason for this was to try and
ensure that maps were no longer leaking.
In this PR: IN_IGNORE was used as the event to ensure map cleanup.
This worked fine when Remove() was called and the next event was
IN_IGNORE, but when a different event was received the main goroutine
that's supposed to be reading from the Events channel would be stuck
waiting for the sync.Cond, which would never be hit because the select
would then block waiting for someone to receive the non-IN_IGNORE event
from the channel so it could proceed to process the IN_IGNORE event that
was waiting in the queue. Deadlock :)
Removing the synchronization then created two nasty races where Remove
followed by Remove would error unnecessarily, and one where Remove
followed by an Add could result in the maps being cleaned up AFTER the
Add call which means the inotify watch is active, but our maps don't
have the values anymore. It then becomes impossible to delete the
watches via the fsnotify code since it checks it's local data before
calling InotifyRemove.
This code attempts to use IN_DELETE_SELF as a means to know when a watch
was deleted as part of an unlink(). That means that we didn't delete the
watch via the fsnotify lib and we should clean up our maps since that
watch no longer exists. This allows us to clean up the maps immediately
when calling Remove since we no longer try to synchronize cleanup
using IN_IGNORE as the sync point.
If Watcher.Add is called with the same name from multiple goroutines and
name is not already in the watches map then the last call to complete
wins and the watch created by the first call to complete is lost.
This commit holds the mutex for the full duration of the update to the
watches map.
* avoid clobbering existing flags if inotify_add_watch fails
Previously, the watch entry is the watches map was updated before the
call to InotifyAddWatch. If InotifyAddWatch fails then this left the
watch entry in an inconsistent state.
projects / fsnotify.git / log