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

Commit 3fb4b7bb authored by Robin Lee's avatar Robin Lee Committed by Pablo Gamito
Browse files

Use stable objects for callbacks 

Fixes a memory leak contributing to b/419430883.

In Android's Runtime, method references are not stable across callsites.

Concretely If you register (this::fun) and unregister (this::fun), this
may lead to twice as much fun as you bargained for. The lambda instances
generated for register and unregister are NOT equal!

This leads to memory leaks as the registration works but the
deregistration is not recognised, leading to everlasting fun.

Bug: 419430883
Test: atest WMShellUnitTests # sufficiently large to trigger the leak
Flag: android.tracing.client_side_proto_logging
Change-Id: Idc478fdf60985d86144e9ec317b198a038f60b1e
parent 674ca99d

core/java/com/android/internal/protolog/PerfettoProtoLogImpl.java

+17 −10
Original line number Diff line number Diff line
@@ -62,6 +62,9 @@ import android.util.proto.ProtoOutputStream; 


import com.android.internal.annotations.VisibleForTesting;

import com.android.internal.protolog.IProtoLogConfigurationService.RegisterClientArgs;

import com.android.internal.protolog.ProtoLogDataSource.Instance.TracingFlushCallback;

import com.android.internal.protolog.ProtoLogDataSource.Instance.TracingInstanceStartCallback;

import com.android.internal.protolog.ProtoLogDataSource.Instance.TracingInstanceStopCallback;

import com.android.internal.protolog.common.ILogger;

import com.android.internal.protolog.common.IProtoLog;

import com.android.internal.protolog.common.IProtoLogGroup;
@@ -93,7 +96,8 @@ import java.util.concurrent.locks.ReentrantReadWriteLock; 
/**

 * A service for the ProtoLog logging system.

 */

public abstract class PerfettoProtoLogImpl extends IProtoLogClient.Stub implements IProtoLog {

public abstract class PerfettoProtoLogImpl extends IProtoLogClient.Stub implements IProtoLog,

        TracingInstanceStartCallback, TracingInstanceStopCallback, TracingFlushCallback {

    private static final String LOG_TAG = "ProtoLog";

    public static final String NULL_STRING = "null";

    private final AtomicInteger mTracingInstances = new AtomicInteger();
@@ -172,9 +176,9 @@ public abstract class PerfettoProtoLogImpl extends IProtoLogClient.Stub implemen 
            connectToConfigurationService();

        }



        mDataSource.registerOnStartCallback(this::onTracingInstanceStart);

        mDataSource.registerOnFlushCallback(this::onTracingFlush);

        mDataSource.registerOnStopCallback(this::onTracingInstanceStop);

        mDataSource.registerOnStartCallback(this);

        mDataSource.registerOnFlushCallback(this);

        mDataSource.registerOnStopCallback(this);

    }



    private void connectToConfigurationService() {
@@ -207,9 +211,9 @@ public abstract class PerfettoProtoLogImpl extends IProtoLogClient.Stub implemen 
     * the datasource and the configuration service to ensure we no longer receive the callback.

     */

    public void disable() {

        mDataSource.unregisterOnStartCallback(this::onTracingInstanceStart);

        mDataSource.unregisterOnFlushCallback(this::onTracingFlush);

        mDataSource.unregisterOnStopCallback(this::onTracingInstanceStop);

        mDataSource.unregisterOnStartCallback(this);

        mDataSource.unregisterOnFlushCallback(this);

        mDataSource.unregisterOnStopCallback(this);



        if (android.tracing.Flags.clientSideProtoLogging()) {

            try {
@@ -419,7 +423,8 @@ public abstract class PerfettoProtoLogImpl extends IProtoLogClient.Stub implemen 
        }

    }



    private void onTracingFlush() {

    @Override

    public void onTracingFlush() {

        Log.d(LOG_TAG, "Executing onTracingFlush");

        waitForExistingBackgroundTasksToComplete();
@@ -747,7 +752,8 @@ public abstract class PerfettoProtoLogImpl extends IProtoLogClient.Stub implemen 
        return -1;

    }



    private void onTracingInstanceStart(

    @Override

    public void onTracingInstanceStart(

            int instanceIdx, ProtoLogDataSource.ProtoLogConfig config) {

        Log.d(LOG_TAG, "Executing onTracingInstanceStart");
@@ -790,7 +796,8 @@ public abstract class PerfettoProtoLogImpl extends IProtoLogClient.Stub implemen 
        this.mTracingInstances.incrementAndGet();

    }



    private synchronized void onTracingInstanceStop(

    @Override

    public synchronized void onTracingInstanceStop(

            int instanceIdx, ProtoLogDataSource.ProtoLogConfig config) {

        Log.d(LOG_TAG, "Executing onTracingInstanceStop");

core/java/com/android/internal/protolog/ProtoLogDataSource.java

+27 −17
Original line number Diff line number Diff line
@@ -58,7 +58,7 @@ public class ProtoLogDataSource extends DataSource<ProtoLogDataSource.Instance, 
    @NonNull

    private final Set<Instance.TracingInstanceStartCallback> mOnStartCallbacks = new HashSet<>();

    @NonNull

    private final Set<Runnable> mOnFlushCallbacks = new HashSet<>();

    private final Set<Instance.TracingFlushCallback> mOnFlushCallbacks = new HashSet<>();

    @NonNull

    private final Set<Instance.TracingInstanceStopCallback> mOnStopCallbacks = new HashSet<>();
@@ -133,17 +133,16 @@ public class ProtoLogDataSource extends DataSource<ProtoLogDataSource.Instance, 
            Instance.TracingInstanceStartCallback onStartCallback) {

        mOnStartCallbacks.add(onStartCallback);



        for (var instanceIndex : mRunningInstances.keySet()) {

            var config = mRunningInstances.get(instanceIndex);

            onStartCallback.run(instanceIndex, config);

        }

        mRunningInstances.forEach((index, config) -> {

            onStartCallback.onTracingInstanceStart(index, config);

        });

    }



    /**

     * Register an onFlush callback that will be called when a tracing instance is about to flush.

     * @param onFlushCallback The callback to call on flushing a tracing instance

     */

    public void registerOnFlushCallback(Runnable onFlushCallback) {

    public void registerOnFlushCallback(Instance.TracingFlushCallback onFlushCallback) {

        mOnFlushCallbacks.add(onFlushCallback);

    }
@@ -167,7 +166,7 @@ public class ProtoLogDataSource extends DataSource<ProtoLogDataSource.Instance, 
     * Unregister an onFlush callback.

     * @param onFlushCallback The callback object to unregister.

     */

    public void unregisterOnFlushCallback(Runnable onFlushCallback) {

    public void unregisterOnFlushCallback(Instance.TracingFlushCallback onFlushCallback) {

        mOnFlushCallbacks.add(onFlushCallback);

    }
@@ -183,13 +182,13 @@ public class ProtoLogDataSource extends DataSource<ProtoLogDataSource.Instance, 
        mRunningInstances.put(instanceIdx, config);



        for (var onStart : mOnStartCallbacks) {

            onStart.run(instanceIdx, config);

            onStart.onTracingInstanceStart(instanceIdx, config);

        }

    }



    private void executeOnFlushCallbacks() {

        for (var onFlush : mOnFlushCallbacks) {

            onFlush.run();

            onFlush.onTracingFlush();

        }

    }
@@ -197,7 +196,7 @@ public class ProtoLogDataSource extends DataSource<ProtoLogDataSource.Instance, 
        mRunningInstances.remove(instanceIdx, config);



        for (var onStop : mOnStopCallbacks) {

            onStop.run(instanceIdx, config);

            onStop.onTracingInstanceStop(instanceIdx, config);

        }

    }
@@ -355,6 +354,7 @@ public class ProtoLogDataSource extends DataSource<ProtoLogDataSource.Instance, 


    public static class Instance extends DataSourceInstance {



        @FunctionalInterface

        public interface TracingInstanceStartCallback {

            /**

             * Execute the tracing instance's onStart callback.
@@ -363,9 +363,18 @@ public class ProtoLogDataSource extends DataSource<ProtoLogDataSource.Instance, 
             * @param config The protolog configuration for the tracing instance we are executing

             *               the callback for.

             */

            void run(int instanceIdx, @NonNull ProtoLogConfig config);

            void onTracingInstanceStart(int instanceIdx, @NonNull ProtoLogConfig config);

        }



        @FunctionalInterface

        public interface TracingFlushCallback {

            /**

             * Execute the tracing instance's onFlush callback.

             */

            void onTracingFlush();

        }



        @FunctionalInterface

        public interface TracingInstanceStopCallback {

            /**

             * Execute the tracing instance's onStop callback.
@@ -374,13 +383,14 @@ public class ProtoLogDataSource extends DataSource<ProtoLogDataSource.Instance, 
             * @param config The protolog configuration for the tracing instance we are executing

             *               the callback for.

             */

            void run(int instanceIdx, @NonNull ProtoLogConfig config);

            void onTracingInstanceStop(int instanceIdx, @NonNull ProtoLogConfig config);

        }





        @NonNull

        private final TracingInstanceStartCallback mOnStart;

        @NonNull

        private final Runnable mOnFlush;

        private final TracingFlushCallback mOnFlush;

        @NonNull

        private final TracingInstanceStopCallback mOnStop;

        @NonNull
@@ -392,7 +402,7 @@ public class ProtoLogDataSource extends DataSource<ProtoLogDataSource.Instance, 
                int instanceIdx,

                @NonNull ProtoLogConfig config,

                @NonNull TracingInstanceStartCallback onStart,

                @NonNull Runnable onFlush,

                @NonNull TracingFlushCallback onFlush,

                @NonNull TracingInstanceStopCallback onStop

        ) {

            super(dataSource, instanceIdx);
@@ -405,17 +415,17 @@ public class ProtoLogDataSource extends DataSource<ProtoLogDataSource.Instance, 


        @Override

        public void onStart(StartCallbackArguments args) {

            this.mOnStart.run(this.mInstanceIndex, this.mConfig);

            this.mOnStart.onTracingInstanceStart(this.mInstanceIndex, this.mConfig);

        }



        @Override

        public void onFlush(FlushCallbackArguments args) {

            this.mOnFlush.run();

            this.mOnFlush.onTracingFlush();

        }



        @Override

        public void onStop(StopCallbackArguments args) {

            this.mOnStop.run(this.mInstanceIndex, this.mConfig);

            this.mOnStop.onTracingInstanceStop(this.mInstanceIndex, this.mConfig);

        }

    }

}