Make watchForChanges actually reload the KeyStore

[kevinherron]

Stacked on #1841 — review that one first. The diff shown here is against fix/keystore-certificate-store-writes and will shrink to just the watcher changes once #1841 merges.

KeyStoreCertificateStore.Settings.watchForChanges promises that a keystore replaced on disk gets picked up without restarting the server. It could not have worked, for several independent reasons, any one of which was enough on its own.

The watcher never called reset() on its WatchKey. A key stays signalled and is never queued again until it's reset, so after the first batch of events take() blocked for the life of the process — the watcher could fire at most once. That wouldn't have helped anyway, because the event filter compared p.toAbsolutePath() against the keystore path, and a directory watch reports a context relative to the watched directory while toAbsolutePath() resolves against user.dir. Unless the keystore happened to sit in the process working directory, the comparison never matched. On top of that only ENTRY_MODIFY was registered, so replacing a keystore the safe way — renaming a temporary file over it, which is exactly what this store does as of #1841 — produced no event at all; on Linux that registration becomes an inotify mask that IN_MOVED_TO isn't part of.

Underneath all of it, the reload didn't reload. It cleared the entry cache and called loadEntries(), which re-queries the KeyStore already held in memory since initialize(). The cache was emptied and immediately refilled with identical stale entries. Even with everything above fixed, the file was never re-read, so a renewed certificate could not have been seen.

The watcher now resets its key, registers ENTRY_CREATE alongside ENTRY_MODIFY, and resolves event contexts against the directory the watch was registered on. Reloading reads into a new KeyStore and swaps it in only once it has loaded, so a file that's unreadable, or still being written, leaves the one in use untouched. Overflow events are treated as a change, since there's no way to tell what was dropped.

One more thing surfaced while testing this. close() closes the WatchService while the watcher thread is parked in take(), which raises ClosedWatchServiceException. Nothing caught it, so it propagated out of run() and every shutdown left an uncaught exception behind. The thread now exits cleanly, and an interrupt during close() restores the interrupt flag instead of swallowing it.

Testing

The end-to-end test enables the setting, has a second store rewrite the file, and waits for the first one to notice. I ran it against the previous implementation to confirm it earns its keep: it times out there after thirty seconds and passes in well under a second against this one. A test that drives a feature purely through the public API is worth very little if it passes either way, and this one didn't.

Narrower tests pin the reload-from-disk behaviour without involving a watch service at all, so a regression there fails deterministically instead of as a timeout, and cover the event path resolution and overflow handling directly.

🤖 Generated with Claude Code