cleanup a crashed or kill apps calls
The telecom dev rel mentioned flows were not echoing the endpoints
whenever an application was swiped up or killed. When I was trying to
reproduce this issue I noticed the client VoIP application was losing
it's CallControl reference even though the call still existed in the
platform.
Telecom should be cleaning up a clients calls if the app crashes via
a RuntimeException or is killed off by the ActivityManager.
The fix is to set a IBinder.DeathRecipient each time a
TransactionalServiceWrapper is created. This cleans up all the calls
every time the binder dies.
Fixes: 285045481
Test: manual;
- crash test app (w/ calls) via RuntimeException
- kill test app (w/ calls) via adb shell am force-stop packageName
Change-Id: Id6158d77cbaf3dd60557ca5e3aef7858d35868f3
diff --git a/src/com/android/server/telecom/TransactionalServiceRepository.java b/src/com/android/server/telecom/TransactionalServiceRepository.java
index f84b934..15278e1 100644
--- a/src/com/android/server/telecom/TransactionalServiceRepository.java
+++ b/src/com/android/server/telecom/TransactionalServiceRepository.java
@@ -16,6 +16,7 @@
package com.android.server.telecom;
+import android.telecom.Log;
import android.telecom.PhoneAccountHandle;
import com.android.internal.telecom.ICallEventCallback;
@@ -28,8 +29,8 @@
* more calls.
*/
public class TransactionalServiceRepository {
-
- private static final Map<PhoneAccountHandle, TransactionalServiceWrapper> lookupTable =
+ private static final String TAG = TransactionalServiceRepository.class.getSimpleName();
+ private static final Map<PhoneAccountHandle, TransactionalServiceWrapper> mServiceLookupTable =
new HashMap<>();
public TransactionalServiceRepository() {
@@ -38,12 +39,15 @@
public TransactionalServiceWrapper addNewCallForTransactionalServiceWrapper
(PhoneAccountHandle phoneAccountHandle, ICallEventCallback callEventCallback,
CallsManager callsManager, Call call) {
-
- TransactionalServiceWrapper service = null;
+ TransactionalServiceWrapper service;
+ // Only create a new TransactionalServiceWrapper if this is the first call for a package.
+ // Otherwise, get the existing TSW and add the new call to the service.
if (!hasExistingServiceWrapper(phoneAccountHandle)) {
+ Log.d(TAG, "creating a new TSW; handle=[%s]", phoneAccountHandle);
service = new TransactionalServiceWrapper(callEventCallback,
callsManager, phoneAccountHandle, call, this);
} else {
+ Log.d(TAG, "add a new call to an existing TSW; handle=[%s]", phoneAccountHandle);
service = getTransactionalServiceWrapper(phoneAccountHandle);
if (service == null) {
throw new IllegalStateException("service is null");
@@ -52,25 +56,25 @@
}
}
- lookupTable.put(phoneAccountHandle, service);
+ mServiceLookupTable.put(phoneAccountHandle, service);
return service;
}
public TransactionalServiceWrapper getTransactionalServiceWrapper(PhoneAccountHandle pah) {
- return lookupTable.get(pah);
+ return mServiceLookupTable.get(pah);
}
public boolean hasExistingServiceWrapper(PhoneAccountHandle pah) {
- return lookupTable.containsKey(pah);
+ return mServiceLookupTable.containsKey(pah);
}
public boolean removeServiceWrapper(PhoneAccountHandle pah) {
+ Log.i(TAG, "removeServiceWrapper: for phoneAccountHandle=[%s]", pah);
if (!hasExistingServiceWrapper(pah)) {
return false;
}
- lookupTable.remove(pah);
+ mServiceLookupTable.remove(pah);
return true;
}
-
}
diff --git a/src/com/android/server/telecom/TransactionalServiceWrapper.java b/src/com/android/server/telecom/TransactionalServiceWrapper.java
index 7093574..25aaad7 100644
--- a/src/com/android/server/telecom/TransactionalServiceWrapper.java
+++ b/src/com/android/server/telecom/TransactionalServiceWrapper.java
@@ -51,17 +51,17 @@
import com.android.server.telecom.voip.VoipCallTransactionResult;
import java.util.ArrayList;
-import java.util.Hashtable;
import java.util.List;
import java.util.Locale;
import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
/**
* Implements {@link android.telecom.CallEventCallback} and {@link android.telecom.CallControl}
* on a per-client basis which is tied to a {@link PhoneAccountHandle}
*/
public class TransactionalServiceWrapper implements
- ConnectionServiceFocusManager.ConnectionServiceFocus, IBinder.DeathRecipient {
+ ConnectionServiceFocusManager.ConnectionServiceFocus {
private static final String TAG = TransactionalServiceWrapper.class.getSimpleName();
// CallControl : Client (ex. voip app) --> Telecom
@@ -84,13 +84,25 @@
private final TransactionalServiceRepository mRepository;
private ConnectionServiceFocusManager.ConnectionServiceFocusListener mConnSvrFocusListener;
// init when constructor is called
- private final Hashtable<String, Call> mTrackedCalls = new Hashtable<>();
+ private final ConcurrentHashMap<String, Call> mTrackedCalls = new ConcurrentHashMap<>();
private final TelecomSystem.SyncRoot mLock;
private final String mPackageName;
// needs to be non-final for testing
private TransactionManager mTransactionManager;
private CallStreamingController mStreamingController;
+
+ // Each TransactionalServiceWrapper should have their own Binder.DeathRecipient to clean up
+ // any calls in the event the application crashes or is force stopped.
+ private final IBinder.DeathRecipient mAppDeathListener = new IBinder.DeathRecipient() {
+ @Override
+ public void binderDied() {
+ Log.i(TAG, "binderDied: for package=[%s]; cleaning calls", mPackageName);
+ cleanupTransactionalServiceWrapper();
+ mICallEventCallback.asBinder().unlinkToDeath(this, 0);
+ }
+ };
+
public TransactionalServiceWrapper(ICallEventCallback callEventCallback,
CallsManager callsManager, PhoneAccountHandle phoneAccountHandle, Call call,
TransactionalServiceRepository repo) {
@@ -105,6 +117,7 @@
mTransactionManager = TransactionManager.getInstance();
mStreamingController = mCallsManager.getCallStreamingController();
mLock = mCallsManager.getLock();
+ setDeathRecipient(callEventCallback);
}
@VisibleForTesting
@@ -128,12 +141,6 @@
}
}
- public Call getCallById(String callId) {
- synchronized (mLock) {
- return mTrackedCalls.get(callId);
- }
- }
-
@VisibleForTesting
public boolean untrackCall(Call call) {
Call removedCall = null;
@@ -158,23 +165,12 @@
return callCount;
}
- @Override
- public void binderDied() {
- // remove all tacked calls from CallsManager && frameworks side
- for (String id : mTrackedCalls.keySet()) {
- Call call = mTrackedCalls.get(id);
- mCallsManager.markCallAsDisconnected(call, new DisconnectCause(DisconnectCause.ERROR));
- mCallsManager.removeCall(call);
- // remove calls from Frameworks side
- if (mICallEventCallback != null) {
- try {
- mICallEventCallback.removeCallFromTransactionalServiceWrapper(call.getId());
- } catch (RemoteException e) {
- // pass
- }
- }
+ public void cleanupTransactionalServiceWrapper() {
+ for (Call call : mTrackedCalls.values()) {
+ mCallsManager.markCallAsDisconnected(call,
+ new DisconnectCause(DisconnectCause.ERROR, "process died"));
+ mCallsManager.removeCall(call); // This will clear mTrackedCalls && ClientTWS
}
- mTrackedCalls.clear();
}
/***
@@ -556,10 +552,10 @@
try {
// remove the call from frameworks wrapper (client side)
mICallEventCallback.removeCallFromTransactionalServiceWrapper(call.getId());
- // remove the call from this class/wrapper (server side)
- untrackCall(call);
} catch (RemoteException e) {
}
+ // remove the call from this class/wrapper (server side)
+ untrackCall(call);
}
}
@@ -605,6 +601,15 @@
return new SerialTransaction(transactions, mLock);
}
+ private void setDeathRecipient(ICallEventCallback callEventCallback) {
+ try {
+ callEventCallback.asBinder().linkToDeath(mAppDeathListener, 0);
+ } catch (Exception e) {
+ Log.w(TAG, "setDeathRecipient: hit exception=[%s] trying to link binder to death",
+ e.toString());
+ }
+ }
+
/***
*********************************************************************************************
** FocusManager **
diff --git a/testapps/transactionalVoipApp/res/layout/in_call_activity.xml b/testapps/transactionalVoipApp/res/layout/in_call_activity.xml
index 54d467e..bed2e1a 100644
--- a/testapps/transactionalVoipApp/res/layout/in_call_activity.xml
+++ b/testapps/transactionalVoipApp/res/layout/in_call_activity.xml
@@ -58,6 +58,12 @@
android:layout_height="wrap_content"
android:text="@string/start_stream"/>
+ <Button
+ android:id="@+id/crash_app"
+ android:layout_width="wrap_content"
+ android:layout_height="wrap_content"
+ android:text="@string/crash_app"/>
+
<LinearLayout
android:layout_width="wrap_content"
android:layout_height="wrap_content"
diff --git a/testapps/transactionalVoipApp/res/values/strings.xml b/testapps/transactionalVoipApp/res/values/strings.xml
index 23a5118..c8486c1 100644
--- a/testapps/transactionalVoipApp/res/values/strings.xml
+++ b/testapps/transactionalVoipApp/res/values/strings.xml
@@ -38,5 +38,6 @@
<string name="request_bluetooth_endpoint">Bluetooth</string>
<!-- extra functionality -->
<string name="start_stream">start streaming</string>
+ <string name="crash_app">throw exception</string>
</resources>
\ No newline at end of file
diff --git a/testapps/transactionalVoipApp/src/com/android/server/telecom/transactionalVoipApp/InCallActivity.java b/testapps/transactionalVoipApp/src/com/android/server/telecom/transactionalVoipApp/InCallActivity.java
index 3e53800..53f5e9c 100644
--- a/testapps/transactionalVoipApp/src/com/android/server/telecom/transactionalVoipApp/InCallActivity.java
+++ b/testapps/transactionalVoipApp/src/com/android/server/telecom/transactionalVoipApp/InCallActivity.java
@@ -164,10 +164,28 @@
}
}
});
+
+ findViewById(R.id.crash_app).setOnClickListener(new View.OnClickListener() {
+ @Override
+ public void onClick(View v) {
+ // To test edge cases, it may be useful to crash the app. To do this, throwing a
+ // RuntimeException is sufficient.
+ throw new RuntimeException(
+ "Intentionally throwing RuntimeException from InCallActivity");
+ }
+ });
+ }
+
+
+ @Override
+ protected void onStop() {
+ Log.i(TAG, "onStop: InCallActivity has stopped");
+ super.onStop();
}
@Override
protected void onDestroy() {
+ Log.i(TAG, "onDestroy: InCallActivity has been destroyed");
disconnectAndStopAudio();
super.onDestroy();
}
@@ -205,7 +223,12 @@
sb.append("Error Getting Id");
}
sb.append("]");
- view.setText(sb.toString());
+ try {
+ view.setText(sb.toString());
+ }
+ catch (Exception e){
+ // ignore updating the ui
+ }
}
private void addCall() {
diff --git a/tests/src/com/android/server/telecom/tests/TransactionalServiceWrapperTest.java b/tests/src/com/android/server/telecom/tests/TransactionalServiceWrapperTest.java
index 98624d4..fa5f2a2 100644
--- a/tests/src/com/android/server/telecom/tests/TransactionalServiceWrapperTest.java
+++ b/tests/src/com/android/server/telecom/tests/TransactionalServiceWrapperTest.java
@@ -25,6 +25,7 @@
import android.content.ComponentName;
+import android.os.IBinder;
import android.os.OutcomeReceiver;
import android.os.RemoteException;
import android.os.ResultReceiver;
@@ -69,6 +70,7 @@
@Mock private TransactionManager mTransactionManager;
@Mock private ICallEventCallback mCallEventCallback;
@Mock private TransactionalServiceRepository mRepository;
+ @Mock private IBinder mIBinder;
private final TelecomSystem.SyncRoot mLock = new TelecomSystem.SyncRoot() {};
@Override
@@ -79,7 +81,7 @@
Mockito.when(mMockCall1.getId()).thenReturn(CALL_ID_1);
Mockito.when(mMockCall2.getId()).thenReturn(CALL_ID_2);
Mockito.when(mCallsManager.getLock()).thenReturn(mLock);
-
+ Mockito.when(mCallEventCallback.asBinder()).thenReturn(mIBinder);
mTransactionalServiceWrapper = new TransactionalServiceWrapper(mCallEventCallback,
mCallsManager, SERVICE_HANDLE, mMockCall1, mRepository);