From 07d3736f5daf61b8a10b8a22f4bfb7c8ef87b635 Mon Sep 17 00:00:00 2001
From: Tyler Gunn <tgunn@google.com>
Date: Fri, 21 May 2021 16:20:16 -0700
Subject: [PATCH] Fix potential exceptions in CallDiagnosticService and missing
 locks.

There is a possibility of a call receiving a details update when it has
already been removed from the CallDiagnosticService.  Handling this case
by just ignoring the details update (since it isn't relevant as the call
is now removed).
Also noted there were a number of places where access to the shared
maps was not synchronized as it should be.

Test: Run manual tests and note no more CDS crashes from SCONE app.
Fixes: 188939131
Change-Id: Ic1f96fd60a70432bef0daaa2e9729c66401ba4ac
---
 .../telecom/CallDiagnosticService.java        | 21 +++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/telecomm/java/android/telecom/CallDiagnosticService.java b/telecomm/java/android/telecom/CallDiagnosticService.java
index 011dc17a1c1e4..336a8ea3f3108 100644
--- a/telecomm/java/android/telecom/CallDiagnosticService.java
+++ b/telecomm/java/android/telecom/CallDiagnosticService.java
@@ -294,6 +294,10 @@ public abstract class CallDiagnosticService extends Service {
         CallDiagnostics callDiagnostics;
         synchronized (mLock) {
             callDiagnostics = mDiagnosticCallByTelecomCallId.get(telecomCallId);
+            if (callDiagnostics == null) {
+                // Possible to get a call update after a call is removed.
+                return;
+            }
             mCallByTelecomCallId.put(telecomCallId, newCallDetails);
         }
         getExecutor().execute(() -> callDiagnostics.handleCallUpdated(newCallDetails));
@@ -306,12 +310,12 @@ public abstract class CallDiagnosticService extends Service {
     private void handleCallRemoved(@NonNull String telecomCallId) {
         Log.i(this, "handleCallRemoved: callId=%s - removed", telecomCallId);
 
-        if (mCallByTelecomCallId.containsKey(telecomCallId)) {
-            mCallByTelecomCallId.remove(telecomCallId);
-        }
-
         CallDiagnostics callDiagnostics;
         synchronized (mLock) {
+            if (mCallByTelecomCallId.containsKey(telecomCallId)) {
+                mCallByTelecomCallId.remove(telecomCallId);
+            }
+
             if (mDiagnosticCallByTelecomCallId.containsKey(telecomCallId)) {
                 callDiagnostics = mDiagnosticCallByTelecomCallId.remove(telecomCallId);
             } else {
@@ -353,7 +357,10 @@ public abstract class CallDiagnosticService extends Service {
     private void handleCallDisconnected(@NonNull String callId,
             @NonNull DisconnectCause disconnectCause) {
         Log.i(this, "handleCallDisconnected: call=%s; cause=%s", callId, disconnectCause);
-        CallDiagnostics callDiagnostics = mDiagnosticCallByTelecomCallId.get(callId);
+        CallDiagnostics callDiagnostics;
+        synchronized (mLock) {
+            callDiagnostics = mDiagnosticCallByTelecomCallId.get(callId);
+        }
         CharSequence message;
         if (disconnectCause.getImsReasonInfo() != null) {
             message = callDiagnostics.onCallDisconnected(disconnectCause.getImsReasonInfo());
@@ -391,7 +398,9 @@ public abstract class CallDiagnosticService extends Service {
             @NonNull CallQuality callQuality) {
         Log.i(this, "handleCallQualityChanged; call=%s, cq=%s", callId, callQuality);
         CallDiagnostics callDiagnostics;
-        callDiagnostics = mDiagnosticCallByTelecomCallId.get(callId);
+        synchronized(mLock) {
+            callDiagnostics = mDiagnosticCallByTelecomCallId.get(callId);
+        }
         if (callDiagnostics != null) {
             callDiagnostics.onCallQualityReceived(callQuality);
         }