diff --git a/components/resources/library/src/commonMain/kotlin/org/jetbrains/compose/resources/ResourceCaches.kt b/components/resources/library/src/commonMain/kotlin/org/jetbrains/compose/resources/ResourceCaches.kt index 6e96b3fa6d..b9f463b156 100644 --- a/components/resources/library/src/commonMain/kotlin/org/jetbrains/compose/resources/ResourceCaches.kt +++ b/components/resources/library/src/commonMain/kotlin/org/jetbrains/compose/resources/ResourceCaches.kt @@ -5,7 +5,7 @@ import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock internal class AsyncCache { - private val cacheScope = CoroutineScope(SupervisorJob()) + private val cacheScope = CoroutineScope(SupervisorJob() + Dispatchers.Unconfined) private val mutex = Mutex() private val cache = mutableMapOf>() @@ -25,13 +25,15 @@ internal class AsyncCache { val request = mutex.withLock { var cached = cache[key] if (cached == null || cached.deferred.isCancelled) { - cached = SharedRequest(cacheScope.async { load() }) + //the request is created lazily to start it outside the critical mutex section + cached = SharedRequest(cacheScope.async(start = CoroutineStart.LAZY) { load() }) cache[key] = cached } cached.listenersCount++ cached } return try { + request.deferred.start() request.deferred.await() } finally { mutex.withLock { diff --git a/components/resources/library/src/commonMain/kotlin/org/jetbrains/compose/resources/ResourceReader.kt b/components/resources/library/src/commonMain/kotlin/org/jetbrains/compose/resources/ResourceReader.kt index aa2467ec18..084e9c0050 100644 --- a/components/resources/library/src/commonMain/kotlin/org/jetbrains/compose/resources/ResourceReader.kt +++ b/components/resources/library/src/commonMain/kotlin/org/jetbrains/compose/resources/ResourceReader.kt @@ -26,6 +26,15 @@ suspend fun readResourceBytes(path: String): ByteArray = DefaultResourceReader.r @InternalResourceApi fun getResourceUri(path: String): String = DefaultResourceReader.getUri(path) +/** + * Interface for reading resource files. + * + * **Warning:** + * - On all platforms except Web, synchronous loading of resources is expected inside this method. + * - On Android and JVM, implementations must not switch to the Main dispatcher + * (e.g. via `withContext(Dispatchers.Main)`) as this can cause a deadlock! + * See: `DeadlockReproducer.kt` + */ @ExperimentalResourceApi interface ResourceReader { suspend fun read(path: String): ByteArray diff --git a/components/resources/library/src/desktopTest/kotlin/org/jetbrains/compose/resources/DeadlockReproducer.kt b/components/resources/library/src/desktopTest/kotlin/org/jetbrains/compose/resources/DeadlockReproducer.kt new file mode 100644 index 0000000000..6a8734cb55 --- /dev/null +++ b/components/resources/library/src/desktopTest/kotlin/org/jetbrains/compose/resources/DeadlockReproducer.kt @@ -0,0 +1,73 @@ +package org.jetbrains.compose.resources + +import androidx.compose.foundation.Image +import androidx.compose.runtime.CompositionLocalProvider +import androidx.compose.ui.test.ExperimentalTestApi +import androidx.compose.ui.test.v2.runComposeUiTest +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext +import java.util.concurrent.atomic.AtomicBoolean +import kotlin.concurrent.thread +import kotlin.test.Test +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +@OptIn(ExperimentalTestApi::class) +class DeadlockReproducer { + + @Test + fun deadlockReproducer() { + val deadlockResourceReader = object : ResourceReader by DefaultResourceReader { + override suspend fun read(path: String): ByteArray { + val arr = DefaultResourceReader.read(path) + return withContext(Dispatchers.Main) { arr } + } + } + val isComposed = AtomicBoolean(false) + val isDeadlocked = AtomicBoolean(true) + + val testThread = thread { + runComposeUiTest { + setContent { + CompositionLocalProvider( + LocalResourceReader provides deadlockResourceReader, + LocalComposeEnvironment provides TestComposeEnvironment + ) { + isComposed.set(true) + Image(painterResource(TestDrawableResource("1.png")), null) + } + } + isDeadlocked.set(false) + } + } + + testThread.join(300) + assertTrue(isComposed.get(), "Composition is not composed") + assertTrue(isDeadlocked.get(), "Deadlock was not detected") + } + + @Test + fun noDeadlockReproducer() { + val isComposed = AtomicBoolean(false) + val isDeadlocked = AtomicBoolean(true) + + val testThread = thread { + runComposeUiTest { + setContent { + CompositionLocalProvider( + LocalResourceReader provides DefaultResourceReader, + LocalComposeEnvironment provides TestComposeEnvironment + ) { + isComposed.set(true) + Image(painterResource(TestDrawableResource("1.png")), null) + } + } + isDeadlocked.set(false) + } + } + + testThread.join(300) + assertTrue(isComposed.get(), "Composition is not composed") + assertFalse(isDeadlocked.get(), "Deadlock was detected") + } +} diff --git a/components/resources/library/src/jvmAndAndroidTest/kotlin/org/jetbrains/compose/resources/ResourceLoadingThreadTest.kt b/components/resources/library/src/jvmAndAndroidTest/kotlin/org/jetbrains/compose/resources/ResourceLoadingThreadTest.kt new file mode 100644 index 0000000000..fa637fd596 --- /dev/null +++ b/components/resources/library/src/jvmAndAndroidTest/kotlin/org/jetbrains/compose/resources/ResourceLoadingThreadTest.kt @@ -0,0 +1,65 @@ +package org.jetbrains.compose.resources + +import kotlinx.coroutines.* +import kotlinx.coroutines.test.runTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertSame +import kotlin.time.Duration.Companion.milliseconds + +class ResourceLoadingThreadTest { + + @Test + fun testResourceIsLoadedOnTheCallingThread() { + val callerThread = Thread.currentThread() + val cache = AsyncCache() + + val loadingThread = runResourceBlocking { + cache.getOrLoad("key") { Thread.currentThread() } + } + + assertSame(callerThread, loadingThread, "A resource must be loaded on the calling thread") + } + + @Test + fun testTheSharedRequestIsNotCancelledByTheFirstCaller() = runTest { + val cache = AsyncCache() + val allowLoadToFinish = CompletableDeferred() + lateinit var result: String + + val firstCaller = GlobalScope.launch(Dispatchers.Default) { + cache.getOrLoad("key") { + allowLoadToFinish.await() + "a value" + } + } + val secondCaller = GlobalScope.launch(Dispatchers.Default) { + result = cache.getOrLoad("key") { + error("must reuse the shared request") + } + } + + delay(100.milliseconds) + firstCaller.cancel() + allowLoadToFinish.complete(Unit) + secondCaller.join() + + assertEquals("a value", result) + } + + @Test + fun testWithContextInsideAReaderDoesNotDeadlock() = runTest { + val cache = AsyncCache() + + val value = withTimeoutOrNull(100.milliseconds) { + runResourceBlocking { + cache.getOrLoad("key") { + //a custom resource reader is free to move its IO to another dispatcher + withContext(Dispatchers.IO) { delay(50); "a value" } + } + } + } + + assertEquals("a value", value) + } +}