Ensure sync thread is started #6884
Conversation
| setActiveSession(session) | ||
| session.configureAndStart(applicationContext, startSyncing = startSync) | ||
| } | ||
| return activeSessionReference.get() |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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...
I don't know about you guys, but I have occasionally encountered the Paused state on my device since forever. I don't remember if it started in a particular version. The solution has always been to remove the app from Recent Apps and then open it again, which causes the sync to resume. |
Thanks for the feedback. On my side I did not repro in the past days. Moving this PR to ready to review. |
bmarty
added
the
Z-NextRelease
| } | ||
| return activeSessionReference.get() | ||
| ?.also { | ||
| if (startSync && !it.syncService().isSyncThreadAlive()) { |
There was a problem hiding this comment.
for my understanding of the error scenario
- app is not started
- receive a push
- session instance is created with
startSyncing = falseand cached - next time the app is launched the sync is not started because the session instance already exists and we only
configureAndStartbrand new instances
the fix is to always start the sync (if startStart = true) on our cached instances if they're not already syncing?
There was a problem hiding this comment.
Yes, that's it, sorry, the code is not crystal clear.
|
SonarCloud Quality Gate failed. |
|
|
It's still a bit early to be sure, but I don't think I've encountered this after switching to ntfy for push notifications, from F-Droid's 60 second polling. It might have been specific to that particular config. |
|
Yep. Too early. Got a Paused even with ntfy. I have no STR, sadly, otherwise I would have created a new issue. |
Type of change
Content
Motivation and context
Regression from #6710, release in Element and SDK v1.4.31 (2022-08-01)
Screenshots / GIFs
NA
Tests
Tested devices
Checklist