This differential replaces any direct database access in iOS native code with GlobalDBSingleton API
Details
Details
Creating test release with those changes would remove iOS 137 crash
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
We should be really sure about this code. If there are any issues, it will probably cause crashes we don't know about. I encourage everybody on this diff to be extremely thorough. We should be 100% sure of every line here. We should not be guessing, or trying things that seem to work without understanding 100% of why/how they work.
Comment Actions
This is ok assuming we're using the proxy. I'm not yet convinced D5233 that this is the right thing to do, but this diff seems to be ok.
| native/ios/Comm/AppDelegate.mm | ||
|---|---|---|
| 171 ↗ | (On Diff #17343) | How sure are we that this doesn't need to be within GlobalDBSingleton? Looking into it, it seems like a couple assignments to instance variables, which should be safe... but we need to make sure there are no assignments from other threads (only one thread should ever call attemptDatabaseInitialization and SQLiteQueryExecutor::initialize), and we should be sure that there are no risks from reading from the variable while the assignment is in progress (if there are, we could consider an atomic variable or something) |
| native/ios/Comm/AppDelegate.mm | ||
|---|---|---|
| 171 ↗ | (On Diff #17343) | attemptDatabaseInitialization and SQLiteQueryExecutor::initialize can be called from any thread since those method do not touch database and use std::call_once semantics which makes them idempotent (after it is called for the first time subsequent calls are no-ops) and thread safe. |
| native/ios/Comm/AppDelegate.mm | ||
|---|---|---|
| 171 ↗ | (On Diff #17343) | Makes sense, thanks! |