Fixed the race condition: print dev mode ready message after WatchService registration - #526
Conversation
| inputUnavailable.wait(500); | ||
| } | ||
| printDevModeMessages(inputUnavailable.get(), firstStartup); | ||
| firstStartup = false; |
There was a problem hiding this comment.
Could you explain why we don't need to clear this boolean anymore? The comment says "If the thread is already running..." so it makes me wonder if firstStartup is always false by the time this is called.
There was a problem hiding this comment.
The firstStartup is always false by the time runHotkeyReaderThread() is reached. watchFiles() sets firstStartup = false before entering the watch loop via initWatchLoop(). The only call site for runHotkeyReaderThread() in ci.common is inside TestJob.run(), which runs after the watch loop is active, so watchFiles() has always completed by then. The old code reset it here to cover the premature call from DevMojo/DevTask before watchFiles() ran. That premature call is the race condition we are removing, and without it the reset is no longer needed.
2 participants
Fixes OpenLiberty/ci.maven#2070
PR Ci.Maven: OpenLiberty/ci.maven#2073
PR Ci.Gradle: OpenLiberty/ci.gradle#1095
In DevUtil.watchFiles(), the "Liberty is running in dev mode." startup banner is now printed after all source, test, config, and resource directories have been registered with the WatchService, but before initWatchLoop() enters the event poll. Previously the banner was emitted by runHotkeyReaderThread() in the callers (ci.maven's DevMojo, ci.gradle's DevTask) before watchFiles() was even called, meaning any file change made immediately after that message by a test or a user could race ahead of the watcher and be silently dropped with no recompile triggered. Moving the print into watchFiles() and checking it in with the existing firstStartup flag makes the ready message a reliable synchronisation point: once a user or test sees it, the WatchService is fully set up and guaranteed to detect changes. The runHotkeyReaderThread() post test run re print path is unchanged.