From 4a9f129874785d34930f165268dbc618e4bc2618 Mon Sep 17 00:00:00 2001 From: Craig Mautner Date: Wed, 11 Jun 2014 13:44:50 -0700 Subject: [PATCH] Close ActivityView and ActivityContainer cleanly. - Do not call IActivityContainer.release() from ActivityView.finalize() if it has already been called from ActivityView.release(). Eliminates IBinder finalized Exception. - Call ActivityRecord.makeFinishing() before calling ActivityStack. destroyActivityLocked(). Forces call to scheduleDestroyActivity() and eventually removeFromHistory(). Otherwise removeFromHistory() is never called and window manager AppWindowTokens become orphans. - Defer call to ActivityContainer.detachLocked() until all activities have finished or timed out. Fixes problem where Display is removed while activities are still launching. - Call ActivityStackSupervisor.deleteActivityContainer() when all activities have finished or timed out. Fixes orphaned ActivityContainers. Fixes bug 15450798. Fixes bug 15484154. Fixes bug 15383479. Fixes bug 15316558. Fixes bug 15168560. Fixes bug 15143914. Change-Id: Id3c641976b6f825458690f9ee063c07818b56f23 --- core/java/android/app/ActivityView.java | 17 ++-- .../com/android/server/am/ActivityStack.java | 15 ++-- .../server/am/ActivityStackSupervisor.java | 87 ++++++++++++------- 3 files changed, 78 insertions(+), 41 deletions(-) diff --git a/core/java/android/app/ActivityView.java b/core/java/android/app/ActivityView.java index c29d75e516efa..94ea2c5b4d907 100644 --- a/core/java/android/app/ActivityView.java +++ b/core/java/android/app/ActivityView.java @@ -354,9 +354,11 @@ public class ActivityView extends ViewGroup { private static class ActivityContainerWrapper { private final IActivityContainer mIActivityContainer; private final CloseGuard mGuard = CloseGuard.get(); + boolean mOpened; // Protected by mGuard. ActivityContainerWrapper(IActivityContainer container) { mIActivityContainer = container; + mOpened = true; mGuard.open("release"); } @@ -424,11 +426,16 @@ public class ActivityView extends ViewGroup { } void release() { - if (DEBUG) Log.v(TAG, "ActivityContainerWrapper: release called"); - try { - mIActivityContainer.release(); - mGuard.close(); - } catch (RemoteException e) { + synchronized (mGuard) { + if (mOpened) { + if (DEBUG) Log.v(TAG, "ActivityContainerWrapper: release called"); + try { + mIActivityContainer.release(); + mGuard.close(); + } catch (RemoteException e) { + } + mOpened = false; + } } } diff --git a/services/core/java/com/android/server/am/ActivityStack.java b/services/core/java/com/android/server/am/ActivityStack.java index 685da13b8ada0..d894e4eb2f109 100755 --- a/services/core/java/com/android/server/am/ActivityStack.java +++ b/services/core/java/com/android/server/am/ActivityStack.java @@ -32,12 +32,11 @@ import static com.android.server.am.ActivityManagerService.VALIDATE_TOKENS; import static com.android.server.am.ActivityStackSupervisor.DEBUG_ADD_REMOVE; import static com.android.server.am.ActivityStackSupervisor.DEBUG_APP; +import static com.android.server.am.ActivityStackSupervisor.DEBUG_CONTAINERS; import static com.android.server.am.ActivityStackSupervisor.DEBUG_SAVED_STATE; import static com.android.server.am.ActivityStackSupervisor.DEBUG_STATES; import static com.android.server.am.ActivityStackSupervisor.HOME_STACK_ID; -import static com.android.server.am.ActivityStackSupervisor.ActivityContainer.CONTAINER_STATE_HAS_SURFACE; - import com.android.internal.os.BatteryStatsImpl; import com.android.server.Watchdog; import com.android.server.am.ActivityManagerService.ItemMatcher; @@ -1275,7 +1274,7 @@ final class ActivityStack { ActivityRecord parent = mActivityContainer.mParentActivity; if ((parent != null && parent.state != ActivityState.RESUMED) || - !mActivityContainer.isAttached()) { + !mActivityContainer.isAttachedLocked()) { // Do not resume this stack if its parent is not resumed. // TODO: If in a loop, make sure that parent stack resumeTopActivity is called 1st. return false; @@ -2539,11 +2538,14 @@ final class ActivityStack { || prevState == ActivityState.INITIALIZING) { // If this activity is already stopped, we can just finish // it right now. - boolean activityRemoved = destroyActivityLocked(r, true, - oomAdj, "finish-imm"); + r.makeFinishing(); + boolean activityRemoved = destroyActivityLocked(r, true, oomAdj, "finish-imm"); if (activityRemoved) { mStackSupervisor.resumeTopActivitiesLocked(); } + if (DEBUG_CONTAINERS) Slog.d(TAG, + "destroyActivityLocked: finishCurrentActivityLocked r=" + r + + " destroy returned removed=" + activityRemoved); return activityRemoved ? null : r; } @@ -2912,6 +2914,7 @@ final class ActivityStack { if (r != null) { mHandler.removeMessages(DESTROY_TIMEOUT_MSG, r); } + if (DEBUG_CONTAINERS) Slog.d(TAG, "activityDestroyedLocked: r=" + r); if (isInStackLocked(token) != null) { if (r.state == ActivityState.DESTROYING) { @@ -3669,7 +3672,7 @@ final class ActivityStack { mStacks.remove(this); mStacks.add(0, this); } - mActivityContainer.onTaskListEmpty(); + mActivityContainer.onTaskListEmptyLocked(); } } diff --git a/services/core/java/com/android/server/am/ActivityStackSupervisor.java b/services/core/java/com/android/server/am/ActivityStackSupervisor.java index ed6a8bbb9a7fc..6cb57de8bc4a7 100644 --- a/services/core/java/com/android/server/am/ActivityStackSupervisor.java +++ b/services/core/java/com/android/server/am/ActivityStackSupervisor.java @@ -105,6 +105,7 @@ public final class ActivityStackSupervisor implements DisplayListener { static final boolean DEBUG_SAVED_STATE = DEBUG || false; static final boolean DEBUG_STATES = DEBUG || false; static final boolean DEBUG_IDLE = DEBUG || false; + static final boolean DEBUG_CONTAINERS = DEBUG || false; public static final int HOME_STACK_ID = 0; @@ -127,6 +128,7 @@ public final class ActivityStackSupervisor implements DisplayListener { static final int HANDLE_DISPLAY_REMOVED = FIRST_SUPERVISOR_STACK_MSG + 7; static final int CONTAINER_CALLBACK_VISIBILITY = FIRST_SUPERVISOR_STACK_MSG + 8; static final int CONTAINER_CALLBACK_TASK_LIST_EMPTY = FIRST_SUPERVISOR_STACK_MSG + 9; + static final int CONTAINER_TASK_LIST_EMPTY_TIMEOUT = FIRST_SUPERVISOR_STACK_MSG + 10; private final static String VIRTUAL_DISPLAY_BASE_NAME = "ActivityViewVirtualDisplay"; @@ -224,7 +226,7 @@ public final class ActivityStackSupervisor implements DisplayListener { // TODO: Add listener for removal of references. /** Mapping from (ActivityStack/TaskStack).mStackId to their current state */ - SparseArray mActivityContainers = new SparseArray(); + private SparseArray mActivityContainers = new SparseArray(); /** Mapping from displayId to display current state */ private final SparseArray mActivityDisplays = @@ -2161,8 +2163,10 @@ public final class ActivityStackSupervisor implements DisplayListener { ActivityContainer createActivityContainer(ActivityRecord parentActivity, IActivityContainerCallback callback) { - ActivityContainer activityContainer = new VirtualActivityContainer(parentActivity, callback); + ActivityContainer activityContainer = + new VirtualActivityContainer(parentActivity, callback); mActivityContainers.put(activityContainer.mStackId, activityContainer); + if (DEBUG_CONTAINERS) Slog.d(TAG, "createActivityContainer: " + activityContainer); parentActivity.mChildContainers.add(activityContainer); return activityContainer; } @@ -2171,6 +2175,8 @@ public final class ActivityStackSupervisor implements DisplayListener { final ArrayList childStacks = parentActivity.mChildContainers; for (int containerNdx = childStacks.size() - 1; containerNdx >= 0; --containerNdx) { ActivityContainer container = childStacks.remove(containerNdx); + if (DEBUG_CONTAINERS) Slog.d(TAG, "removeChildActivityContainers: removing " + + container); container.release(); } } @@ -2178,11 +2184,8 @@ public final class ActivityStackSupervisor implements DisplayListener { void deleteActivityContainer(IActivityContainer container) { ActivityContainer activityContainer = (ActivityContainer)container; if (activityContainer != null) { - activityContainer.mStack.finishAllActivitiesLocked(); - final ActivityRecord parent = activityContainer.mParentActivity; - if (parent != null) { - parent.mChildContainers.remove(activityContainer); - } + if (DEBUG_CONTAINERS) Slog.d(TAG, "deleteActivityContainer: ", + new RuntimeException("here").fillInStackTrace()); final int stackId = activityContainer.mStackId; mActivityContainers.remove(stackId); mWindowManager.removeStack(stackId); @@ -2950,6 +2953,13 @@ public final class ActivityStackSupervisor implements DisplayListener { } } } break; + case CONTAINER_TASK_LIST_EMPTY_TIMEOUT: { + synchronized (mService) { + Slog.w(TAG, "Timeout waiting for all activities in task to finish. " + + msg.obj); + ((ActivityContainer) msg.obj).onTaskListEmptyLocked(); + } + } break; } } } @@ -3006,8 +3016,10 @@ public final class ActivityStackSupervisor implements DisplayListener { @Override public int getDisplayId() { - if (mActivityDisplay != null) { - return mActivityDisplay.mDisplayId; + synchronized (mService) { + if (mActivityDisplay != null) { + return mActivityDisplay.mDisplayId; + } } return -1; } @@ -3016,10 +3028,12 @@ public final class ActivityStackSupervisor implements DisplayListener { public boolean injectEvent(InputEvent event) { final long origId = Binder.clearCallingIdentity(); try { - if (mActivityDisplay != null) { - return mInputManagerInternal.injectInputEvent(event, - mActivityDisplay.mDisplayId, - InputManager.INJECT_INPUT_EVENT_MODE_ASYNC); + synchronized (mService) { + if (mActivityDisplay != null) { + return mInputManagerInternal.injectInputEvent(event, + mActivityDisplay.mDisplayId, + InputManager.INJECT_INPUT_EVENT_MODE_ASYNC); + } } return false; } finally { @@ -3029,10 +3043,16 @@ public final class ActivityStackSupervisor implements DisplayListener { @Override public void release() { - mContainerState = CONTAINER_STATE_FINISHING; - mStack.finishAllActivitiesLocked(); - detachLocked(); - mWindowManager.removeStack(mStackId); + synchronized (mService) { + if (mContainerState == CONTAINER_STATE_FINISHING) { + return; + } + mContainerState = CONTAINER_STATE_FINISHING; + final Message msg = + mHandler.obtainMessage(CONTAINER_TASK_LIST_EMPTY_TIMEOUT, this); + mHandler.sendMessageDelayed(msg, 1000); + mStack.finishAllActivitiesLocked(); + } } private void detachLocked() { @@ -3123,15 +3143,17 @@ public final class ActivityStackSupervisor implements DisplayListener { return ActivityStackSupervisor.this; } - boolean isAttached() { + boolean isAttachedLocked() { return mActivityDisplay != null; } void getBounds(Point outBounds) { - if (mActivityDisplay != null) { - mActivityDisplay.getBounds(outBounds); - } else { - outBounds.set(0, 0); + synchronized (mService) { + if (mActivityDisplay != null) { + mActivityDisplay.getBounds(outBounds); + } else { + outBounds.set(0, 0); + } } } @@ -3154,7 +3176,12 @@ public final class ActivityStackSupervisor implements DisplayListener { return true; } - void onTaskListEmpty() { + void onTaskListEmptyLocked() { + mHandler.removeMessages(CONTAINER_TASK_LIST_EMPTY_TIMEOUT, this); + if (!mStack.isHomeStack()) { + detachLocked(); + deleteActivityContainer(this); + } mHandler.obtainMessage(CONTAINER_CALLBACK_TASK_LIST_EMPTY, this).sendToTarget(); } @@ -3173,7 +3200,7 @@ public final class ActivityStackSupervisor implements DisplayListener { mParentActivity = parent; mCallback = callback; mContainerState = CONTAINER_STATE_NO_SURFACE; - mIdString = "VirtualActivtyContainer{" + mStackId + ", parent=" + mParentActivity + "}"; + mIdString = "VirtualActivityContainer{" + mStackId + ", parent=" + mParentActivity + "}"; } @Override @@ -3219,22 +3246,22 @@ public final class ActivityStackSupervisor implements DisplayListener { } } - setSurfaceIfReady(); + setSurfaceIfReadyLocked(); if (DEBUG_STACK) Slog.d(TAG, "setSurface: " + this + " to display=" + virtualActivityDisplay); } @Override - boolean isAttached() { - return mSurface != null && super.isAttached(); + boolean isAttachedLocked() { + return mSurface != null && super.isAttachedLocked(); } @Override void setDrawn() { synchronized (mService) { mDrawn = true; - setSurfaceIfReady(); + setSurfaceIfReadyLocked(); } } @@ -3244,8 +3271,8 @@ public final class ActivityStackSupervisor implements DisplayListener { return false; } - private void setSurfaceIfReady() { - if (DEBUG_STACK) Slog.v(TAG, "setSurfaceIfReady: mDrawn=" + mDrawn + + private void setSurfaceIfReadyLocked() { + if (DEBUG_STACK) Slog.v(TAG, "setSurfaceIfReadyLocked: mDrawn=" + mDrawn + " mContainerState=" + mContainerState + " mSurface=" + mSurface); if (mDrawn && mSurface != null && mContainerState == CONTAINER_STATE_NO_SURFACE) { ((VirtualActivityDisplay) mActivityDisplay).setSurface(mSurface);