Stop a crash on app exit / mode switch: discard UtcTimer pool submits during teardown - #618
Conversation
Root cause ---------- UtcTimer runs two java.util.Timer tasks: a 10 ms cycle-boundary check (secTask) and a 1 s heartbeat (heartBeatTask). Each tick submits its callback to a cached thread pool via execute(). delete() tears the timer down by cancelling both Timers and then calling shutdownNow() on both pools. Timer.cancel() does not wait for a TimerTask that is already running, so a tick can be mid-run() — about to call pool.execute() — at the instant delete() shuts that pool down from another thread. With the default AbortPolicy, execute() then throws RejectedExecutionException. secTask's catch only handles InterruptedException and heartBeatTask has no catch at all, so the exception escapes TimerTask.run(), terminates the Timer thread, and reaches the process's default uncaught-exception handler — crashing the app. delete() runs on every app exit (ComposeMainActivity.onDestroy) and every FT8/FT4/FT2 mode switch (rebuildTimer), while the 1 s heartbeat is always live, so the race is exercised routinely. Fix --- Build both pools with a ThreadPoolExecutor.DiscardPolicy rejected-execution handler (otherwise identical to Executors.newCachedThreadPool()). A submit that loses the race with shutdown is now silently dropped — the correct behaviour during teardown, since the cycle/heartbeat callback is moot once we are shutting down. This is a no-op in normal operation: with an unbounded maximum pool size over a SynchronousQueue, a submit is never rejected until the pool has been shut down. Testing ------- Added pure-JVM tests to UtcTimerTest: the discarding pool tolerates a submit after shutdownNow() (the crashing path), a vanilla cached pool still throws there (documenting the pre-fix bug), and the discarding pool still runs work submitted before shutdown. Full :app:testDebugUnitTest passes (29 UtcTimerTest cases). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #618 +/- ##
=========================================
Coverage 36.62% 36.62%
Complexity 197 197
=========================================
Files 216 216
Lines 26885 26885
Branches 3294 3294
=========================================
Hits 9847 9847
Misses 16811 16811
Partials 227 227
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This pull request hardens UtcTimer teardown so app exit and FT8/FT4/FT2 mode switches don’t intermittently crash due to a RejectedExecutionException race between TimerTask.run() submitting work and delete() shutting down the underlying executors.
Changes:
- Replace
Executors.newCachedThreadPool()with a cachedThreadPoolExecutorconfigured to discard late submits after shutdown to avoidRejectedExecutionExceptionduring teardown. - Add unit tests that reproduce the pre-fix rejection behavior and validate the new pool’s post-shutdown behavior and normal operation.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| ft8af/app/src/main/java/com/k1af/ft8af/timer/UtcTimer.java | Introduces newDiscardingCachedThreadPool() and switches both timer executors to use it to prevent teardown-time crashes. |
| ft8af/app/src/test/java/com/k1af/ft8af/timer/UtcTimerTest.java | Adds regression tests documenting the old crash behavior and verifying the new executor’s behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…down DiscardPolicy drops every rejection, so a rejection during normal operation (e.g. thread creation failing under resource exhaustion) would silently swallow the cycle/heartbeat callback. DiscardOnShutdownPolicy discards only when executor.isShutdown() — the teardown race this fix targets — and delegates everything else to AbortPolicy, keeping real failures visible. New test: a saturated still-running pool with the policy installed still throws RejectedExecutionException. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
UtcTimer.delete()can crash the app with an uncaughtRejectedExecutionExceptionwhen the timer is torn down while one of its own ticks is submitting to the thread pool being shut down. This happens on every app exit and every FT8/FT4/FT2 mode switch, so it is a genuine (if intermittent) instability, not a theoretical one.Root cause
UtcTimerruns twojava.util.Timertasks:secTask— a 10 ms cycle-boundary check that, at a slot boundary, doescachedThreadPool.execute(doSomething).heartBeatTask— a 1 s heartbeat that always doesheartBeatThreadPool.execute(doHeartBeat).delete()cancels bothTimers and then callsshutdownNow()on both pools:Timer.cancel()does not wait for aTimerTaskthat is already running. So a tick can be mid-run()— about to callpool.execute()— at the instantdelete()shuts that pool down from another thread. With the defaultAbortPolicy,execute()then throwsRejectedExecutionException.secTask'scatchonly handlesInterruptedException, andheartBeatTaskhas nocatchat all, so the exception escapesTimerTask.run(), terminates theTimerthread, and reaches the process's default uncaught-exception handler → app crash.Reachability:
delete()is called fromComposeMainActivity.onDestroy(every app exit) and fromFT8SignalListener.rebuildTimer/FT8TransmitSignal.rebuildTimeron every operating-mode switch — all while the 1 s heartbeat is live, so the window is exercised routinely across the user base.Fix
Construct both pools with a
ThreadPoolExecutor.DiscardPolicyrejected-execution handler (otherwise identical toExecutors.newCachedThreadPool()). A submit that loses the race with shutdown is now silently discarded — the correct behaviour during teardown, since the cycle/heartbeat callback is moot once we are shutting down.This is a no-op in normal operation: with an unbounded maximum pool size over a
SynchronousQueue, a submit is never rejected until the pool has been shut down. The change is localized to pool construction; the timer/callback logic is untouched.Testing
./gradlew :app:testDebugUnitTest --tests com.k1af.ft8af.timer.UtcTimerTest— all 29 cases pass, including 3 new pure-JVM tests:discardingPool_doesNotThrowWhenSubmittingAfterShutdown— the exact crashing path (executeaftershutdownNow) no longer throws.defaultCachedPool_throwsWhenSubmittingAfterShutdown_documentingTheBug— a vanilla cached pool still throws there, pinning the pre-fix behaviour.discardingPool_stillRunsSubmittedWorkBeforeShutdown— normal-path work still executes.Risk assessment
Low. No protocol/DSP/timing behaviour changes — the firing instants, callbacks, and pool semantics in normal operation are identical (
DiscardPolicyonly activates post-shutdown). No native code touched; performance characteristics unchanged.🤖 Generated with Claude Code