Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 4ac59489 authored by Jan Sebechlebsky's avatar Jan Sebechlebsky
Browse files

Fix ContextImpl leak in ActivityThread.

ActivityThread caches system ui contexts per display,
but never removes them when the corresponding displays
are removed.

This is especially problematic with VirtualDisplay-s and ActivityThread
instance used in system server.

This cl modifies the cache to use WeakRef and allow instances to
be garbage collected.

Also, since the context can be moved between the displays, it's possible
that the cached instance doesn't correspond to the display in the
SparseArray key at the moment it's fetched.

This cl stores the instances in the simple array list
(The number of instances at the same time should be low, and we
would anyway need to traverse the whole SparseArray to check for
moved instances).

Bug: 330472805
Test: verifying heapdump after atest CtsVirtualDevicesAppLaunchTestCases --iterations 10
Change-Id: I8811945560381653cac8363d4086f87d253e22db
parent dcfc1a87
Loading
Loading
Loading
Loading
+30 −13
Original line number Diff line number Diff line
@@ -365,7 +365,7 @@ public final class ActivityThread extends ClientTransactionHandler
    @UnsupportedAppUsage
    private ContextImpl mSystemContext;
    @GuardedBy("this")
    private SparseArray<ContextImpl> mDisplaySystemUiContexts;
    private ArrayList<WeakReference<ContextImpl>> mDisplaySystemUiContexts;

    @UnsupportedAppUsage
    static volatile IPackageManager sPackageManager;
@@ -3033,14 +3033,19 @@ public final class ActivityThread extends ClientTransactionHandler
    public ContextImpl getSystemUiContext(int displayId) {
        synchronized (this) {
            if (mDisplaySystemUiContexts == null) {
                mDisplaySystemUiContexts = new SparseArray<>();
                mDisplaySystemUiContexts = new ArrayList<>();
            }
            ContextImpl systemUiContext = mDisplaySystemUiContexts.get(displayId);
            if (systemUiContext == null) {
                systemUiContext = ContextImpl.createSystemUiContext(getSystemContext(), displayId);
                mDisplaySystemUiContexts.put(displayId, systemUiContext);

            mDisplaySystemUiContexts.removeIf(contextRef -> contextRef.refersTo(null));

            ContextImpl context = getSystemUiContextNoCreateLocked(displayId);
            if (context != null) {
                return context;
            }
            return systemUiContext;

            context = ContextImpl.createSystemUiContext(getSystemContext(), displayId);
            mDisplaySystemUiContexts.add(new WeakReference<>(context));
            return context;
        }
    }

@@ -3048,18 +3053,30 @@ public final class ActivityThread extends ClientTransactionHandler
    @Override
    public ContextImpl getSystemUiContextNoCreate() {
        synchronized (this) {
            if (mDisplaySystemUiContexts == null) return null;
            return mDisplaySystemUiContexts.get(DEFAULT_DISPLAY);
            if (mDisplaySystemUiContexts == null) {
                return null;
            }
            return getSystemUiContextNoCreateLocked(DEFAULT_DISPLAY);
        }
    }

    @GuardedBy("this")
    @Nullable
    private ContextImpl getSystemUiContextNoCreateLocked(int displayId) {
        for (int i = 0; i < mDisplaySystemUiContexts.size(); i++) {
            ContextImpl context = mDisplaySystemUiContexts.get(i).get();
            if (context != null && context.getDisplayId() == displayId) {
                return context;
            }
        }
        return null;
    }

    void onSystemUiContextCleanup(ContextImpl context) {
        synchronized (this) {
            if (mDisplaySystemUiContexts == null) return;
            final int index = mDisplaySystemUiContexts.indexOfValue(context);
            if (index >= 0) {
                mDisplaySystemUiContexts.removeAt(index);
            }
            mDisplaySystemUiContexts.removeIf(
                    contextRef -> contextRef.refersTo(null) || contextRef.refersTo(context));
        }
    }