Commits
Tarun Sukhani authored f0d3f3d22c4
fix: hoist Play.detectChanges PROD fast-path above synchronized boundary (release 1.12.15)
PF-61: even after PF-60 fixed the queued-one-shot side of JobsPlugin
shutdown, "Jobs scheduler did not terminate within 30000 ms" still fires
deterministically on every clean shutdown of an idle JVM with any
@Every job. Thread-dump analysis surfaces a hard lock-ordering deadlock:
Thread-0 (JVM shutdown hook)
Play.stop [holds Play.class]
pluginCollection.onApplicationStop
JobsPlugin.onApplicationStop
VirtualThreadScheduledExecutor.shutdownGracefully(30000)
virtualExecutor.awaitTermination [waits up to 30s]
jobs-vthread-N (in-flight @Every iteration)
Job.run
Invoker.Invocation.init
Play.detectChanges [BLOCKED waiting for Play.class]
Both methods were `static synchronized`, so both took Play.class as
their monitor. Play.stop holds it for the duration of plugin shutdown;
any VT dispatched moments before SIGTERM that's running through the
@Every invocation pipeline blocks at detectChanges' synchronized
entry — even though the body would return immediately in PROD.
shutdownGracefully cannot return until the VTs drain; the VTs cannot
proceed until shutdown releases the lock; the deadlock self-resolves
only when the 30s timeout fires shutdownNow().
Fix: split detectChanges into a thin unsynchronized wrapper that holds
the PROD short-circuit, plus a private detectChangesLocked() that
holds the existing DEV body. PROD mode now returns without acquiring
the monitor, breaking the deadlock. DEV mode (where the body actually
mutates classloader/router state) still synchronizes — hot-reload
safety is preserved.
JMM safety: Play.mode is set once during Play.init (Play.java:271,
281) before any VT dispatches. The thread-start happens-before edge
publishes that write to every later @Every VT, so the unsynchronized
read is safe without volatile.
Public API unchanged: callers keep invoking Play.detectChanges()
exactly as before; only the internal implementation moves.
Regression test: PlayDetectChangesTest.detectChangesIsLockFreeInProdMode
holds Play.class from a separate thread and verifies detectChanges()
returns in <500ms with mode=PROD. Pre-fix would block for the full
lock-holder timeout; post-fix returns in ~78ms. Future regressions
that re-merge the synchronized boundary will be caught.
Together with PF-60, completes the JobsPlugin-shutdown fix: clean
PROD shutdowns of idle JVMs now complete in O(in-flight VT runtime)
rather than O(stop-timeout).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>