diff options
| author | Peter Geissler <peter@haxx.in> | 2026-08-28 15:10:39 +0200 |
|---|---|---|
| committer | Peter Geissler <peter@haxx.in> | 2026-08-28 15:10:39 +0200 |
| commit | b184ca357c6989b77171547071614a78b375cbcb (patch) | |
| tree | 4c76ba4de410ce93d1c4a987b6e4392be51087ec /include | |
| parent | path: unit tests for the two spellings (diff) | |
| download | slosh-b184ca357c6989b77171547071614a78b375cbcb.tar.gz slosh-b184ca357c6989b77171547071614a78b375cbcb.tar.xz slosh-b184ca357c6989b77171547071614a78b375cbcb.zip | |
windows watcher: rebuild the watch set instead of freezing it
watch_config() rebuilds the set of watched directories after every
reload, because a config that just changed may include different files
than it did a second ago. The inotify and kqueue branches unwatch and
re-add. The Windows branch did neither: unwatching was a comment saying
there was nothing to do, and sl_watch_add() appended to an array the
pump thread had already finished reading.
The pump captured `int n = w->ndirs` once, at the top, and armed one
ReadDirectoryChangesW per directory it found. Everything added after
that was stored and never watched. So an include added to a running
session loaded fine and then sat there: editing it reloaded nothing,
while editing the main config still worked, which is the shape of bug
that gets reported as "sometimes".
It was worse than deterministic. `w->ndirs` was written by the server
thread and read by the pump with nothing between them, so whether a
directory made it in depended on which won the start-up race --
_beginthreadex returns before the new thread runs, so the includes
present at startup usually did, and the ones added later never could.
And because nothing was ever closed, each reload re-added the same
directories: handles accumulated until the cap of 8, after which
sl_watch_add() failed quietly and no new directory could be watched at
all for the life of the session.
So the pump owns the handles and the server owns the wish. An
overlapped read can only be cancelled by the thread that issued it,
which settles where the boundary goes: the server writes a directory
list under a lock, bumps a generation and pokes an event; the pump
notices the generation moved, disarms what it had and arms what is
wanted now. Cancelling waits for the cancellation to land before the
slot is reused, because the OVERLAPPED and its buffer live in that
slot. Duplicate directories are dropped on the way in -- a config and
the theme beside it are usually in the same one -- and the compare is
case-insensitive, as the filesystem is.
sl_watch_close() also stopped every watcher in the process rather than
the one it was given, and freed nothing. It now finds its own, joins
the thread, and releases what that leaves behind.
Verified on the Windows box, with a fully backslashed SLOSH_CONFIG:
editing the config reloads; adding an include and then editing it
reloads; repointing the include at an unrelated directory reloads;
editing the file the include used to name does *not*, which is the
rebuild working rather than an append; and after twelve reloads in a
row it is still watching, where the old cap would have given up after
eight.
Diffstat (limited to 'include')
| -rw-r--r-- | include/compat_win.h | 4 |
1 files changed, 4 insertions, 0 deletions
diff --git a/include/compat_win.h b/include/compat_win.h index b52f512..c8bf1f1 100644 --- a/include/compat_win.h +++ b/include/compat_win.h @@ -259,6 +259,10 @@ void sl_globfree(glob_t *g); * reads it as "something under a watched directory changed" and re-stats the * files it cares about -- the same conclusion its kqueue branch reaches. */ int sl_watch_init(void); /* -> pollable fd, or -1 */ +/* The watch set is rebuilt, not appended to: clear, then add what the config + * names now. A reload may name different files than the last one did, and the + * inotify and kqueue branches rebuild for that reason too. */ +void sl_watch_clear(int fd); int sl_watch_add(int fd, const char *dir); void sl_watch_close(int fd); int sl_watch_drain(int fd); /* consume readiness; 1 if anything changed */ |
