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

Ensure sync thread is started #6884

Merged
merged 9 commits into from
Aug 22, 2022
Merged
Show file tree
Hide file tree
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
1 change: 1 addition & 0 deletions changelog.d/6884.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Ensure SyncThread is started when the app is launched after a Push has been received.
1 change: 1 addition & 0 deletions changelog.d/6884.sdk
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Rename `DebugService.logDbUsageInfo` (resp. `Session.logDbUsageInfo`) to `DebugService.getDbUsageInfo` (resp. `Session.getDbUsageInfo`) and return a String instead of logging. The caller may want to log the String.
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ interface DebugService {
fun getAllRealmConfigurations(): List<RealmConfiguration>

/**
* Prints out info on DB size to logcat.
* Get info on DB size.
*/
fun logDbUsageInfo()
fun getDbUsageInfo(): String
}
Original file line number Diff line number Diff line change
Expand Up @@ -323,9 +323,9 @@ interface Session {
fun getUiaSsoFallbackUrl(authenticationSessionId: String): String

/**
* Debug API, will print out info on DB size to logcat.
* Debug API, will return info about the DB.
*/
fun logDbUsageInfo()
fun getDbUsageInfo(): String

/**
* Debug API, return the list of all RealmConfiguration used by this session.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,11 @@ interface SyncService {
*/
fun getSyncState(): SyncState

/**
* This method returns true if the sync thread is alive, i.e. started.
*/
fun isSyncThreadAlive(): Boolean

/**
* This method allows to listen the sync state.
* @return a [LiveData] of [SyncState].
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ internal abstract class RealmLiveEntityObserver<T : RealmObject>(protected val r
LiveEntityObserver, RealmChangeListener<RealmResults<T>> {

private companion object {
val BACKGROUND_HANDLER = createBackgroundHandler("LIVE_ENTITY_BACKGROUND")
val BACKGROUND_HANDLER = createBackgroundHandler("Matrix-LIVE_ENTITY_BACKGROUND")
}

protected val observerScope = CoroutineScope(SupervisorJob() + BACKGROUND_HANDLER.asCoroutineDispatcher())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,15 @@ package org.matrix.android.sdk.internal.database.tools
import io.realm.Realm
import io.realm.RealmConfiguration
import org.matrix.android.sdk.BuildConfig
import timber.log.Timber

internal class RealmDebugTools(
private val realmConfiguration: RealmConfiguration
) {
/**
* Log info about the DB.
* Get info about the DB.
*/
fun logInfo(baseName: String) {
buildString {
fun getInfo(baseName: String): String {
return buildString {
append("\n$baseName Realm located at : ${realmConfiguration.realmDirectory}/${realmConfiguration.realmFileName}")

if (BuildConfig.LOG_PRIVATE_DATA) {
Expand All @@ -54,7 +53,6 @@ internal class RealmDebugTools(
separator()
}
}
.let { Timber.i(it) }
}

private fun StringBuilder.separator() = append("\n==============================================")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ internal class DefaultDebugService @Inject constructor(
realmConfigurationGlobal
}

override fun logDbUsageInfo() {
RealmDebugTools(realmConfigurationAuth).logInfo("Auth")
RealmDebugTools(realmConfigurationGlobal).logInfo("Global")
sessionManager.getLastSession()?.logDbUsageInfo()
override fun getDbUsageInfo() = buildString {
append(RealmDebugTools(realmConfigurationAuth).getInfo("Auth"))
append(RealmDebugTools(realmConfigurationGlobal).getInfo("Global"))
append(sessionManager.getLastSession()?.getDbUsageInfo())
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ internal object MatrixModule {
io = Dispatchers.IO,
computation = Dispatchers.Default,
main = Dispatchers.Main,
crypto = createBackgroundHandler("Crypto_Thread").asCoroutineDispatcher(),
crypto = createBackgroundHandler("Matrix-Crypto_Thread").asCoroutineDispatcher(),
dmVerif = Executors.newSingleThreadExecutor().asCoroutineDispatcher()
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -263,11 +263,11 @@ internal class DefaultSession @Inject constructor(
}
}

override fun logDbUsageInfo() {
RealmDebugTools(realmConfiguration).logInfo("Session")
RealmDebugTools(realmConfigurationCrypto).logInfo("Crypto")
RealmDebugTools(realmConfigurationIdentity).logInfo("Identity")
RealmDebugTools(realmConfigurationContentScanner).logInfo("ContentScanner")
override fun getDbUsageInfo() = buildString {
append(RealmDebugTools(realmConfiguration).getInfo("Session"))
append(RealmDebugTools(realmConfigurationCrypto).getInfo("Crypto"))
append(RealmDebugTools(realmConfigurationIdentity).getInfo("Identity"))
append(RealmDebugTools(realmConfigurationContentScanner).getInfo("ContentScanner"))
}

override fun getRealmConfigurations(): List<RealmConfiguration> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ internal class EventSenderProcessorThread @Inject constructor(
private val queuedTaskFactory: QueuedTaskFactory,
private val taskExecutor: TaskExecutor,
private val memento: QueueMemento
) : Thread("SENDER_THREAD_SID_${sessionParams.credentials.sessionId()}"), EventSenderProcessor {
) : Thread("Matrix-SENDER_THREAD_SID_${sessionParams.credentials.sessionId()}"), EventSenderProcessor {

private fun markAsManaged(task: QueuedTask) {
memento.track(task)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ internal class DefaultTimeline(
) : Timeline {

companion object {
val BACKGROUND_HANDLER = createBackgroundHandler("DefaultTimeline_Thread")
val BACKGROUND_HANDLER = createBackgroundHandler("Matrix-DefaultTimeline_Thread")
}

override val timelineID = UUID.randomUUID().toString()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ internal class DefaultSyncService @Inject constructor(

override fun getSyncState() = getSyncThread().currentState()

override fun isSyncThreadAlive() = getSyncThread().isAlive

override fun getSyncRequestStateFlow() = syncRequestStateTracker.syncRequestState

override fun hasAlreadySynced(): Boolean {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ internal class SyncThread @Inject constructor(
private val backgroundDetectionObserver: BackgroundDetectionObserver,
private val activeCallHandler: ActiveCallHandler,
private val lightweightSettingsStorage: DefaultLightweightSettingsStorage
) : Thread("SyncThread"), NetworkConnectivityChecker.Listener, BackgroundDetectionObserver.Listener {
) : Thread("Matrix-SyncThread"), NetworkConnectivityChecker.Listener, BackgroundDetectionObserver.Listener {

private var state: SyncState = SyncState.Idle
private var liveState = MutableLiveData(state)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,13 @@ internal class DefaultBackgroundDetectionObserver : BackgroundDetectionObserver
}

override fun onStart(owner: LifecycleOwner) {
Timber.v("App returning to foreground…")
Timber.d("App returning to foreground…")
isInBackground = false
listeners.forEach { it.onMoveToForeground() }
}

override fun onStop(owner: LifecycleOwner) {
Timber.v("App going to background…")
Timber.d("App going to background…")
isInBackground = true
listeners.forEach { it.onMoveToBackground() }
}
Expand Down
2 changes: 1 addition & 1 deletion vector/src/main/java/im/vector/app/VectorApplication.kt
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ class VectorApplication :
}

private fun createFontThreadHandler(): Handler {
val handlerThread = HandlerThread("fonts")
val handlerThread = HandlerThread("Vector-fonts")
handlerThread.start()
return Handler(handlerThread.looper)
}
Expand Down
15 changes: 11 additions & 4 deletions vector/src/main/java/im/vector/app/core/di/ActiveSessionHolder.kt
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import android.content.Context
import arrow.core.Option
import im.vector.app.ActiveSessionDataSource
import im.vector.app.core.extensions.configureAndStart
import im.vector.app.core.extensions.startSyncing
import im.vector.app.core.pushers.UnifiedPushHelper
import im.vector.app.core.services.GuardServiceStarter
import im.vector.app.features.call.webrtc.WebRtcCallManager
Expand Down Expand Up @@ -100,10 +101,16 @@ class ActiveSessionHolder @Inject constructor(
}

suspend fun getOrInitializeSession(startSync: Boolean): Session? {
return activeSessionReference.get() ?: sessionInitializer.tryInitialize(readCurrentSession = { activeSessionReference.get() }) { session ->
setActiveSession(session)
session.configureAndStart(applicationContext, startSyncing = startSync)
}
return activeSessionReference.get()
Copy link
Contributor

@ouchadam ouchadam Aug 19, 2022

Choose a reason for hiding this comment

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

this might be clashing with https://github.com/vector-im/element-android/blob/develop/vector/src/main/java/im/vector/app/VectorApplication.kt#L176

I've always wondered if we should stop syncing when entering the foreground 🤔

EDIT - I guess the difference is foreground sync thread vs background sync workers

Copy link
Member Author

Choose a reason for hiding this comment

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

I've always wondered if we should stop syncing when entering the foreground

in foreground we should always have sync polling. Did you mean 'background'?

Also in VectorApplication, the Session may not be restored yet.

I am still investigating, but I agree this code is quite tricky...

?.also {
if (startSync && !it.syncService().isSyncThreadAlive()) {
Copy link
Contributor

@ouchadam ouchadam Aug 22, 2022

Choose a reason for hiding this comment

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

for my understanding of the error scenario

  • app is not started
  • receive a push
  • session instance is created with startSyncing = false and cached
  • next time the app is launched the sync is not started because the session instance already exists and we only configureAndStart brand new instances

the fix is to always start the sync (if startStart = true) on our cached instances if they're not already syncing?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that's it, sorry, the code is not crystal clear.

it.startSyncing(applicationContext)
}
}
?: sessionInitializer.tryInitialize(readCurrentSession = { activeSessionReference.get() }) { session ->
setActiveSession(session)
session.configureAndStart(applicationContext, startSyncing = startSync)
}
}

fun isWaitingForSessionInitialization() = activeSessionReference.get() == null && authenticationService.hasAuthenticatedSessions()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import org.matrix.android.sdk.api.session.sync.FilterService
import timber.log.Timber

fun Session.configureAndStart(context: Context, startSyncing: Boolean = true) {
Timber.i("Configure and start session for $myUserId")
Timber.i("Configure and start session for $myUserId. startSyncing: $startSyncing")
open()
filterService().setFilter(FilterService.FilterPreset.ElementFilter)
if (startSyncing) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ package im.vector.app.features.home.room.detail.timeline.helper
import android.os.Handler
import android.os.HandlerThread

private const val THREAD_NAME = "Timeline_Building_Thread"
private const val THREAD_NAME = "Vector-Timeline_Building_Thread"

object TimelineAsyncHelper {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ class BugReporter @Inject constructor(
private val systemLocaleProvider: SystemLocaleProvider,
private val matrix: Matrix,
private val buildMeta: BuildMeta,
private val processInfo: ProcessInfo,
private val sdkIntProvider: BuildVersionSdkIntProvider,
) {
var inMultiWindowMode = false
Expand Down Expand Up @@ -499,10 +500,26 @@ class BugReporter @Inject constructor(
*/
fun openBugReportScreen(activity: FragmentActivity, reportType: ReportType = ReportType.BUG_REPORT) {
screenshot = takeScreenshot(activity)
matrix.debugService().logDbUsageInfo()
logDbInfo()
logProcessInfo()
logOtherInfo()
activity.startActivity(BugReportActivity.intent(activity, reportType))
}

private fun logOtherInfo() {
Timber.i("SyncThread state: " + activeSessionHolder.getSafeActiveSession()?.syncService()?.getSyncState())
}

private fun logDbInfo() {
val dbInfo = matrix.debugService().getDbUsageInfo()
Timber.i(dbInfo)
}

private fun logProcessInfo() {
val pInfo = processInfo.getInfo()
Timber.i(pInfo)
}

private fun rageShakeAppNameForReport(reportType: ReportType): String {
// As per https://github.com/matrix-org/rageshake
// app: Identifier for the application (eg 'riot-web').
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/*
* Copyright (c) 2022 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 im.vector.app.features.rageshake

import android.annotation.SuppressLint
import android.app.Application
import android.os.Build
import android.os.Process
import java.lang.reflect.Method
import javax.inject.Inject

class ProcessInfo @Inject constructor() {
fun getInfo() = buildString {
append("===========================================\n")
append("* PROCESS INFO *\n")
append("===========================================\n")
val processId = Process.myPid()
append("ProcessId: $processId\n")
append("ProcessName: ${getProcessName()}\n")
append(getThreadInfo())
append("===========================================\n")
}

@SuppressLint("PrivateApi")
private fun getProcessName(): String? {
return if (Build.VERSION.SDK_INT >= 28) {
Application.getProcessName()
} else {
try {
val activityThread = Class.forName("android.app.ActivityThread")
val getProcessName: Method = activityThread.getDeclaredMethod("currentProcessName")
getProcessName.invoke(null) as? String
} catch (t: Throwable) {
null
}
}
}

private fun getThreadInfo() = buildString {
append("Thread activeCount: ${Thread.activeCount()}\n")
Thread.getAllStackTraces().keys
.sortedBy { it.name }
.forEach { thread -> append(thread.getInfo()) }
}
}

private fun Thread.getInfo() = buildString {
append("Thread '$name':")
append(" id: $id")
append(" priority: $priority")
append(" group name: ${threadGroup?.name ?: "null"}")
append(" state: $state")
append(" isAlive: $isAlive")
append(" isDaemon: $isDaemon")
append(" isInterrupted: $isInterrupted")
append("\n")
}