From 356b52de364d8d3ac3aa73d9844d3427a1591f13 Mon Sep 17 00:00:00 2001 From: Mattia Iavarone Date: Wed, 31 Jul 2019 13:04:26 +0200 Subject: [PATCH] Reuse fallback WorkerHandler --- .../engine/CameraIntegrationTest.java | 4 +- .../internal/utils/WorkerHandlerTest.java | 2 +- .../internal/utils/WorkerHandler.java | 52 ++++++++++++++----- 3 files changed, 43 insertions(+), 15 deletions(-) diff --git a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/engine/CameraIntegrationTest.java b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/engine/CameraIntegrationTest.java index 5971d98d..86172788 100644 --- a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/engine/CameraIntegrationTest.java +++ b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/engine/CameraIntegrationTest.java @@ -86,7 +86,7 @@ public abstract class CameraIntegrationTest extends BaseTest { @Before public void setUp() { LOG.e("Test started. Setting up camera."); - WorkerHandler.destroy(); + WorkerHandler.destroyAll(); uiSync(new Runnable() { @Override @@ -126,7 +126,7 @@ public abstract class CameraIntegrationTest extends BaseTest { public void tearDown() { LOG.e("Test ended. Tearing down camera."); camera.destroy(); - WorkerHandler.destroy(); + WorkerHandler.destroyAll(); } private void waitForUiException() throws Throwable { diff --git a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/internal/utils/WorkerHandlerTest.java b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/internal/utils/WorkerHandlerTest.java index acf53a49..459b9052 100644 --- a/cameraview/src/androidTest/java/com/otaliastudios/cameraview/internal/utils/WorkerHandlerTest.java +++ b/cameraview/src/androidTest/java/com/otaliastudios/cameraview/internal/utils/WorkerHandlerTest.java @@ -218,7 +218,7 @@ public class WorkerHandlerTest extends BaseTest { public void testDestroy() { final WorkerHandler handler = WorkerHandler.get("handler"); assertTrue(handler.getThread().isAlive()); - WorkerHandler.destroy(); + WorkerHandler.destroyAll(); // Wait for the thread to die. try { handler.getThread().join(500); } catch (InterruptedException ignore) {} assertFalse(handler.getThread().isAlive()); diff --git a/cameraview/src/main/java/com/otaliastudios/cameraview/internal/utils/WorkerHandler.java b/cameraview/src/main/java/com/otaliastudios/cameraview/internal/utils/WorkerHandler.java index 0b932842..fe33edbd 100644 --- a/cameraview/src/main/java/com/otaliastudios/cameraview/internal/utils/WorkerHandler.java +++ b/cameraview/src/main/java/com/otaliastudios/cameraview/internal/utils/WorkerHandler.java @@ -25,6 +25,13 @@ public class WorkerHandler { private final static CameraLogger LOG = CameraLogger.create(WorkerHandler.class.getSimpleName()); private final static ConcurrentHashMap> sCache = new ConcurrentHashMap<>(4); + private final static String FALLBACK_NAME = "FallbackCameraThread"; + + // Store a hard reference to the fallback handler. We never use this, only update it + // anytime get() is called. This should ensure that this instance is not collected. + @SuppressWarnings("FieldCanBeLocal") + private static WorkerHandler sFallbackHandler; + /** * Gets a possibly cached handler with the given name. * @param name the handler name @@ -36,14 +43,19 @@ public class WorkerHandler { //noinspection ConstantConditions WorkerHandler cached = sCache.get(name).get(); if (cached != null) { - HandlerThread thread = cached.mThread; - if (thread.isAlive() && !thread.isInterrupted()) { + if (cached.getThread().isAlive() && !cached.getThread().isInterrupted()) { LOG.w("get:", "Reusing cached worker handler.", name); return cached; + } else { + // Cleanup the old thread before creating a new one + cached.destroy(); + LOG.w("get:", "Thread reference found, but not alive or interrupted. Removing.", name); + sCache.remove(name); } + } else { + LOG.w("get:", "Thread reference died. Removing.", name); + sCache.remove(name); } - LOG.w("get:", "Thread reference died, removing.", name); - sCache.remove(name); } LOG.i("get:", "Creating new handler.", name); @@ -58,7 +70,8 @@ public class WorkerHandler { */ @NonNull public static WorkerHandler get() { - return get("FallbackCameraThread"); + sFallbackHandler = get(FALLBACK_NAME); + return sFallbackHandler; } /** @@ -183,6 +196,7 @@ public class WorkerHandler { * Returns the android backing {@link Looper}. * @return the looper */ + @SuppressWarnings("WeakerAccess") @NonNull public Looper getLooper() { return mThread.getLooper(); @@ -197,21 +211,35 @@ public class WorkerHandler { return mExecutor; } + /** + * Destroys this handler and its thread. After this method returns, the handler + * should be considered unusable. + * + * Internal note: this does not remove the thread from our cache, but it does + * interrupt it, so the next {@link #get(String)} call will remove it. + * In any case, we only store weak references. + */ + @SuppressWarnings("WeakerAccess") + public void destroy() { + HandlerThread thread = getThread(); + if (thread.isAlive()) { + thread.interrupt(); + thread.quit(); + // after quit(), the thread will die at some point in the future. Might take some ms. + // try { handler.getThread().join(); } catch (InterruptedException ignore) {} + } + } + /** * Destroys all handlers, interrupting their work and * removing them from our cache. */ - public static void destroy() { + public static void destroyAll() { for (String key : sCache.keySet()) { WeakReference ref = sCache.get(key); //noinspection ConstantConditions WorkerHandler handler = ref.get(); - if (handler != null && handler.getThread().isAlive()) { - handler.getThread().interrupt(); - handler.getThread().quit(); - // after quit(), the thread will die at some point in the future. Might take some ms. - // try { handler.getThread().join(); } catch (InterruptedException ignore) {} - } + if (handler != null) handler.destroy(); ref.clear(); } sCache.clear();