Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix GraphicBuffer leaks in system_server

Browse Source 
IWindowId is being kept alive by binder in certain cases, which
will leak WindowState, which can leak a whole lot including
the snapshot GraphicBuffer.

- Fix a leak in WindowContainer.mTmpChain1/2
- When the window gets removed, make sure to release the reference
to the window state so even if Binder keeps IWindowId alive, we
don't keep WindowState alive
- Make sure that even if we leak TaskSnapshotSurface we don't leak
the GraphicBuffer.

Test: Boot, some basic sanity testing.
Bug: 36279079
Change-Id: I4a0a784b32bc2df47934b1bed1d62b0682beb2dd
This commit is contained in:
Jorim Jaggi
2017-05-16 22:26:02 +02:00
parent 95cf170ce9
commit 4448e1ebb2
3 changed files with 77 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
services/core/java/com/android/server/wm/TaskSnapshotSurface.java
View File

@@ -113,7 +113,7 @@ class TaskSnapshotSurface implements StartingSurface {
private final Rect mStableInsets = new Rect();
private final Rect mContentInsets = new Rect();
private final Rect mFrame = new Rect();
private final TaskSnapshot mSnapshot;
private TaskSnapshot mSnapshot;
private final CharSequence mTitle;
private boolean mHasDrawn;
private long mShownTime;
@@ -267,6 +267,9 @@ class TaskSnapshotSurface implements StartingSurface {
mHasDrawn = true;
}
reportDrawn();
// In case window manager leaks us, make sure we don't retain the snapshot.
mSnapshot = null;
}
private void drawSizeMatchSnapshot(GraphicBuffer buffer) {

67
services/core/java/com/android/server/wm/WindowContainer.java
View File

@@ -655,39 +655,44 @@ class WindowContainer<E extends WindowContainer> implements Comparable<WindowCon
final LinkedList<WindowContainer> thisParentChain = mTmpChain1;
final LinkedList<WindowContainer> otherParentChain = mTmpChain2;
getParents(thisParentChain);
other.getParents(otherParentChain);
try {
getParents(thisParentChain);
other.getParents(otherParentChain);
// Find the common ancestor of both containers.
WindowContainer commonAncestor = null;
WindowContainer thisTop = thisParentChain.peekLast();
WindowContainer otherTop = otherParentChain.peekLast();
while (thisTop != null && otherTop != null && thisTop == otherTop) {
commonAncestor = thisParentChain.removeLast();
otherParentChain.removeLast();
thisTop = thisParentChain.peekLast();
otherTop = otherParentChain.peekLast();
// Find the common ancestor of both containers.
WindowContainer commonAncestor = null;
WindowContainer thisTop = thisParentChain.peekLast();
WindowContainer otherTop = otherParentChain.peekLast();
while (thisTop != null && otherTop != null && thisTop == otherTop) {
commonAncestor = thisParentChain.removeLast();
otherParentChain.removeLast();
thisTop = thisParentChain.peekLast();
otherTop = otherParentChain.peekLast();
}
// Containers don't belong to the same hierarchy???
if (commonAncestor == null) {
throw new IllegalArgumentException("No in the same hierarchy this="
+ thisParentChain + " other=" + otherParentChain);
}
// Children are always considered greater than their parents, so if one of the containers
// we are comparing it the parent of the other then whichever is the child is greater.
if (commonAncestor == this) {
return -1;
} else if (commonAncestor == other) {
return 1;
}
// The position of the first non-common ancestor in the common ancestor list determines
// which is greater the which.
final WindowList<WindowContainer> list = commonAncestor.mChildren;
return list.indexOf(thisParentChain.peekLast()) > list.indexOf(otherParentChain.peekLast())
? 1 : -1;
} finally {
mTmpChain1.clear();
mTmpChain2.clear();
}
// Containers don't belong to the same hierarchy???
if (commonAncestor == null) {
throw new IllegalArgumentException("No in the same hierarchy this="
+ thisParentChain + " other=" + otherParentChain);
}
// Children are always considered greater than their parents, so if one of the containers
// we are comparing it the parent of the other then whichever is the child is greater.
if (commonAncestor == this) {
return -1;
} else if (commonAncestor == other) {
return 1;
}
// The position of the first non-common ancestor in the common ancestor list determines
// which is greater the which.
final WindowList<WindowContainer> list = commonAncestor.mChildren;
return list.indexOf(thisParentChain.peekLast()) > list.indexOf(otherParentChain.peekLast())
? 1 : -1;
}
private void getParents(LinkedList<WindowContainer> parents) {

52
services/core/java/com/android/server/wm/WindowState.java
View File

@@ -138,6 +138,7 @@ import com.android.internal.util.ToBooleanFunction;
import com.android.server.input.InputWindowHandle;
import java.io.PrintWriter;
import java.lang.ref.WeakReference;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.LinkedList;
@@ -170,7 +171,7 @@ class WindowState extends WindowContainer<WindowState> implements WindowManagerP
final int mOwnerUid;
/** The owner has {@link android.Manifest.permission#INTERNAL_SYSTEM_WINDOW} */
final boolean mOwnerCanAddInternalSystemWindow;
final IWindowId mWindowId;
final WindowId mWindowId;
WindowToken mToken;
// The same object as mToken if this is an app window and null for non-app windows.
AppWindowToken mAppToken;
@@ -587,20 +588,7 @@ class WindowState extends WindowContainer<WindowState> implements WindowManagerP
mAppToken = mToken.asAppWindowToken();
mOwnerUid = ownerId;
mOwnerCanAddInternalSystemWindow = ownerCanAddInternalSystemWindow;
mWindowId = new IWindowId.Stub() {
@Override
public void registerFocusObserver(IWindowFocusObserver observer) {
WindowState.this.registerFocusObserver(observer);
}
@Override
public void unregisterFocusObserver(IWindowFocusObserver observer) {
WindowState.this.unregisterFocusObserver(observer);
}
@Override
public boolean isFocused() {
return WindowState.this.isFocused();
}
};
mWindowId = new WindowId(this);
mAttrs.copyFrom(a);
mViewVisibility = viewVisibility;
mPolicy = mService.mPolicy;
@@ -4435,6 +4423,40 @@ class WindowState extends WindowContainer<WindowState> implements WindowManagerP
}
}
private static final class WindowId extends IWindowId.Stub {
private final WeakReference<WindowState> mOuter;
private WindowId(WindowState outer) {
// Use a weak reference for the outer class. This is important to prevent the following
// leak: Since we send this class to the client process, binder will keep it alive as
// long as the client keeps it alive. Now, if the window is removed, we need to clear
// out our reference so even though this class is kept alive we don't leak WindowState,
// which can keep a whole lot of classes alive.
mOuter = new WeakReference<>(outer);
}
@Override
public void registerFocusObserver(IWindowFocusObserver observer) {
final WindowState outer = mOuter.get();
if (outer != null) {
outer.registerFocusObserver(observer);
}
}
@Override
public void unregisterFocusObserver(IWindowFocusObserver observer) {
final WindowState outer = mOuter.get();
if (outer != null) {
outer.unregisterFocusObserver(observer);
}
}
@Override
public boolean isFocused() {
final WindowState outer = mOuter.get();
return outer != null && outer.isFocused();
}
}
boolean usesRelativeZOrdering() {
if (!isChildWindow()) {
return false;