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 crash about several DataStores using the same file #2312

Merged
merged 5 commits into from
Jan 30, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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 @@ -33,6 +33,7 @@ interface SessionComponent : NodeFactoriesBindings {
interface Builder {
@BindsInstance
fun client(matrixClient: MatrixClient): Builder

fun build(): SessionComponent
}

Expand Down
1 change: 1 addition & 0 deletions changelog.d/2308.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix 'There are multiple DataStores active for the same file' crashes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am tempted to say that it's not necessary to add a file for the changelog, since the crash was not on the previous release.

Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* Copyright (c) 2024 New Vector Ltd
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.element.android.libraries.di.annotations

import javax.inject.Qualifier

/**
* Qualifies a [CoroutineScope] object which represents the base coroutine scope to use for an active session.
*/
@Retention(AnnotationRetention.RUNTIME)
@MustBeDocumented
@Qualifier
annotation class SessionCoroutineScope
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,14 @@ import io.element.android.libraries.matrix.api.sync.SyncService
import io.element.android.libraries.matrix.api.user.MatrixSearchUserResults
import io.element.android.libraries.matrix.api.user.MatrixUser
import io.element.android.libraries.matrix.api.verification.SessionVerificationService
import kotlinx.coroutines.CoroutineScope
import java.io.Closeable

interface MatrixClient : Closeable {
val sessionId: SessionId
val roomListService: RoomListService
val mediaLoader: MatrixMediaLoader
val sessionCoroutineScope: CoroutineScope
suspend fun getRoom(roomId: RoomId): MatrixRoom?
suspend fun findDM(userId: UserId): RoomId?
suspend fun ignoreUser(userId: UserId): Result<Unit>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,10 @@ class RustMatrixClient(
private val clock: SystemClock,
) : MatrixClient {
override val sessionId: UserId = UserId(client.userId())
override val sessionCoroutineScope = appCoroutineScope.childScope(dispatchers.main, "Session-$sessionId")

private val innerRoomListService = syncService.roomListService()
private val sessionDispatcher = dispatchers.io.limitedParallelism(64)
private val sessionCoroutineScope = appCoroutineScope.childScope(dispatchers.main, "Session-$sessionId")
private val rustSyncService = RustSyncService(syncService, sessionCoroutineScope)
private val verificationService = RustSessionVerificationService(rustSyncService, sessionCoroutineScope)
private val pushersService = RustPushersService(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,15 @@ import com.squareup.anvil.annotations.ContributesTo
import dagger.Module
import dagger.Provides
import io.element.android.libraries.di.SessionScope
import io.element.android.libraries.di.annotations.SessionCoroutineScope
import io.element.android.libraries.matrix.api.MatrixClient
import io.element.android.libraries.matrix.api.encryption.EncryptionService
import io.element.android.libraries.matrix.api.media.MatrixMediaLoader
import io.element.android.libraries.matrix.api.notificationsettings.NotificationSettingsService
import io.element.android.libraries.matrix.api.room.RoomMembershipObserver
import io.element.android.libraries.matrix.api.roomlist.RoomListService
import io.element.android.libraries.matrix.api.verification.SessionVerificationService
import kotlinx.coroutines.CoroutineScope

@Module
@ContributesTo(SessionScope::class)
Expand Down Expand Up @@ -60,4 +62,10 @@ object SessionMatrixModule {
fun provideMediaLoader(matrixClient: MatrixClient): MatrixMediaLoader {
return matrixClient.mediaLoader
}

@SessionCoroutineScope
@Provides
fun provideSessionCoroutineScope(matrixClient: MatrixClient): CoroutineScope {
return matrixClient.sessionCoroutineScope
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,13 @@ import io.element.android.libraries.matrix.test.roomlist.FakeRoomListService
import io.element.android.libraries.matrix.test.sync.FakeSyncService
import io.element.android.libraries.matrix.test.verification.FakeSessionVerificationService
import io.element.android.tests.testutils.simulateLongTask
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.delay
import kotlinx.coroutines.test.TestScope

class FakeMatrixClient(
override val sessionId: SessionId = A_SESSION_ID,
override val sessionCoroutineScope: CoroutineScope = TestScope(),
private val userDisplayName: Result<String> = Result.success(A_USER_NAME),
private val userAvatarUrl: Result<String> = Result.success(AN_AVATAR_URL),
override val roomListService: RoomListService = FakeRoomListService(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,22 +29,32 @@
import io.element.android.libraries.di.ApplicationContext
import io.element.android.libraries.di.SessionScope
import io.element.android.libraries.di.SingleIn
import io.element.android.libraries.di.annotations.SessionCoroutineScope
import io.element.android.libraries.matrix.api.core.SessionId
import io.element.android.libraries.matrix.api.user.CurrentSessionIdHolder
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.map
import java.io.File
import javax.inject.Inject

@ContributesBinding(SessionScope::class)
@SingleIn(SessionScope::class)
class DefaultSessionPreferencesStore @Inject constructor(
@ApplicationContext context: Context,
currentSessionIdHolder: CurrentSessionIdHolder,
@SessionCoroutineScope sessionCoroutineScope: CoroutineScope,
) : SessionPreferencesStore {
companion object {
fun storeFile(context: Context, sessionId: SessionId): File {
val hashedUserId = sessionId.value.hash().take(16)
return context.preferencesDataStoreFile("session_${hashedUserId}_preferences")

Check warning on line 51 in libraries/preferences/impl/src/main/kotlin/io/element/android/libraries/preferences/impl/store/DefaultSessionPreferencesStore.kt

View check run for this annotation

Codecov / codecov/patch

libraries/preferences/impl/src/main/kotlin/io/element/android/libraries/preferences/impl/store/DefaultSessionPreferencesStore.kt#L50-L51

Added lines #L50 - L51 were not covered by tests
}
}
private val sendPublicReadReceiptsKey = booleanPreferencesKey("sendPublicReadReceipts")
private val hashedUserId = currentSessionIdHolder.current.value.hash().take(16)

private val dataStoreFile = context.preferencesDataStoreFile("session_${hashedUserId}_preferences")
private val store = PreferenceDataStoreFactory.create { dataStoreFile }
private val dataStoreFile = storeFile(context, currentSessionIdHolder.current)
private val store = PreferenceDataStoreFactory.create(scope = sessionCoroutineScope) { dataStoreFile }

Check warning on line 57 in libraries/preferences/impl/src/main/kotlin/io/element/android/libraries/preferences/impl/store/DefaultSessionPreferencesStore.kt

View check run for this annotation

Codecov / codecov/patch

libraries/preferences/impl/src/main/kotlin/io/element/android/libraries/preferences/impl/store/DefaultSessionPreferencesStore.kt#L56-L57

Added lines #L56 - L57 were not covered by tests

override suspend fun setSendPublicReadReceipts(enabled: Boolean) = update(sendPublicReadReceiptsKey, enabled)
override fun isSendPublicReadReceiptsEnabled(): Flow<Boolean> = get(sendPublicReadReceiptsKey, true)
Expand Down
Loading