Page MenuHomePhabricator
Differential D6958

[native] Move CommSecureStore::get to main thread to avoid JNI crash
ClosedPublic
Actions

Authored by ashoat on Mar 6 2023, 11:55 AM.
Tags
None
Referenced Files
F3492639: D6958.id23472.diff
Thu, Dec 19, 12:57 AM
Unknown Object (File)
Sat, Dec 14, 8:11 AM
Unknown Object (File)
Sat, Dec 14, 8:11 AM
Unknown Object (File)
Sat, Dec 14, 8:11 AM
Unknown Object (File)
Sat, Dec 14, 8:10 AM
Unknown Object (File)
Sat, Dec 14, 8:05 AM
Unknown Object (File)
Wed, Dec 4, 7:51 AM
Unknown Object (File)
Wed, Dec 4, 7:51 AM
View All Files
Subscribers
tomek

Details

Reviewers
atul
marcin
Commits
rCOMM980305ee4c95: [native] Move CommSecureStore::get to main thread to avoid JNI crash
Summary

This is option 2 from this Linear comment.

Depends on D6957

Test Plan

Test that the error goes away in favor of another see (see Linear)

Also need to test this on iOS before landing to make sure it doesn't break that, but should be okay

Diff Detail

Repository
rCOMM Comm
Branch
master
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

ashoat created this revision.Mar 6 2023, 11:55 AM
Herald added a subscriber: tomek. · View Herald TranscriptMar 6 2023, 11:55 AM
ashoat updated this revision to Diff 23469.Mar 6 2023, 11:56 AM

Minor

Harbormaster completed remote builds in B17130: Diff 23468.Mar 6 2023, 12:10 PM
Harbormaster completed remote builds in B17131: Diff 23469.
ashoat requested review of this revision.Mar 6 2023, 12:12 PM
atul accepted this revision.Mar 6 2023, 12:37 PM
atul added inline comments.
native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp
834–835 ↗(On Diff #23469)

Verified locally that this was a valid way to split string across multiple lines.. don't think I've seen his before

This revision is now accepted and ready to land.Mar 6 2023, 12:37 PM
Closed by commit rCOMM980305ee4c95: [native] Move CommSecureStore::get to main thread to avoid JNI crash (authored by ashoat). · Explain WhyMar 6 2023, 1:25 PM
This revision was automatically updated to reflect the committed changes.
ashoat added a commit: rCOMM980305ee4c95: [native] Move CommSecureStore::get to main thread to avoid JNI crash.
ashoat mentioned this in D6959: Run everuthing regarding NotificationsCryptoModule with native Android Accessible.Mar 6 2023, 4:56 PM
marcin added a reverting change: D6980: Revert "[native] Move CommSecureStore::get to main thread to avoid JNI crash".Mar 7 2023, 5:37 AM
marcin added a reverting change: rCOMM2453e10d5b13: Revert "[native] Move CommSecureStore::get to main thread to avoid JNI crash".Mar 8 2023, 3:00 AM

Revision Contents
Changeset List

PathSize
native/
cpp/
CommonCpp/
NativeModules/
100 lines
Notifications/
BackgroundDataStorage/
6 lines
16 lines

Diff 23468

View Options

native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp

Loading...
View Options

native/cpp/CommonCpp/Notifications/BackgroundDataStorage/NotificationsCryptoModule.h

Loading...
View Options

native/cpp/CommonCpp/Notifications/BackgroundDataStorage/NotificationsCryptoModule.cpp

Loading...