The decode pipeline runs on its own thread and calls back into + * {@code MainViewModel} after each pass to kick off background work (QTH/grid + * lookups, network TX audio). Those pools are shut down in + * {@code MainViewModel.onCleared()} when the ViewModel is destroyed (app close, + * connection reset/reconnect). A decode result that lands in the same instant + * the pool is shut down would otherwise reach {@link ExecutorService#execute} + * on a terminated pool and throw {@link RejectedExecutionException} on the + * decode thread, crashing the app. See the {@code afterDecode} race fixed + * alongside this helper. + */ +public final class ExecutorUtils { + + private ExecutorUtils() { + } + + /** + * Submit {@code task} to {@code pool}, tolerating a pool that is shutting + * down. The {@link ExecutorService#isShutdown()} check avoids the common + * case; the catch covers the narrow race where the pool is shut down + * between the check and {@link ExecutorService#execute}. + * + * @return {@code true} if the task was accepted, {@code false} if it was + * dropped because the pool (or task) was unavailable. + */ + public static boolean safeExecute(ExecutorService pool, Runnable task) { + if (pool == null || task == null || pool.isShutdown()) { + return false; + } + try { + pool.execute(task); + return true; + } catch (RejectedExecutionException e) { + // Pool was shut down concurrently (ViewModel teardown). Drop the task. + return false; + } + } +} diff --git a/ft8af/app/src/main/java/com/k1af/ft8af/MainViewModel.java b/ft8af/app/src/main/java/com/k1af/ft8af/MainViewModel.java index 3bda4674..f7ec10f4 100644 --- a/ft8af/app/src/main/java/com/k1af/ft8af/MainViewModel.java +++ b/ft8af/app/src/main/java/com/k1af/ft8af/MainViewModel.java @@ -591,7 +591,10 @@ public void afterDecode(long utc, float time_sec, int sequential getQTHRunnable.messages = messages; - getQTHThreadPool.execute(getQTHRunnable);//query location via thread pool + //query location via thread pool; guarded so a decode landing during + //ViewModel teardown (pool shut down in onCleared()) drops the lookup + //instead of crashing with RejectedExecutionException. + ExecutorUtils.safeExecute(getQTHThreadPool, getQTHRunnable); //this variable also notifies message list changes mutable_Decoded_Counter.postValue( @@ -685,8 +688,8 @@ public void onTransmitByWifi(Ft8Message msg) { if (baseRig.isConnected()) { sendWaveDataRunnable.baseRig = baseRig; sendWaveDataRunnable.message = msg; - //send network data packets via thread pool - sendWaveDataThreadPool.execute(sendWaveDataRunnable); + //send network data packets via thread pool (guarded against teardown race) + ExecutorUtils.safeExecute(sendWaveDataThreadPool, sendWaveDataRunnable); } } } @@ -714,7 +717,7 @@ public void onTransmitOverCAT(Ft8Message msg) {//send audio message via CAT } sendWaveDataRunnable.baseRig = baseRig; sendWaveDataRunnable.message = msg; - sendWaveDataThreadPool.execute(sendWaveDataRunnable); + ExecutorUtils.safeExecute(sendWaveDataThreadPool, sendWaveDataRunnable); } }, new OnTransmitSuccess() {//when QSO is successful diff --git a/ft8af/app/src/test/java/com/k1af/ft8af/ExecutorUtilsTest.java b/ft8af/app/src/test/java/com/k1af/ft8af/ExecutorUtilsTest.java new file mode 100644 index 00000000..396da0a6 --- /dev/null +++ b/ft8af/app/src/test/java/com/k1af/ft8af/ExecutorUtilsTest.java @@ -0,0 +1,61 @@ +package com.k1af.ft8af; + +import static com.google.common.truth.Truth.assertThat; + +import org.junit.Test; + +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; + +/** + * Unit tests for {@link ExecutorUtils#safeExecute(ExecutorService, Runnable)}. + * + *
Locks the invariant behind the {@code afterDecode} crash fix: submitting a + * decode-followup task to a pool that has been shut down (ViewModel teardown) + * must drop the task quietly instead of throwing + * {@link java.util.concurrent.RejectedExecutionException}. + */ +public class ExecutorUtilsTest { + + @Test + public void activePool_runsTaskAndReturnsTrue() throws InterruptedException { + ExecutorService pool = Executors.newSingleThreadExecutor(); + try { + CountDownLatch ran = new CountDownLatch(1); + boolean accepted = ExecutorUtils.safeExecute(pool, ran::countDown); + assertThat(accepted).isTrue(); + assertThat(ran.await(2, TimeUnit.SECONDS)).isTrue(); + } finally { + pool.shutdownNow(); + } + } + + @Test + public void shutdownPool_dropsTaskAndReturnsFalse() { + ExecutorService pool = Executors.newSingleThreadExecutor(); + pool.shutdown(); + // The pre-fix code path: execute() on a terminated pool threw and crashed + // the decode thread. safeExecute must swallow it and report the drop. + boolean accepted = ExecutorUtils.safeExecute(pool, () -> { + throw new AssertionError("task must not run on a shut-down pool"); + }); + assertThat(accepted).isFalse(); + } + + @Test + public void nullPool_returnsFalse() { + assertThat(ExecutorUtils.safeExecute(null, () -> { })).isFalse(); + } + + @Test + public void nullTask_returnsFalse() { + ExecutorService pool = Executors.newSingleThreadExecutor(); + try { + assertThat(ExecutorUtils.safeExecute(pool, null)).isFalse(); + } finally { + pool.shutdownNow(); + } + } +}