Try to fix #4007 #4193

bmarty · 2021-10-08T18:27:58Z

Still crashing from time to time :/

Log is always

D/CleanupSession: Realm instance (4 - 1)
D/CleanupSession: Cleanup: delete session params...
D/CleanupSession: Cleanup: cancel pending works...
D/CleanupSession: Cleanup: clear session data...
D/CleanupSession: Cleanup: clear crypto data...
D/CleanupSession: Cleanup: clear the database keys
D/CleanupSession: Cleanup: release session...
D/CleanupSession: Wait for all Realm instance to be closed (3 - 0)
D/CleanupSession: Waiting 10ms
D/CleanupSession: Wait for all Realm instance to be closed (0 - 0)
D/CleanupSession: Cleanup: clear file system

Whatever the value of TIME_TO_WAIT_MILLIS (tested with 1, 10, 100, 200).

Also other crashes can occur, not all the time...

github-actions · 2021-10-08T18:50:47Z

Unit Test Results

  42 files ±0   42 suites ±0 44s ⏱️ -2s
  83 tests ±0   83 ✔️ ±0 0 💤 ±0 0 ❌ ±0
220 runs ±0 220 ✔️ ±0 0 💤 ±0 0 ❌ ±0

Results for commit a7ec76b. ± Comparison against base commit 7088e5c.

♻️ This comment has been updated with latest results.

ariskotsomitopoulos · 2021-10-09T08:32:28Z

Still crashing from time to time :/

Log is always

D/CleanupSession: Realm instance (4 - 1)
D/CleanupSession: Cleanup: delete session params...
D/CleanupSession: Cleanup: cancel pending works...
D/CleanupSession: Cleanup: clear session data...
D/CleanupSession: Cleanup: clear crypto data...
D/CleanupSession: Cleanup: clear the database keys
D/CleanupSession: Cleanup: release session...
D/CleanupSession: Wait for all Realm instance to be closed (3 - 0)
D/CleanupSession: Waiting 10ms
D/CleanupSession: Wait for all Realm instance to be closed (0 - 0)
D/CleanupSession: Cleanup: clear file system

Whatever the value of TIME_TO_WAIT_MILLIS (tested with 1, 10, 100, 200).

Also other crashes can occur, not all the time...

Relative issue

ouchadam · 2021-10-11T16:26:26Z

TIL from the docs https://docs.mongodb.com/realm-legacy/docs/java/latest/#closing-realms

Realm instances are reference counted—if you call getInstance twice in a thread, you need to call close twice as well. This allows you to implement Runnable classes without having to worry about which thread will execute them: simply start it with getInstance and end it with close.

from the original issue #4007 the stacktrace looks like realm is still closing/has deleted a required file after it initialised the new instance of the database, very odd! 🤔

'/data/user/0/im.vector.app.debug/files/.../crypto_store.realm.lock' does not exist.

bmarty · 2021-10-12T07:51:02Z

Actually I think all the Realm instance are closed, then when delete the files, but after that DefaultDownloadKeysForUsers wants to write to the database. Something is not properly cancelled.

ouchadam · 2021-10-12T07:55:30Z

could it be that the wrong realm instance is somehow being used after logout? maybe a singleton somewhere is still referencing the closed instance

Wait for Realm instance to be effectively closed before deleting Realm files

bmarty · 2021-10-12T10:13:59Z

Looking again at the stacktrace from #4007 , I can see:

at com.zhuinden.monarchy.Monarchy.runTransactionSync(Monarchy.java:164)

Which is called from Monarchy.writeAsync, and from our codebase here

So this commit try to cancel all the pending async request(s).

No crash observed, but I would like to see a log like like this:

W/RealmCryptoStore: Closing RealmCryptoStore, 1 async task(s) cancelled

But I have only seen

W/RealmCryptoStore: Closing RealmCryptoStore, 0 async task(s) cancelled

WDYT?

ouchadam · 2021-10-12T10:24:18Z

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/IMXCryptoStore.kt

@@ -379,7 +379,8 @@ internal interface IMXCryptoStore {

    fun getOrAddOutgoingSecretShareRequest(secretName: String, recipients: Map<String, List<String>>): OutgoingSecretRequest?

-    fun saveGossipingEvent(event: Event)
+    fun saveGossipingEvent(event: Event) = saveGossipingEvents(listOf(event))


ouchadam · 2021-10-12T10:25:05Z

...-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/cleanup/CleanupSession.kt

+                delay(TIME_TO_WAIT_MILLIS)
+                timeToWaitMillis -= TIME_TO_WAIT_MILLIS
+            } else {
+                timeToWaitMillis = 0


should we also log when we fail to close all instances in time, would it be helpful for rageshakes?

I think we will see the bunch of similar logs and it will be enough

bmarty mentioned this pull request Oct 8, 2021

Feature/aris/issue 4007 logout crash #4191

Closed

Try to fix #4007

0d85299

Wait for Realm instance to be effectively closed before deleting Realm files

bmarty force-pushed the feature/bma/fix_logout_crash branch from b4a345c to 0d85299 Compare October 12, 2021 08:41

bmarty added 3 commits October 12, 2021 11:38

Avoid code duplication

73c08e2

CleanupSession: start by releasing the session, then empty the databases

4a7e0a5

Ensure no async transaction will occurs if the store is closed

2d97640

ouchadam reviewed Oct 12, 2021

View reviewed changes

ouchadam approved these changes Oct 12, 2021

View reviewed changes

bmarty added 2 commits October 12, 2021 14:06

Add changelog

d11f4e5

Also call monarchyWriteAsyncExecutor.awaitTermination

a7ec76b

BillCarsonFr approved these changes Oct 12, 2021

View reviewed changes

bmarty marked this pull request as ready for review October 12, 2021 12:55

bmarty enabled auto-merge October 12, 2021 12:55

bmarty merged commit 7338982 into develop Oct 12, 2021

bmarty deleted the feature/bma/fix_logout_crash branch October 12, 2021 13:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Try to fix #4007 #4193

Try to fix #4007 #4193

bmarty commented Oct 8, 2021

github-actions bot commented Oct 8, 2021 •

edited

Loading

ariskotsomitopoulos commented Oct 9, 2021

ouchadam commented Oct 11, 2021

bmarty commented Oct 12, 2021

ouchadam commented Oct 12, 2021 •

edited

Loading

bmarty commented Oct 12, 2021

ouchadam Oct 12, 2021

ouchadam Oct 12, 2021

bmarty Oct 12, 2021

Try to fix #4007 #4193

Try to fix #4007 #4193

Conversation

bmarty commented Oct 8, 2021

github-actions bot commented Oct 8, 2021 • edited Loading

Unit Test Results

ariskotsomitopoulos commented Oct 9, 2021

ouchadam commented Oct 11, 2021

bmarty commented Oct 12, 2021

ouchadam commented Oct 12, 2021 • edited Loading

bmarty commented Oct 12, 2021

ouchadam Oct 12, 2021

Choose a reason for hiding this comment

ouchadam Oct 12, 2021

Choose a reason for hiding this comment

bmarty Oct 12, 2021

Choose a reason for hiding this comment

github-actions bot commented Oct 8, 2021 •

edited

Loading

ouchadam commented Oct 12, 2021 •

edited

Loading