oppia / oppia-android

A free, online & offline learning platform to make quality education accessible for all.
https://www.oppia.org
Apache License 2.0
300 stars 502 forks source link

[BUG]: InMemoryBlockingCache is not thread-safe #5354

Open masclot opened 3 months ago

masclot commented 3 months ago

Describe the bug

InMemoryBlockingCache needs to process CRUD operations sequentially. To do so, it launches coroutines in a scope with a single-threaded dispatcher. However, using a single-thread does not guarantee sequentiality nor atomicity when combined with coroutines:

Potential solutions include:

Steps To Reproduce

No steps, errors can happen randomly. Might be related to #5064.

Expected Behavior

The class should be thread-safe

Screenshots/Videos

No response

What device/emulator are you using?

No response

Which Android version is your device/emulator running?

No response

Which version of the Oppia Android app are you using?

No response

Additional Context

No response

masclot commented 3 months ago

Sample test within InMemoryBlockingCacheTest that would fail with the current implementation:

  @Test
  fun testSequentiality() = runBlocking {
    val first = Job()
    val second = Job()

    val cache = cacheFactory.create<String>()
    val firstUpdate = cache.updateAsync {
      first.join()
      "first"
    }
    val secondUpdate = cache.updateAsync {
      second.join()
      "second"
    }
    second.complete()
    testCoroutineDispatchers.advanceUntilIdle()
    first.complete()
    testCoroutineDispatchers.advanceUntilIdle()
    firstUpdate.await()
    secondUpdate.await()
    assertThat(awaitCompletion(cache.readAsync())).isEqualTo("second")
  }
BenHenning commented 3 months ago

Ah I didn't realize that Job could be used as a stand-in replacement for a completable future--that's actually really useful to know.

It's great you devised a test that should fail. It looks like it should be consistent, but I can't be sure without running it. That should be a great test to use for high confidence verification of the final implementation.