diff --git a/CHANGELOG.md b/CHANGELOG.md index 447a90c623..cee7f14aa1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ Mapbox welcomes participation and contributions from everyone. * Fix a bug in cameraForGeometry returning incorrect camera options when pitch > 0. ([1568](https://github.com/mapbox/mapbox-maps-android/pull/1568)) * Fix Android memory leak when destroying platform view annotation manager. ([1568](https://github.com/mapbox/mapbox-maps-android/pull/1568)) * Fix style getters for terrain, light and atmosphere resetting properties. ([1573](https://github.com/mapbox/mapbox-maps-android/pull/1573)) +* Fix possible ANR when destroying renderer. ([1567](https://github.com/mapbox/mapbox-maps-android/pull/1567)) ## Dependencies Bump gl-native to v10.8.0-beta.1, common to v23.0.0-beta.1. ([1568](https://github.com/mapbox/mapbox-maps-android/pull/1568)) diff --git a/sdk/src/main/java/com/mapbox/maps/renderer/MapboxRenderThread.kt b/sdk/src/main/java/com/mapbox/maps/renderer/MapboxRenderThread.kt index dc1e74566a..2bf30459ad 100644 --- a/sdk/src/main/java/com/mapbox/maps/renderer/MapboxRenderThread.kt +++ b/sdk/src/main/java/com/mapbox/maps/renderer/MapboxRenderThread.kt @@ -403,7 +403,7 @@ internal class MapboxRenderThread : Choreographer.FrameCallback { logI(TAG, "RenderThread : surface destroyed") lock.withLock { // in some situations `destroy` is called earlier than onSurfaceDestroyed - in that case no need to clean up - if (renderHandlerThread.started) { + if (renderHandlerThread.isRunning) { renderHandlerThread.post { awaitingNextVsync = false Choreographer.getInstance().removeFrameCallback(this) @@ -545,7 +545,7 @@ internal class MapboxRenderThread : Choreographer.FrameCallback { internal fun destroy() { lock.withLock { // do nothing if destroy for some reason called more than once to avoid deadlock - if (renderHandlerThread.started) { + if (renderHandlerThread.isRunning) { renderHandlerThread.post { lock.withLock { if (renderCreated) { diff --git a/sdk/src/main/java/com/mapbox/maps/renderer/RenderHandlerThread.kt b/sdk/src/main/java/com/mapbox/maps/renderer/RenderHandlerThread.kt index 1dfa69c70b..8884328dbb 100644 --- a/sdk/src/main/java/com/mapbox/maps/renderer/RenderHandlerThread.kt +++ b/sdk/src/main/java/com/mapbox/maps/renderer/RenderHandlerThread.kt @@ -10,11 +10,14 @@ import com.mapbox.maps.logW internal class RenderHandlerThread { @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) - internal lateinit var handlerThread: HandlerThread + internal val handlerThread: HandlerThread = + HandlerThread(HANDLE_THREAD_NAME, THREAD_PRIORITY_DISPLAY) + + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) internal var handler: Handler? = null - internal val started - get() = handlerThread.isAlive + internal val isRunning + get() = handler != null && handlerThread.isAlive fun post(task: () -> Unit) { postDelayed(task, 0, EventType.DEFAULT) @@ -29,7 +32,6 @@ internal class RenderHandlerThread { } fun start(): Handler { - handlerThread = HandlerThread(HANDLE_THREAD_NAME, THREAD_PRIORITY_DISPLAY) handlerThread.start() return Handler(handlerThread.looper).also { handler = it diff --git a/sdk/src/test/java/com/mapbox/maps/renderer/MapboxRenderThreadTest.kt b/sdk/src/test/java/com/mapbox/maps/renderer/MapboxRenderThreadTest.kt index e0a244195b..300ac205f0 100644 --- a/sdk/src/test/java/com/mapbox/maps/renderer/MapboxRenderThreadTest.kt +++ b/sdk/src/test/java/com/mapbox/maps/renderer/MapboxRenderThreadTest.kt @@ -105,9 +105,10 @@ class MapboxRenderThreadTest { idleHandler() } - private fun mockCountdownRunnable(latch: CountDownLatch) = mockk(relaxUnitFun = true).also { - every { it.run() } answers { latch.countDown() } - } + private fun mockCountdownRunnable(latch: CountDownLatch) = + mockk(relaxUnitFun = true).also { + every { it.run() } answers { latch.countDown() } + } @After fun cleanup() { @@ -212,7 +213,7 @@ class MapboxRenderThreadTest { verifyOnce { eglCore.release() } verifyOnce { mapboxRenderer.destroyRenderer() } verifyOnce { fpsManager.destroy() } - assertFalse(renderHandlerThread.started) + assertFalse(renderHandlerThread.isRunning) } @Test @@ -227,7 +228,7 @@ class MapboxRenderThreadTest { verifyOnce { eglCore.release() } verifyOnce { mapboxRenderer.destroyRenderer() } verifyOnce { fpsManager.destroy() } - assertFalse(renderHandlerThread.started) + assertFalse(renderHandlerThread.isRunning) } @Test @@ -390,7 +391,7 @@ class MapboxRenderThreadTest { fun destroyTest() { initRenderThread() mapboxRenderThread.destroy() - assertFalse(renderHandlerThread.started) + assertFalse(renderHandlerThread.isRunning) } @Test @@ -876,4 +877,26 @@ class MapboxRenderThreadTest { mapboxRenderer.createRenderer() } } + + @Test(timeout = 10000) // Added timeout to ensure that if test fails, test does not hang forever. + fun onSurfaceWithActivityDestroyedBeforeSurfaceWithDestroyTaskInQueueTest() { + initRenderThread() + provideValidSurface() + waitZeroCounter { + val latch = this + // simulate real situation that `destroyRenderer` schedules some task in queue + // which was leading to a deadlock in `onSurfaceDestroyed` + every { mapboxRenderer.destroyRenderer() } answers { + renderHandlerThread.handler?.post { + Thread.sleep(500) + latch.countDown() + } + } + mapboxRenderThread.destroy() + mapboxRenderThread.onSurfaceDestroyed() + } + verifyOnce { eglCore.release() } + verifyOnce { mapboxRenderer.destroyRenderer() } + assertFalse(renderHandlerThread.isRunning) + } } \ No newline at end of file diff --git a/sdk/src/test/java/com/mapbox/maps/renderer/RenderHandlerThreadTest.kt b/sdk/src/test/java/com/mapbox/maps/renderer/RenderHandlerThreadTest.kt index aa25681cf1..719beef407 100644 --- a/sdk/src/test/java/com/mapbox/maps/renderer/RenderHandlerThreadTest.kt +++ b/sdk/src/test/java/com/mapbox/maps/renderer/RenderHandlerThreadTest.kt @@ -5,6 +5,7 @@ import com.mapbox.maps.logW import com.mapbox.verifyNo import io.mockk.* import org.junit.After +import org.junit.Assert.assertThrows import org.junit.Before import org.junit.Test import org.junit.runner.RunWith @@ -36,15 +37,44 @@ class RenderHandlerThreadTest { renderHandlerThread.start() assert(renderHandlerThread.handler != null) assert(renderHandlerThread.handlerThread.isAlive) + assert(renderHandlerThread.isRunning) } @Test - fun stopTest() { + fun startStartTest() { + renderHandlerThread.start() + assertThrows(IllegalThreadStateException::class.java) { + renderHandlerThread.start() + } + } + + @Test + fun startStopTest() { renderHandlerThread.start() renderHandlerThread.stop() assert(renderHandlerThread.handler == null) } + @Test + fun startStopStartTest() { + renderHandlerThread.start() + renderHandlerThread.stop() + assertThrows(IllegalThreadStateException::class.java) { + renderHandlerThread.start() + } + } + + @Test + fun isRunningTest() { + with(renderHandlerThread) { + assert(!isRunning) + start() + assert(isRunning) + stop() + assert(!isRunning) + } + } + @Test fun clearDefaultMessagesTest() { renderHandlerThread.start()