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

[BUG] MutableStore occasionally hangs when cancelling streams #657

Closed
amrfarid140 opened this issue Oct 1, 2024 · 1 comment · Fixed by #658
Closed

[BUG] MutableStore occasionally hangs when cancelling streams #657

amrfarid140 opened this issue Oct 1, 2024 · 1 comment · Fixed by #658
Labels
bug Something isn't working

Comments

@amrfarid140
Copy link
Contributor

amrfarid140 commented Oct 1, 2024

Describe the bug
MutableStore doesn't emit any values If you have a Store and you cancel flows then start new ones using store.stream.

I believe this is due to a deadlock that happens where one the locks remains locked when a cancellation exception is propagated. I've wrote a simple test to reproduce it.

We have an iOS app and use Kotlin multiplatform and that's where we noticed the issue. One of our flows cancel and relaunches a MutableStore streams and we noticed the whole app just doesn't get any data from the store.

This is only happening on a MutableStore. If you switch the provided test into a non-mutable store the test consistently passes.

// This is test class was chosen just of convenience. It already exist in the "Store" repo.
class StoreWithInMemoryCacheTests {
....


@Test
    fun storeDeadlock() =
        testScope.runTest {
            repeat(1000) {
                val store =
                    StoreBuilder
                        .from(
                            fetcher = Fetcher.of { key: Int -> "fetcher_${key}" },
                            sourceOfTruth = SourceOfTruth.Companion.of(
                                reader = { key ->
                                    flow<String> {
                                        emit("source_of_truth_${key}")
                                    }
                                },
                                writer = { key: Int, local: String ->

                                }
                            )
                        )
                        .disableCache()
                        .toMutableStoreBuilder(
                            converter = object : Converter<String, String, String> {
                                override fun fromNetworkToLocal(network: String): String {
                                    return network
                                }

                                override fun fromOutputToLocal(output: String): String {
                                    return output
                                }
                            },
                        )
                        .build(
                            updater = object : Updater<Int, String, Unit> {
                                var callCount = -1
                                override suspend fun post(key: Int, value: String): UpdaterResult {
                                    callCount += 1
                                    if (callCount % 2 == 0) {
                                        throw IllegalArgumentException(key.toString() + "value:$value")
                                    } else {
                                        return UpdaterResult.Success.Untyped("")
                                    }
                                }

                                override val onCompletion: OnUpdaterCompletion<Unit>?
                                    get() = null

                            }
                        )

                val jobs = mutableListOf<Job>()
                jobs.add(
                    store.stream<Nothing>(StoreReadRequest.cached(1, refresh = true))
                        .mapNotNull { it.dataOrNull() }
                        .launchIn(CoroutineScope(Dispatchers.Default))
                )
                val job1 = store.stream<Nothing>(StoreReadRequest.cached(0, refresh = true))
                    .mapNotNull { it.dataOrNull() }
                    .launchIn(CoroutineScope(Dispatchers.Default))
                jobs.add(
                    store.stream<Nothing>(StoreReadRequest.cached(2, refresh = true))
                        .mapNotNull { it.dataOrNull() }
                        .launchIn(CoroutineScope(Dispatchers.Default)))
                jobs.add(
                    store.stream<Nothing>(StoreReadRequest.cached(3, refresh = true))
                        .mapNotNull { it.dataOrNull() }
                        .launchIn(CoroutineScope(Dispatchers.Default)))
                job1.cancel()
                assertEquals(
                    expected = "source_of_truth_0",
                    actual = store.stream<Nothing>(StoreReadRequest.cached(0, refresh = true))
                        .mapNotNull { it.dataOrNull() }.first()
                )
                jobs.forEach {
                    it.cancel()
                    assertEquals(
                        expected = "source_of_truth_0",
                        actual = store.stream<Nothing>(StoreReadRequest.cached(0, refresh = true))
                            .mapNotNull { it.dataOrNull() }.first()
                    )
                }
            }
        }

To Reproduce
Steps to reproduce the behavior:

  1. Go to StoreWithInMemoryCacheTests
  2. Add the provided test
  3. Run it with :store:cleanIosSimulatorArm64Test :store:iosSimulatorArm64Test --tests "org.mobilenativefoundation.store.store5.StoreWithInMemoryCacheTests.storeDeadlock" or :store:testDebugUnitTest --tests "org.mobilenativefoundation.store.store5.StoreWithInMemoryCacheTests.storeDeadlock"
  4. See test sometimes timing out and sometimes succeeding (with low repeat count).

Expected behavior
Test always succeeds

Screenshots
If applicable, add screenshots to help explain your problem.

Smartphone (please complete the following information):

  • OS: iOS and Android
  • Store Version: 5.0.0-beta02 and 5.0.0-alpha04
@jacobian2020
Copy link

Calling store.get() also hangs indefinitely when there an error in converter, this might be related.

@github-project-automation github-project-automation bot moved this from 🆕 Triage to ✅ Done in Store Roadmap Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants