Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: exception when executing multiple fetches of the same avatar [WPB-9640] #3101

Merged
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ import com.wire.kalium.logic.feature.asset.GetAvatarAssetUseCase
import com.wire.kalium.logic.feature.asset.GetMessageAssetUseCase
import com.wire.kalium.logic.feature.asset.MessageAssetResult
import com.wire.kalium.logic.feature.asset.PublicAssetResult
import kotlinx.coroutines.sync.Mutex
import kotlinx.coroutines.sync.withLock
import java.util.concurrent.ConcurrentHashMap

internal class AssetImageFetcher(
private val assetFetcherParameters: AssetFetcherParameters,
Expand All @@ -45,7 +48,9 @@ internal class AssetImageFetcher(
private const val DEFAULT_RETRY_ATTEMPT = 0
}

override suspend fun fetch(): FetchResult? {
override suspend fun fetch(): FetchResult = MutexMap.withLock(assetFetcherParameters.data.uniqueKey, ::fetchJob)

private suspend fun fetchJob(): FetchResult {
with(assetFetcherParameters) {
return when (data) {
is ImageAsset.UserAvatarAsset -> {
Expand Down Expand Up @@ -115,3 +120,42 @@ enum class AssetImageRetryPolicy {
EXPONENTIAL_RETRY_WHEN_CONNECTED,
DO_NOT_RETRY
}

/**
* Creates a mutex associated with a key and locks it so that there's only one execution going for a given key at a given time.
* When the lock for the given key is executed for the first time, it will create a new entry in the map and lock the mutex.
* When another lock is executed for the same key while it's locked, it will increase the count and lock the mutex so that it'll wait
* and execute after the first execution unlocks the mutex. The count is there to keep the mutex in the map as long as it's needed.
* After the last unlock, the mutex is removed from the map.
*/
private object MutexMap {
private val assetMutex = ConcurrentHashMap<String, Pair<Int, Mutex>>()

suspend fun <T> withLock(key: String, action: suspend () -> T): T =
increaseCountAndGetMutex(key).let { (_, mutex) ->
mutex.withLock {
action().also {
decreaseCountAndRemoveMutexIfNeeded(key)
}
}
}

private fun increaseCountAndGetMutex(key: String): Pair<Int, Mutex> =
assetMutex.compute(key) { _, value ->
((value ?: (0 to Mutex()))).let { (count, mutex) ->
count + 1 to mutex
}
}!!

private fun decreaseCountAndRemoveMutexIfNeeded(key: String) {
assetMutex.compute(key) { _, value ->
value?.let { (count, mutex) ->
if (count <= 1) {
null
} else {
count - 1 to mutex
}
}
}
}
}
Loading