Fix concurrency issue related to new outgoing call broadcast.
Fixes arace condition seen with the new outgoing call broadcast.
The scenario occurs when an incoming call is handled by an app which
receives the NewOutgoingCallBroadcast. That app cancels the call by
modifying the new outgoing callbroadcast. Meanwhile, it places that
same call again, expecting that Telecom will reuse the same same.
HOWEVER, if the system delays passing of the new outgoing call broadcast
back to Telecom, the app will have placed a new outgoing call BEFORE
telecom is aware that the call was cancelled.
The consequence of this is that in CallsManager#startOutgoingCall, when
we first get the call to reuse, it will come back empty. Meanwhile, by
the time we get into the various completable futures, the call WILL be
in the list of calls which can be reused. Since the reusable call was
not found earlier on, we end up aborting the new outgoing call.
Test: wrote unit test to reproduce this scenario and modified code to fix
the test.
Fixes: 171324351
Change-Id: Ieda35ee4f729ad7bbfa87b43b7ef5d11de9f3dab
diff --git a/src/com/android/server/telecom/CallsManager.java b/src/com/android/server/telecom/CallsManager.java
index ff3122f..4972602 100755
--- a/src/com/android/server/telecom/CallsManager.java
+++ b/src/com/android/server/telecom/CallsManager.java
@@ -1573,6 +1573,17 @@
// call transitioning into the CONNECTING state.
if (isReusedCall) {
return CompletableFuture.completedFuture(finalCall);
+ } else {
+ Call reusableCall = reuseOutgoingCall(handle);
+ if (reusableCall != null) {
+ Log.i(CallsManager.this,
+ "reusable call %s came in later; disconnect it.",
+ reusableCall.getId());
+ mPendingCallsToDisconnect.remove(reusableCall);
+ reusableCall.disconnect();
+ markCallAsDisconnected(reusableCall,
+ new DisconnectCause(DisconnectCause.CANCELED));
+ }
}
// If we can not supportany more active calls, our options are to move a call
@@ -5327,4 +5338,9 @@
intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
mContext.startActivityAsUser(intent, mCurrentUserHandle);
}
+
+ @VisibleForTesting
+ public void addToPendingCallsToDisconnect(Call call) {
+ mPendingCallsToDisconnect.add(call);
+ }
}
diff --git a/tests/src/com/android/server/telecom/tests/CallsManagerTest.java b/tests/src/com/android/server/telecom/tests/CallsManagerTest.java
index 0cc04d6..3fd5e60 100644
--- a/tests/src/com/android/server/telecom/tests/CallsManagerTest.java
+++ b/tests/src/com/android/server/telecom/tests/CallsManagerTest.java
@@ -42,8 +42,11 @@
import android.content.ComponentName;
import android.content.Context;
+import android.content.Intent;
import android.net.Uri;
import android.os.Bundle;
+import android.os.Handler;
+import android.os.Looper;
import android.os.Process;
import android.os.SystemClock;
import android.os.UserHandle;
@@ -56,6 +59,7 @@
import android.telephony.TelephonyManager;
import android.test.suitebuilder.annotation.MediumTest;
import android.test.suitebuilder.annotation.SmallTest;
+import android.util.Pair;
import android.widget.Toast;
import com.android.server.telecom.AsyncRingtonePlayer;
@@ -116,6 +120,8 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CountDownLatch;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.TimeUnit;
@@ -1381,6 +1387,80 @@
}
/**
+ * This test verifies a race condition seen with the new outgoing call broadcast.
+ * The scenario occurs when an incoming call is handled by an app which receives the
+ * NewOutgoingCallBroadcast. That app cancels the call by modifying the new outgoing call
+ * broadcast. Meanwhile, it places that same call again, expecting that Telecom will reuse the
+ * same same. HOWEVER, if the system delays passing of the new outgoing call broadcast back to
+ * Telecom, the app will have placed a new outgoing call BEFORE telecom is aware that the call
+ * was cancelled.
+ * The consequence of this is that in CallsManager#startOutgoingCall, when we first get the
+ * call to reuse, it will come back empty. Meanwhile, by the time we get into the various
+ * completable futures, the call WILL be in the list of calls which can be reused. Since the
+ * reusable call was not found earlier on, we end up aborting the new outgoing call.
+ * @throws Exception
+ */
+ @SmallTest
+ @Test
+ public void testReuseCallConcurrency() throws Exception {
+ // Ensure contact info lookup succeeds
+ doAnswer(invocation -> {
+ Uri handle = invocation.getArgument(0);
+ CallerInfo info = new CallerInfo();
+ CompletableFuture<Pair<Uri, CallerInfo>> callerInfoFuture = new CompletableFuture<>();
+ callerInfoFuture.complete(new Pair<>(handle, info));
+ return callerInfoFuture;
+ }).when(mCallerInfoLookupHelper).startLookup(any(Uri.class));
+
+ // Ensure we have candidate phone account handle info.
+ when(mPhoneAccountRegistrar.getOutgoingPhoneAccountForScheme(any(), any())).thenReturn(
+ SIM_1_HANDLE);
+ when(mPhoneAccountRegistrar.getCallCapablePhoneAccounts(any(), anyBoolean(),
+ any(), anyInt(), anyInt())).thenReturn(
+ new ArrayList<>(Arrays.asList(SIM_1_HANDLE, SIM_2_HANDLE)));
+
+ // Let's add an existing call which is in connecting state; this emulates the case where
+ // we have an outgoing call which we have not yet disconnected as a result of the new
+ // outgoing call broadcast cancelling the call.
+ Call outgoingCall = addSpyCall(CallState.CONNECTING);
+
+ final CountDownLatch latch = new CountDownLatch(1);
+ // Get the handler for the main looper, which is the same one the CallsManager will use.
+ // We'll post a little something to block up the handler for now. This prevents
+ // startOutgoingCall from process it's completablefutures.
+ Handler handler = new Handler(Looper.getMainLooper());
+ handler.post(() -> {
+ try {
+ latch.await();
+ } catch (InterruptedException e) {
+ e.printStackTrace();
+ }
+ });
+
+ // Now while the main handler is blocked up we'll start another outgoing call.
+ CompletableFuture<Call> callFuture = mCallsManager.startOutgoingCall(
+ outgoingCall.getHandle(), outgoingCall.getTargetPhoneAccount(), new Bundle(),
+ UserHandle.CURRENT, new Intent(), "com.test.stuff");
+
+ // And we'll add the initial outgoing call to the list of pending disconnects; this
+ // emulates a scenario where the pending disconnect call came in AFTER this call began.
+ mCallsManager.addToPendingCallsToDisconnect(outgoingCall);
+
+ // And we'll unblock the handler; this will let all the startOutgoingCall futures to happen.
+ latch.countDown();
+
+ // Wait for the future to become the present.
+ callFuture.join();
+
+ // We should have gotten a call out of this; if we did not then it means the call was
+ // aborted.
+ assertNotNull(callFuture.get());
+
+ // And the original call should be disconnected now.
+ assertEquals(CallState.DISCONNECTED, outgoingCall.getState());
+ }
+
+ /**
* Ensures that if we have two calls hosted by the same connection manager, but with
* different target phone accounts, we can swap between them.
* @throws Exception
@@ -1460,7 +1540,6 @@
// Mocks some methods to not call the real method.
doNothing().when(callSpy).unhold();
doNothing().when(callSpy).hold();
- doNothing().when(callSpy).disconnect();
doNothing().when(callSpy).answer(Matchers.anyInt());
doNothing().when(callSpy).setStartWithSpeakerphoneOn(Matchers.anyBoolean());