Adding the resource-deallocation utility.
Change-Id: I73ee3b648d7b224017bb2445597f199f979be5de
diff --git a/src/com/android/telecomm/BinderDeallocator.java b/src/com/android/telecomm/BinderDeallocator.java
new file mode 100644
index 0000000..ab7db25
--- /dev/null
+++ b/src/com/android/telecomm/BinderDeallocator.java
@@ -0,0 +1,121 @@
+/*
+ * Copyright (C) 2014 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.telecomm;
+
+import android.util.Log;
+
+import com.google.common.collect.Sets;
+
+import java.util.Iterator;
+import java.util.Set;
+
+/**
+ * Keeps track of all outstanding binder connections and provides a facility to clean up unnecessary
+ * binders (after initializing outgoing or incoming calls). This in turn allows the classes involved
+ * in incoming and outgoing initialization to not have to unbind at all possible failure conditions,
+ * which are numerous.
+ *
+ * To provide (theoretical) context, the two candidate approaches seem to be (1) provide a notion of
+ * guaranteed outcome to every attempted action only deallocating resources upon no pending actions,
+ * and (2) while other outcomes may be provided, only guarantee the potential outcome of success and
+ * then rely on timeouts to treat all expired actions as failures. Since Telecomm pretty much has to
+ * deal with timeouts (e.g. due to relying on third-party code), it seems to make little or no sense
+ * to guarantee an outcome to every attempted action particularly via using timeouts. Instead, since
+ * relying on timeouts is required in both cases, using a centralized timeout solution appears to be
+ * beneficial. This gives rise to the following implementation. Any time the switchboard enters what
+ * can be thought of as a critical section, it is likely to cause certain resources to be created or
+ * bounded. Additional resources may be created and/or bounded throughout that section just as well.
+ * To account for that, the switchboard is expected to acquire a use permit and then release it once
+ * the critical section is exited. Since two or more critical sections may overlap (by design), it
+ * is imperative that no resources are deallocated until the last critical section is exited. This
+ * ensures that resources that have been obtained but not yet used aren't identified as unused which
+ * would otherwise lead to their removal. This also allows switchboard to maintain a single timeout
+ * loop, freeing for example the incoming/outgoing call managers from needing to implement the same.
+ * Once the switchboard expires certain actions (e.g. pending outgoing calls) it should also release
+ * the corresponding permit, just as it would upon explicit success/failure outcomes. Subsequently,
+ * once all pending permits are released, a resource-deallocation cycle can safely follow. In terms
+ * of implementation, there is no real need to associate specific resources with the action/s during
+ * which these resources turned up to be necessary such that counting permits seems sufficient.
+ */
+final class BinderDeallocator {
+
+ private static final String TAG = BinderDeallocator.class.getSimpleName();
+
+ /**
+ * The number of actions currently permitted to use previously-allocated resources and/or create
+ * new ones.
+ */
+ private int mPermitCount = 0;
+
+ /**
+ * The set of all known binders, either in use or potentially about to be used.
+ */
+ @SuppressWarnings("rawtypes")
+ private Set<ServiceBinder> mBinders = Sets.newHashSet();
+
+ /**
+ * Accounts for the action entering a critical section (i.e. potentially needing access to
+ * resources).
+ */
+ void acquireUsePermit() {
+ ThreadUtil.checkOnMainThread();
+
+ mPermitCount++;
+ }
+
+ /**
+ * Updates the set of binders.
+ *
+ * @param binders The binders to potentially add to the all-inclusive set of known binders,
+ * see {@link #mBinders}.
+ */
+ @SuppressWarnings("rawtypes")
+ void updateBinders(Set<? extends ServiceBinder> binders) {
+ ThreadUtil.checkOnMainThread();
+
+ mBinders.addAll(binders);
+ }
+
+ /**
+ * Accounts for the action exiting a critical section (i.e. no longer needing access to
+ * resources).
+ */
+ void releaseUsePermit() {
+ ThreadUtil.checkOnMainThread();
+
+ if (mPermitCount < 1) {
+ Log.wtf(TAG, "releaseUsePermit should only be invoked upon mPermitCount > 0");
+ } else if (--mPermitCount == 0) {
+ deallocateUnusedResources();
+ }
+ }
+
+ /**
+ * Starts a resource-deallocation cycle.
+ */
+ @SuppressWarnings("rawtypes")
+ private void deallocateUnusedResources() {
+ Iterator<ServiceBinder> iterator = mBinders.iterator();
+ while (iterator.hasNext()) {
+ ServiceBinder binder = iterator.next();
+ if (binder.getAssociatedCallCount() < 1) {
+ binder.unbind();
+ mBinders.remove(binder);
+ }
+ }
+ }
+}
diff --git a/src/com/android/telecomm/Call.java b/src/com/android/telecomm/Call.java
index 7049742..8f04f1f 100644
--- a/src/com/android/telecomm/Call.java
+++ b/src/com/android/telecomm/Call.java
@@ -16,9 +16,9 @@
package com.android.telecomm;
-import android.os.RemoteException;
import android.telecomm.CallInfo;
import android.telecomm.CallState;
+import android.telecomm.ICallServiceSelector;
import android.util.Log;
import com.google.common.base.Preconditions;
@@ -55,11 +55,16 @@
private String mHandle;
/**
- * The call service which is currently connecting this call, null as long as the call is not
- * connected.
+ * The call service which is attempted or already connecting this call.
*/
private CallServiceWrapper mCallService;
+ /**
+ * The call-service selector for this call.
+ * TODO(gilad): Switch to using a wrapper object, see {@link #mCallService}.
+ */
+ private ICallServiceSelector mCallServiceSelector;
+
/** Read-only and parcelable version of this call. */
private CallInfo mCallInfo;
@@ -133,6 +138,14 @@
}
void setCallService(CallServiceWrapper callService) {
+ Preconditions.checkNotNull(callService);
+
+ if (mCallService != null) {
+ // Should never be the case, basically covering for potential programming errors.
+ decrementAssociatedCallCount(mCallService);
+ }
+
+ callService.incrementAssociatedCallCount();
mCallService = callService;
}
@@ -140,7 +153,34 @@
* Clears the associated call service.
*/
void clearCallService() {
- setCallService(null);
+ decrementAssociatedCallCount(mCallService);
+ mCallService = null;
+ }
+
+ void setCallServiceSelector(ICallServiceSelector selector) {
+ Preconditions.checkNotNull(selector);
+ mCallServiceSelector = selector;
+ }
+
+ void clearCallServiceSelector() {
+ mCallServiceSelector = null;
+
+ // TODO(gilad): Un-comment once selectors are converted into wrappers.
+ // decrementAssociatedCallCount(mCallServiceSelector);
+ }
+
+ /**
+ * Aborts ongoing attempts to connect this call. No-op once the call is connected or has been
+ * disconnected. See {@link #disconnect} for already-connected calls.
+ */
+ void abort() {
+ if (mState == CallState.NEW ||
+ mState == CallState.DIALING ||
+ mState == CallState.RINGING) {
+
+ // TODO(gilad): Add CallState.ABORTED and set it here.
+ // mState = CallState.ABORTED;
+ }
}
/**
@@ -216,4 +256,11 @@
private void clearCallInfo() {
mCallInfo = null;
}
+
+ @SuppressWarnings("rawtypes")
+ private void decrementAssociatedCallCount(ServiceBinder binder) {
+ if (binder != null) {
+ binder.decrementAssociatedCallCount();
+ }
+ }
}
diff --git a/src/com/android/telecomm/IncomingCallsManager.java b/src/com/android/telecomm/IncomingCallsManager.java
index 1539cbf..13f1319 100644
--- a/src/com/android/telecomm/IncomingCallsManager.java
+++ b/src/com/android/telecomm/IncomingCallsManager.java
@@ -16,8 +16,6 @@
package com.android.telecomm;
-import android.os.Handler;
-import android.os.Looper;
import android.telecomm.CallInfo;
import android.util.Log;
@@ -36,16 +34,8 @@
private static final String TAG = IncomingCallsManager.class.getSimpleName();
- /**
- * The amount of time to wait for details of an incoming call, in milliseconds.
- * TODO(santoscordon): Likely needs adjustment.
- */
- private static final int INCOMING_CALL_TIMEOUT_MS = 1000;
-
private final Switchboard mSwitchboard;
- private final Handler mHandler = new Handler(Looper.getMainLooper());
-
/** Maps call ID to the call. */
private final Map<String, Call> mPendingIncomingCalls = Maps.newHashMap();
@@ -64,7 +54,7 @@
*
* @param call The call object.
*/
- void retrieveIncomingCall(Call call) {
+ void retrieveIncomingCall(final Call call) {
ThreadUtil.checkOnMainThread();
Log.d(TAG, "retrieveIncomingCall");
@@ -74,11 +64,13 @@
mPendingIncomingCalls.put(callId, call);
- // TODO(santoscordon): Timeout will not be necessary after cleanup via tick() is implemented
- // in Switchboard.
- startTimeoutForCall(call);
+ Runnable errorCallback = new Runnable() {
+ @Override public void run() {
+ handleFailedIncomingCall(call);
+ }
+ };
- Runnable errorCallback = getFailedIncomingCallback(call);
+ // TODO(gilad): call.retrieve*Call() seems a bit unusual, consider revisiting.
call.getCallService().retrieveIncomingCall(callId, errorCallback);
}
@@ -115,29 +107,4 @@
mSwitchboard.handleFailedIncomingCall(call);
}
}
-
- /**
- * Starts a timeout to timebox the retrieval of an incoming call. When the timeout expires,
- * it will notify switchboard that the incoming call was not retrieved and thus does not exist
- * as far as Telecomm is concerned.
- *
- * @param call The call.
- */
- private void startTimeoutForCall(Call call) {
- Runnable timeoutCallback = getFailedIncomingCallback(call);
- mHandler.postDelayed(timeoutCallback, INCOMING_CALL_TIMEOUT_MS);
- }
-
- /**
- * Returns a runnable to be invoked upon failure to get details for an incoming call.
- *
- * @param call The failed incoming call.
- */
- private Runnable getFailedIncomingCallback(final Call call) {
- return new Runnable() {
- @Override public void run() {
- handleFailedIncomingCall(call);
- }
- };
- }
}
diff --git a/src/com/android/telecomm/OutgoingCallProcessor.java b/src/com/android/telecomm/OutgoingCallProcessor.java
index 99c0274..c5dd7ac 100644
--- a/src/com/android/telecomm/OutgoingCallProcessor.java
+++ b/src/com/android/telecomm/OutgoingCallProcessor.java
@@ -87,12 +87,6 @@
private Iterator<ICallServiceSelector> mSelectorIterator;
- /**
- * The last call service which we asked to place the call. If null, it indicates that there
- * exists no call service that we expect to place this call.
- */
- private CallServiceWrapper mCallService;
-
private boolean mIsAborted = false;
/**
@@ -143,6 +137,8 @@
if (mSelectors.isEmpty() || mCallServiceDescriptors.isEmpty()) {
// TODO(gilad): Consider adding a failure message/type to differentiate the various
// cases, or potentially throw an exception in this case.
+ // TODO(gilad): Perform this check all the way up in switchboard to short-circuit
+ // the current detour.
mOutgoingCallsManager.handleFailedOutgoingCall(mCall);
} else if (mSelectorIterator == null) {
mSelectorIterator = mSelectors.iterator();
@@ -155,8 +151,10 @@
* switchboard through the outgoing-calls manager.
*/
void abort() {
- ThreadUtil.checkOnMainThread();
- resetCallService();
+ mCall.abort();
+
+ // TODO(gilad): Add logic to notify the relevant call service and/or selector.
+
mIsAborted = true;
}
@@ -166,8 +164,16 @@
* placed the call.
*/
void handleSuccessfulCallAttempt() {
- mCall.setCallService(mCallService);
+ ThreadUtil.checkOnMainThread();
+
+ if (mIsAborted) {
+ // TODO(gilad): Ask the call service to drop the call?
+ return;
+ }
+
mCall.setState(CallState.DIALING);
+
+ // TODO(gilad): Seems better/clearer not to invoke "abort" on successfully-connected calls.
abort();
mSwitchboard.handleSuccessfulOutgoingCall(mCall);
@@ -180,6 +186,10 @@
* @param reason The call-service supplied reason for the failed call attempt.
*/
void handleFailedCallAttempt(String reason) {
+ ThreadUtil.checkOnMainThread();
+
+ mCall.clearCallService();
+ mCall.clearCallServiceSelector();
attemptNextCallService();
}
@@ -194,6 +204,8 @@
if (mSelectorIterator.hasNext()) {
ICallServiceSelector selector = mSelectorIterator.next();
+ mCall.setCallServiceSelector(selector);
+
ICallServiceSelectionResponse.Stub response = createSelectionResponse();
try {
selector.select(mCall.toCallInfo(), mCallServiceDescriptors, response);
@@ -256,34 +268,30 @@
if (mCallServiceDescriptorIterator.hasNext()) {
CallServiceDescriptor descriptor = mCallServiceDescriptorIterator.next();
- mCallService = mCallServicesById.get(descriptor.getCallServiceId());
- if (mCallService == null) {
+ final CallServiceWrapper callService =
+ mCallServicesById.get(descriptor.getCallServiceId());
+
+ if (callService == null) {
attemptNextCallService();
} else {
BindCallback callback = new BindCallback() {
@Override public void onSuccess() {
- mCallService.call(mCall.toCallInfo());
+ callService.call(mCall.toCallInfo());
}
@Override public void onFailure() {
attemptNextCallService();
}
};
+
+ mCall.setCallService(callService);
+
// TODO(santoscordon): Consider making bind private to CallServiceWrapper and having
// CSWrapper.call() do the bind automatically.
- mCallService.bind(callback);
+ callService.bind(callback);
}
} else {
mCallServiceDescriptorIterator = null;
- resetCallService();
attemptNextSelector();
}
}
-
- /**
- * Nulls out the reference to the current call service. Invoked when the call service is no longer
- * expected to place the outgoing call.
- */
- private void resetCallService() {
- mCallService = null;
- }
}
diff --git a/src/com/android/telecomm/OutgoingCallsManager.java b/src/com/android/telecomm/OutgoingCallsManager.java
index 247af8c..0a41c08 100644
--- a/src/com/android/telecomm/OutgoingCallsManager.java
+++ b/src/com/android/telecomm/OutgoingCallsManager.java
@@ -124,4 +124,16 @@
mOutgoingCallProcessors.remove(call.getId());
mSwitchboard.handleFailedOutgoingCall(call);
}
+
+ /**
+ * Aborts any ongoing attempts to connect the specified (outgoing) call.
+ *
+ * @param call The call to be aborted.
+ */
+ void abort(Call call) {
+ OutgoingCallProcessor processor = mOutgoingCallProcessors.remove(call.getId());
+ if (processor != null) {
+ processor.abort();
+ }
+ }
}
diff --git a/src/com/android/telecomm/ServiceBinder.java b/src/com/android/telecomm/ServiceBinder.java
index 3c84967..2b9cc5c 100644
--- a/src/com/android/telecomm/ServiceBinder.java
+++ b/src/com/android/telecomm/ServiceBinder.java
@@ -93,6 +93,9 @@
/** The binder provided by {@link ServiceConnection#onServiceConnected} */
private IBinder mBinder;
+ /** The number of calls currently associated with this service. */
+ private int mAssociatedCallCount = 0;
+
/**
* Indicates that an unbind request was made when the service was not yet bound. If the service
* successfully connects when this is true, it should be unbound immediately.
@@ -153,6 +156,23 @@
return true;
}
+ final void incrementAssociatedCallCount() {
+ mAssociatedCallCount++;
+ }
+
+ final void decrementAssociatedCallCount() {
+ if (mAssociatedCallCount > 0) {
+ mAssociatedCallCount--;
+ } else {
+ Log.wtf(TAG, mComponentName.getClassName() +
+ ": ignoring a request to decrement mAssociatedCallCount below zero");
+ }
+ }
+
+ final int getAssociatedCallCount() {
+ return mAssociatedCallCount;
+ }
+
/**
* Unbinds from the service if already bound, no-op otherwise.
*/
diff --git a/src/com/android/telecomm/Switchboard.java b/src/com/android/telecomm/Switchboard.java
index 8661e51..4967c36 100644
--- a/src/com/android/telecomm/Switchboard.java
+++ b/src/com/android/telecomm/Switchboard.java
@@ -27,6 +27,7 @@
import android.util.Log;
import java.util.Collection;
+import java.util.Iterator;
import java.util.List;
import java.util.Set;
@@ -42,7 +43,13 @@
* The frequency of invoking tick in milliseconds.
* TODO(gilad): May require tuning.
*/
- private final static int TICK_FREQUENCY = 250;
+ private final static int TICK_FREQUENCY_MS = 250;
+
+ /**
+ * The timeout beyond which to drop ongoing attempts to place/receive calls.
+ * TODO(gilad): May require tuning.
+ */
+ private final static int NEW_CALL_TIMEOUT_MS = 5000;
private final CallsManager mCallsManager;
@@ -56,6 +63,8 @@
private final CallServiceSelectorRepository mSelectorRepository;
+ private final BinderDeallocator mBinderDeallocator;
+
/** Used to schedule tasks on the main (UI) thread. */
private final Handler mHandler = new Handler(Looper.getMainLooper());
@@ -108,6 +117,8 @@
mSelectorRepository = new CallServiceSelectorRepository(this);
mCallServiceRepository =
new CallServiceRepository(this, mOutgoingCallsManager, mIncomingCallsManager);
+
+ mBinderDeallocator = new BinderDeallocator();
}
/**
@@ -118,7 +129,7 @@
* @param call The yet-to-be-connected outgoing-call object.
*/
void placeOutgoingCall(Call call) {
- ThreadUtil.checkOnMainThread();
+ mBinderDeallocator.acquireUsePermit();
// Reset prior to initiating the next lookup. One case to consider is (1) placeOutgoingCall
// is invoked with call A, (2) the call-service lookup completes, but the one for selectors
@@ -147,6 +158,8 @@
*/
void retrieveIncomingCall(Call call, CallServiceDescriptor descriptor) {
Log.d(TAG, "retrieveIncomingCall");
+ mBinderDeallocator.acquireUsePermit();
+
CallServiceWrapper callService = mCallServiceRepository.getCallService(descriptor);
call.setCallService(callService);
mIncomingCallsManager.retrieveIncomingCall(call);
@@ -161,7 +174,7 @@
* hence effectively omitted from the specified list.
*/
void setCallServices(Set<CallServiceWrapper> callServices) {
- ThreadUtil.checkOnMainThread();
+ mBinderDeallocator.updateBinders(mCallServices);
mCallServices = callServices;
processNewOutgoingCalls();
@@ -177,7 +190,8 @@
*/
void setSelectors(Set<ICallServiceSelector> selectors) {
// TODO(santoscordon): This should take in CallServiceSelectorWrapper instead of the direct
- // ICallServiceSelector implementation. Copy what we have for CallServiceWrapper.
+ // ICallServiceSelector implementation. Copy what we have for CallServiceWrapper. Also need
+ // to invoke updateBinders(selectors) once this to-do is addressed.
ThreadUtil.checkOnMainThread();
// TODO(gilad): Add logic to include the built-in selectors (e.g. for dealing with
@@ -193,10 +207,10 @@
* see {@link OutgoingCallProcessor}.
*/
void handleSuccessfulOutgoingCall(Call call) {
- mCallsManager.handleSuccessfulOutgoingCall(call);
+ Log.d(TAG, "handleSuccessfulOutgoingCall");
- // Process additional (new) calls, if any.
- processNewOutgoingCalls();
+ mCallsManager.handleSuccessfulOutgoingCall(call);
+ finalizeOutgoingCall(call);
}
/**
@@ -204,10 +218,11 @@
* selector/call-service implementations, see {@link OutgoingCallProcessor}.
*/
void handleFailedOutgoingCall(Call call) {
- // TODO(gilad): More here.
+ Log.d(TAG, "handleFailedOutgoingCall");
- // Process additional (new) calls, if any.
- processNewOutgoingCalls();
+ // TODO(gilad): Notify mCallsManager.
+
+ finalizeOutgoingCall(call);
}
/**
@@ -217,7 +232,9 @@
*/
void handleSuccessfulIncomingCall(Call call) {
Log.d(TAG, "handleSuccessfulIncomingCall");
+
mCallsManager.handleSuccessfulIncomingCall(call);
+ mBinderDeallocator.releaseUsePermit();
}
/**
@@ -227,10 +244,14 @@
* @param call The call.
*/
void handleFailedIncomingCall(Call call) {
+ Log.d(TAG, "handleFailedIncomingCall");
+
// Since we set the call service before calling into incoming-calls manager, we clear it for
// good measure if an error is reported.
call.clearCallService();
+ mBinderDeallocator.releaseUsePermit();
+
// At the moment there is nothing more to do if an incoming call is not retrieved. We may at
// a future date bind to the in-call app optimistically during the incoming-call sequence
// and this method could tell {@link CallsManager} to unbind from the in-call app if the
@@ -251,19 +272,7 @@
* Schedules the next tick invocation.
*/
private void scheduleNextTick() {
- mHandler.postDelayed(mTicker, TICK_FREQUENCY);
- }
-
- /**
- * Performs the set of tasks that needs to be executed on polling basis.
- * TODO(gilad): Check for stale pending calls that may need to be terminated etc, see
- * mNewOutgoingCalls and mPendingOutgoingCalls.
- * TODO(gilad): Also intended to trigger the call switching/hand-off logic when applicable.
- */
- private void tick() {
- // TODO(gilad): More here.
- // TODO(santoscordon): Clear mCallServices if there exist no more new or pending outgoing
- // calls.
+ mHandler.postDelayed(mTicker, TICK_FREQUENCY_MS);
}
/**
@@ -304,6 +313,56 @@
}
/**
+ * Finalizes the outgoing-call sequence, regardless if it succeeded or failed.
+ */
+ private void finalizeOutgoingCall(Call call) {
+ mPendingOutgoingCalls.remove(call);
+
+ mBinderDeallocator.releaseUsePermit();
+ processNewOutgoingCalls(); // Process additional (new) calls, if any.
+ }
+
+ /**
+ * Performs the set of tasks that needs to be executed on polling basis.
+ */
+ private void tick() {
+ // TODO(gilad): Add the necessary logic to support switching.
+
+ expireStaleOutgoingCalls(mNewOutgoingCalls);
+ expireStaleOutgoingCalls(mPendingOutgoingCalls);
+ }
+
+ /**
+ * Identifies stale calls and takes the necessary steps to mark these as expired.
+ *
+ * @param calls The set of calls to iterate through.
+ */
+ private void expireStaleOutgoingCalls(Set<Call> calls) {
+ if (calls.isEmpty()) {
+ return;
+ }
+
+ Iterator<Call> iterator = calls.iterator();
+ while (iterator.hasNext()) {
+ Call call = iterator.next();
+ if (call.getAgeInMilliseconds() >= NEW_CALL_TIMEOUT_MS) {
+ mOutgoingCallsManager.abort(call);
+ calls.remove(call);
+
+ // TODO(gilad): We may also have expired calls that are not yet associated with an
+ // OutgoingCallProcessor (e.g. when newer calls are "blocked" on older-yet-expired
+ // ones), in which case call.abort may need to be invoked directly. Alternatively
+ // we can also create an OutgoingCallsManager instance for every new call at intent-
+ // processing time.
+
+ // TODO(gilad): Notify the user in the relevant cases (at least outgoing).
+
+ mBinderDeallocator.releaseUsePermit();
+ }
+ }
+ }
+
+ /**
* Determines whether or not the specified collection is either null or empty.
*
* @param collection Either null or the collection object to be evaluated.