Skip to content

Commit b5daa4d

Browse files
terrakokterrakok
committed
[resources] Check cached deferreds and drop them if they are cancelled. (#4819)
Before the fix we could cancel a coroutine and the cancelled deferred was saved in cache. ## Release Notes ### Fixes - Resources - _(prerelease fix)_ Fix a cached empty resource on a Compose for Web if the resource loading was canceled during progress (cherry picked from commit dbab893)
1 parent abfd6c9 commit b5daa4d

File tree

4 files changed

+57
-64
lines changed

4 files changed

+57
-64
lines changed

‎components/resources/library/src/commonMain/kotlin/org/jetbrains/compose/resources/AsyncCache.kt‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
package org.jetbrains.compose.resources
2+
3+
import kotlinx.coroutines.CoroutineStart
4+
import kotlinx.coroutines.Deferred
5+
import kotlinx.coroutines.async
6+
import kotlinx.coroutines.coroutineScope
7+
import kotlinx.coroutines.sync.Mutex
8+
import kotlinx.coroutines.sync.withLock
9+
10+
internal class AsyncCache<K, V> {
11+
private val mutex = Mutex()
12+
private val cache = mutableMapOf<K, Deferred<V>>()
13+
14+
suspend fun getOrLoad(key: K, load: suspend () -> V): V = coroutineScope {
15+
val deferred = mutex.withLock {
16+
var cached = cache[key]
17+
if (cached == null || cached.isCancelled) {
18+
//LAZY - to free the mutex lock as fast as possible
19+
cached = async(start = CoroutineStart.LAZY) { load() }
20+
cache[key] = cached
21+
}
22+
cached
23+
}
24+
deferred.await()
25+
}
26+
27+
//@TestOnly
28+
fun clear() {
29+
cache.clear()
30+
}
31+
}

‎components/resources/library/src/commonMain/kotlin/org/jetbrains/compose/resources/ImageResources.kt‎

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
package org.jetbrains.compose.resources
22

3-
import androidx.compose.runtime.*
3+
import androidx.compose.runtime.Composable
4+
import androidx.compose.runtime.Immutable
5+
import androidx.compose.runtime.getValue
6+
import androidx.compose.runtime.remember
47
import androidx.compose.ui.graphics.ImageBitmap
58
import androidx.compose.ui.graphics.painter.BitmapPainter
69
import androidx.compose.ui.graphics.painter.Painter
@@ -9,9 +12,6 @@ import androidx.compose.ui.graphics.vector.rememberVectorPainter
912
import androidx.compose.ui.platform.LocalDensity
1013
import androidx.compose.ui.unit.Density
1114
import androidx.compose.ui.unit.dp
12-
import kotlinx.coroutines.*
13-
import kotlinx.coroutines.sync.Mutex
14-
import kotlinx.coroutines.sync.withLock
1515
import org.jetbrains.compose.resources.vector.toImageVector
1616
import org.jetbrains.compose.resources.vector.xmldom.Element
1717

@@ -91,6 +91,7 @@ fun vectorResource(resource: DrawableResource): ImageVector {
9191
}
9292

9393
internal expect class SvgElement
94+
9495
internal expect fun SvgElement.toSvgPainter(density: Density): Painter
9596

9697
private val emptySvgPainter: Painter by lazy { BitmapPainter(emptyImageBitmap) }
@@ -135,8 +136,7 @@ private sealed interface ImageCache {
135136
class Svg(val painter: Painter) : ImageCache
136137
}
137138

138-
private val imageCacheMutex = Mutex()
139-
private val imageCache = mutableMapOf<String, Deferred<ImageCache>>()
139+
private val imageCache = AsyncCache<String, ImageCache>()
140140

141141
//@TestOnly
142142
internal fun dropImageCache() {
@@ -147,14 +147,4 @@ private suspend fun loadImage(
147147
path: String,
148148
resourceReader: ResourceReader,
149149
decode: (ByteArray) -> ImageCache
150-
): ImageCache = coroutineScope {
151-
val deferred = imageCacheMutex.withLock {
152-
imageCache.getOrPut(path) {
153-
//LAZY - to free the mutex lock as fast as possible
154-
async(start = CoroutineStart.LAZY) {
155-
decode(resourceReader.read(path))
156-
}
157-
}
158-
}
159-
deferred.await()
160-
}
150+
): ImageCache = imageCache.getOrLoad(path) { decode(resourceReader.read(path)) }

‎components/resources/library/src/commonMain/kotlin/org/jetbrains/compose/resources/StringResourcesUtils.kt‎

Lines changed: 16 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,6 @@
11
package org.jetbrains.compose.resources
22

3-
import kotlinx.coroutines.*
4-
import kotlinx.coroutines.sync.Mutex
5-
import kotlinx.coroutines.sync.withLock
63
import org.jetbrains.compose.resources.plural.PluralCategory
7-
import org.jetbrains.compose.resources.vector.xmldom.Element
8-
import org.jetbrains.compose.resources.vector.xmldom.NodeList
94
import kotlin.io.encoding.Base64
105
import kotlin.io.encoding.ExperimentalEncodingApi
116

@@ -20,8 +15,7 @@ internal sealed interface StringItem {
2015
data class Array(val items: List<String>) : StringItem
2116
}
2217

23-
private val stringsCacheMutex = Mutex()
24-
private val stringItemsCache = mutableMapOf<String, Deferred<StringItem>>()
18+
private val stringItemsCache = AsyncCache<String, StringItem>()
2519
//@TestOnly
2620
internal fun dropStringItemsCache() {
2721
stringItemsCache.clear()
@@ -30,28 +24,22 @@ internal fun dropStringItemsCache() {
3024
internal suspend fun getStringItem(
3125
resourceItem: ResourceItem,
3226
resourceReader: ResourceReader
33-
): StringItem = coroutineScope {
34-
val deferred = stringsCacheMutex.withLock {
35-
stringItemsCache.getOrPut("${resourceItem.path}/${resourceItem.offset}-${resourceItem.size}") {
36-
//LAZY - to free the mutex lock as fast as possible
37-
async(start = CoroutineStart.LAZY) {
38-
val record = resourceReader.readPart(
39-
resourceItem.path,
40-
resourceItem.offset,
41-
resourceItem.size
42-
).decodeToString()
43-
val recordItems = record.split('|')
44-
val recordType = recordItems.first()
45-
val recordData = recordItems.last()
46-
when (recordType) {
47-
"plurals" -> recordData.decodeAsPlural()
48-
"string-array" -> recordData.decodeAsArray()
49-
else -> recordData.decodeAsString()
50-
}
51-
}
52-
}
27+
): StringItem = stringItemsCache.getOrLoad(
28+
key = "${resourceItem.path}/${resourceItem.offset}-${resourceItem.size}"
29+
) {
30+
val record = resourceReader.readPart(
31+
resourceItem.path,
32+
resourceItem.offset,
33+
resourceItem.size
34+
).decodeToString()
35+
val recordItems = record.split('|')
36+
val recordType = recordItems.first()
37+
val recordData = recordItems.last()
38+
when (recordType) {
39+
"plurals" -> recordData.decodeAsPlural()
40+
"string-array" -> recordData.decodeAsArray()
41+
else -> recordData.decodeAsString()
5342
}
54-
deferred.await()
5543
}
5644

5745
@OptIn(ExperimentalEncodingApi::class)

‎components/resources/library/src/commonMain/kotlin/org/jetbrains/compose/resources/plural/PluralRuleList.kt‎

Lines changed: 3 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,7 @@
55

66
package org.jetbrains.compose.resources.plural
77

8-
import kotlinx.coroutines.CoroutineStart
9-
import kotlinx.coroutines.Deferred
10-
import kotlinx.coroutines.async
11-
import kotlinx.coroutines.coroutineScope
12-
import kotlinx.coroutines.sync.Mutex
13-
import kotlinx.coroutines.sync.withLock
8+
import org.jetbrains.compose.resources.AsyncCache
149
import org.jetbrains.compose.resources.InternalResourceApi
1510
import org.jetbrains.compose.resources.LanguageQualifier
1611
import org.jetbrains.compose.resources.RegionQualifier
@@ -21,8 +16,7 @@ internal class PluralRuleList(private val rules: Array<PluralRule>) {
2116
}
2217

2318
companion object {
24-
private val cacheMutex = Mutex()
25-
private val cache = Array<Deferred<PluralRuleList>?>(cldrPluralRuleLists.size) { null }
19+
private val cache = AsyncCache<Int, PluralRuleList>()
2620
private val emptyList = PluralRuleList(emptyArray())
2721

2822
@OptIn(InternalResourceApi::class)
@@ -36,17 +30,7 @@ internal class PluralRuleList(private val rules: Array<PluralRule>) {
3630

3731
suspend fun getInstance(cldrLocaleName: String): PluralRuleList {
3832
val listIndex = cldrPluralRuleListIndexByLocale[cldrLocaleName]!!
39-
return coroutineScope {
40-
val deferred = cacheMutex.withLock {
41-
if (cache[listIndex] == null) {
42-
cache[listIndex] = async(start = CoroutineStart.LAZY) {
43-
createInstance(listIndex)
44-
}
45-
}
46-
cache[listIndex]!!
47-
}
48-
deferred.await()
49-
}
33+
return cache.getOrLoad(listIndex) { createInstance(listIndex) }
5034
}
5135

5236
@OptIn(InternalResourceApi::class)

0 commit comments

Comments
 (0)