https://kotlinlang.org logo
#coroutines
Title
# coroutines
s

Sergey Shevtsov

02/13/2023, 4:31 PM
Hi everyone! Help me, please. How would you solve this issue? I have a class which is a wrapper around the shared preferences. I added reactivity there with callbackFlow. It looks like this:
Copy code
private val preferences = // shared preferences initialization

private val mutex = Mutex()

override fun observeString(prefKey: String, defaultValue: String?): Flow<String?> {
  return observeValueChanges(prefKey).mapWithLock {
    preferences.getString(prefKey, defaultValue)
  }.flowOn(<http://Dispatchers.IO|Dispatchers.IO>)
}

override fun observeBoolean(prefKey: String, defaultValue: Boolean): Flow<Boolean> {
  return observeValueChanges(prefKey).mapWithLock {
    preferences.getBoolean(prefKey, defaultValue)
  }.flowOn(<http://Dispatchers.IO|Dispatchers.IO>)
}

private fun observeValueChanges(prefKey: String): Flow<Unit> {
  return callbackFlow {
    val listener = OnSharedPreferenceChangeListener { _, key ->
      if (prefKey == key) {
        trySend(Unit)
      }
    }
    preferences.registerOnSharedPreferenceChangeListener(listener)
    send(Unit)
    awaitClose { preferences.unregisterOnSharedPreferenceChangeListener(listener) }
  }
}

private inline fun <T, R> Flow<T>.mapWithLock(crossinline transform: suspend (T) -> R): Flow<R> {
  return transform { value ->
    return@transform emit(
      mutex.withLock {
        transform(value)
      }
    )
  }
}
Coroutines mutex locking is used for synchronized write/read operations to avoid ConcurrentModificationException. The issue is that registering/unregistering a listener is not thread safe. I can't just synchronize this with the coroutine mutex, because the awaitClose lambda is not suspend. Possible solutions, in my opinion, are the following: 1. Creating a single thread dispatcher and calling registering/unregistering on them. 2. Just to use
synchronized { }
, but here's the question. How good is this practice of mixing concurrency of coroutines and no-coroutines? I would be glad to see more elegant solutions, if any. Thank you.
n

Nick Allen

02/13/2023, 5:57 PM
SharedPreferences appears to be thread safe: https://android.googlesource.com/platform/frameworks/base.git/+/master/core/java/android/app/SharedPreferencesImpl.java#256 Why do you think it'll throw a ConcurrentModificationException exception?
synchronized { ... }
is fine to call with coroutines so long as you don't include suspend methods in the synchronized block, in which case, it has all the same benefits and risks as when used with non-coroutine code.
d

denis090712

02/13/2023, 6:10 PM
ConcurrentModificationException is not about coroutines or thread-safe access. It means that the collection you are currently iterating has changed. Just try to copy your original collection and then use it. Anyway, without a full stack trace, I can’t say what collection it is and whether there is a problem with SharedPreferences implementation.
s

Sergey Shevtsov

02/13/2023, 6:58 PM
Maybe I didn't express myself clearly) The issue with the ConcurrentModificationException has been solved with the coroutines mutex. I was getting this exception because I was modifying the pref file and reading it at the same time. I asked about registering/unregistering of listeners. Without any kind of synchronization, these operations may be performed in the wrong order. I'm looking for a good way to synchronize them.
In any case, thanks for your answers) I'm more inclined to use the
synchronized { ... }
option.
d

denis090712

02/13/2023, 7:24 PM
I wouldn’t recommend you use synchronized or any other java synchronization primitives with coroutines if you don’t fully understand their impact. If you don’t want to run into a deadlock or get other unexpected behaviour use thread confinement instead to achieve the same result.
n

Nick Allen

02/13/2023, 10:54 PM
The issue with the ConcurrentModificationException has been solved with the coroutines mutex.
That's seems unlikely given that those methods are already thread safe. The entire bodies of
getString
and
getBoolean
are wrapped in
synchronized
blocks when I peek at the AOSP.
I asked about registering/unregistering of listeners. Without any kind of synchronization, these operations may be performed in the wrong order. I'm looking for a good way to synchronize them.
Same. Also, code above
awaitClose
will run entirely before code inside the
awaitClose
block.
u

uli

02/14/2023, 8:18 PM
Are you using SharedPreferences? Or EncryptedSharedPreferences?
s

Sergey Shevtsov

02/16/2023, 3:27 PM
Forgot to mention) But it's important. My bad. I'm using EncryptedSharedPreferences. They are not thread safe like SharedPreferences.
u

uli

02/16/2023, 3:57 PM
Yes, that’s evil. A drop in replacement that breaks the contract.
Not only is it not thread safe, also the listeners are not called in the main thread as documented here, but within the call stack and thread of the
editor.commit()/.apply()
call!
And almost no KDoc at all which makes you look at the SharedPreferences KDoc
Maybe you can use another flow like
Copy code
preferences.registerOnSharedPreferenceChangeListener(listener)
    send(Unit)
    var registered = MutableStateFlow<Boolean>(true)
    launch {
        registered.filter{ it.not }
        .first {
            preferences.unregisterOnSharedPreferenceChangeListener(listener)
        }
    }
    awaitClose { registered = false }
^^^Take that just as a sketch of the idea. This code has not seen an ide 🙂 Nah, then you could also just launch form awaitClose, both will probably give you other races
another raw idea: • register a single shared flow on the preferences that starts on first collection // This will avoid the concurrent modification of the listeners List • then instead of registering a furhter listener, you just start collecting the shared flow
70 Views