fix: resolve thread safety issue in CallRecordDatabase #475
Conversation
| @@ -157,14 +157,18 @@ public void clear() { | |||
|
|
|||
| public CallRecord get(final CallRecord record) { | |||
| try { | |||
| return callRecordList.get(callRecordList.indexOf(record)); | |||
| synchronized (callRecordList) { | |||
There was a problem hiding this comment.
Under what condition are there multiple threads accessing the call record database?
There was a problem hiding this comment.
The issue we ran into was rejecting or accepting a call at the same time that the calling end hangs up, but there are probably others.
This becomes extra problematic because it results in a null return type, which from most of the call points of get or remove will result in an exception that crashes the app since they are wrapped in Objects.requireNonNull. This does not entirely address that problem, but is a start.
There was a problem hiding this comment.
@jhutchins Are you using new-arch? I ask because historically all invocations were coming from the same thread. IE. the main thread, and therefore impossible for a remove to happen at the exact same time as a addition. Thank you for providing the use-case and we will attempt to recreate the issue you are experiencing...
There was a problem hiding this comment.
No, we are explicitly disabling new-arch in our gradle.properties file.
In our case, we are answering the call from the JavaScript and in that case you can see that the various methods within CallRecordDatabase are accessed from different threads.
The same will be true when you hang up using a button on the active call screen, there will be a get from the mqt_native_modules thread which could cause an issue if both ends of a call try to hang up at the same time.
Fix #474
Submission Checklist
CHANGELOG.mdto reflect any feature, bug fixes, or known issues made in the source codeFiles changedtab prior to submitting the pull request for review to ensure proper usage of the style guideDescription
Add proper synchronization to the
removeandgetmethods of theCallRecordDatabasein the Android native code.Breakdown
getmethodremovemethodValidation