Page MenuHomePhabricator
Differential D12624

[native] Add CommSecureStore.set on main thread to resolve race with DB thread
ClosedPublic
Actions

Authored by ashoat on Sat, Jun 29, 12:27 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Jul 4, 10:05 AM
Unknown Object (File)
Tue, Jul 2, 6:02 PM
Unknown Object (File)
Tue, Jul 2, 6:02 PM
Unknown Object (File)
Tue, Jul 2, 6:01 PM
Unknown Object (File)
Tue, Jul 2, 6:00 PM
Subscribers
None

Details

Reviewers
marcin
tomek
Commits
rCOMM4386aef2000d: [native] Add CommSecureStore.get on main thread to resolve race with DB thread
Summary

Start by reading the included code comment, which explains why we need it.

This resolves ENG-8069, which was an issue caused by D11702, which was supposed to address ENG-7700.

This Linear comment has some backstory.

Test Plan
  1. Test on physical Android device. Confirm that before this diff it can reproduce ENG-8069
  2. Delete Comm app from phone. Deploy a release build with yarn react-native run-android --variant=release (may need to make sure native/facts/authoritative_keyserver.json is not set)
  3. Repeat the following steps 10 times:
    • Open the app and wait for LoggedOutModal to render
    • Kill the app
    • Open the app again and see if it crashes
    • Between attempts, clear the app's internal storage via settings

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ashoat created this revision.Sat, Jun 29, 12:27 PM
ashoat added a comment.Sat, Jun 29, 12:28 PM

Because this is a major issue affecting end users, I'm going to land this without review and immediately put out a new release.

ashoat edited the summary of this revision. (Show Details)Sat, Jun 29, 12:30 PM
Harbormaster completed remote builds in B30015: Diff 41831.Sat, Jun 29, 12:45 PM
ashoat requested review of this revision.Sat, Jun 29, 12:45 PM
This revision was not accepted when it landed; it landed in state Needs Review.Sat, Jun 29, 12:55 PM
Closed by commit rCOMM4386aef2000d: [native] Add CommSecureStore.get on main thread to resolve race with DB thread (authored by ashoat). · Explain Why
This revision was automatically updated to reflect the committed changes.
ashoat added a commit: rCOMM4386aef2000d: [native] Add CommSecureStore.get on main thread to resolve race with DB thread.
ashoat retitled this revision from [native] Add CommSecureStore.get on main thread to resolve race with DB thread to [native] Add CommSecureStore.set on main thread to resolve race with DB thread.Sat, Jun 29, 12:58 PM

Revision Contents
Changeset List

PathSize
native/
android/
app/
src/
main/
java/
app/
comm/
android/
7 lines

Diff 41832

View Options

native/android/app/src/main/java/app/comm/android/CommCoreJSIModulePackage.java

Loading...