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

Commit ecc22e2f authored by TreeHugger Robot's avatar TreeHugger Robot Committed by Android (Google) Code Review
Browse files

Merge changes I789db896,I19e91f0f

* changes:
  iorap: Handle binder failures by gracefully attempting to reconnect
  iorap: Update AppLaunchEvent binder calls to use protobuf
parents 59e3189d 9d33ee90
Loading
Loading
Loading
Loading
+12 −1
Original line number Diff line number Diff line
@@ -9975,10 +9975,22 @@ public class Intent implements Parcelable, Cloneable {
        writeToProto(proto, fieldId, true, true, true, false);
    }

    /** @hide */
    public void writeToProto(ProtoOutputStream proto) {
        // Same input parameters that toString() gives to toShortString().
        writeToProtoWithoutFieldId(proto, true, true, true, false);
    }

    /** @hide */
    public void writeToProto(ProtoOutputStream proto, long fieldId, boolean secure, boolean comp,
            boolean extras, boolean clip) {
        long token = proto.start(fieldId);
        writeToProtoWithoutFieldId(proto, secure, comp, extras, clip);
        proto.end(token);
    }

    private void writeToProtoWithoutFieldId(ProtoOutputStream proto, boolean secure, boolean comp,
            boolean extras, boolean clip) {
        if (mAction != null) {
            proto.write(IntentProto.ACTION, mAction);
        }
@@ -10023,7 +10035,6 @@ public class Intent implements Parcelable, Cloneable {
        if (mSelector != null) {
            proto.write(IntentProto.SELECTOR, mSelector.toShortString(secure, comp, extras, clip));
        }
        proto.end(token);
    }

    /**
+31 −7
Original line number Diff line number Diff line
@@ -24,9 +24,8 @@ import android.content.Intent;
import android.content.pm.ActivityInfo;
import android.os.Parcel;
import android.os.Parcelable;
import android.util.proto.ProtoOutputStream;

// TODO: fix this. either move this class into system server or add a dependency on
// these wm classes to libiorap-java and libiorap-java-tests (somehow).
import com.android.server.wm.ActivityMetricsLaunchObserver;
import com.android.server.wm.ActivityMetricsLaunchObserver.ActivityRecordProto;
import com.android.server.wm.ActivityMetricsLaunchObserver.Temperature;
@@ -113,12 +112,12 @@ public abstract class AppLaunchEvent implements Parcelable {
        @Override
        protected void writeToParcelImpl(Parcel p, int flags) {
            super.writeToParcelImpl(p, flags);
            intent.writeToParcel(p, flags);
            IntentProtoParcelable.write(p, intent, flags);
        }

        IntentStarted(Parcel p) {
            super(p);
            intent = Intent.CREATOR.createFromParcel(p);
            intent = IntentProtoParcelable.create(p);
        }
    }

@@ -232,8 +231,7 @@ public abstract class AppLaunchEvent implements Parcelable {
    }

     public static class ActivityLaunchCancelled extends AppLaunchEvent {
        public final @Nullable
        @ActivityRecordProto byte[] activityRecordSnapshot;
        public final @Nullable @ActivityRecordProto byte[] activityRecordSnapshot;

        public ActivityLaunchCancelled(@SequenceId long sequenceId,
                @Nullable @ActivityRecordProto byte[] snapshot) {
@@ -352,7 +350,6 @@ public abstract class AppLaunchEvent implements Parcelable {
            ActivityLaunchCancelled.class,
    };

    // TODO: move to @ActivityRecordProto byte[] once we have unit tests.
    public static class ActivityRecordProtoParcelable {
        public static void write(Parcel p, @ActivityRecordProto byte[] activityRecordSnapshot,
                int flags) {
@@ -365,4 +362,31 @@ public abstract class AppLaunchEvent implements Parcelable {
            return data;
        }
    }

    public static class IntentProtoParcelable {
        private static final int INTENT_PROTO_CHUNK_SIZE = 1024;

        public static void write(Parcel p, @NonNull Intent intent, int flags) {
            // There does not appear to be a way to 'reset' a ProtoOutputBuffer stream,
            // so create a new one every time.
            final ProtoOutputStream protoOutputStream =
                    new ProtoOutputStream(INTENT_PROTO_CHUNK_SIZE);
            // Write this data out as the top-most IntentProto (i.e. it is not a sub-object).
            intent.writeToProto(protoOutputStream);
            final byte[] bytes = protoOutputStream.getBytes();

            p.writeByteArray(bytes);
        }

        // TODO: Should be mockable for testing?
        // We cannot deserialize in the platform because we don't have a 'readFromProto'
        // code.
        public static @NonNull Intent create(Parcel p) {
            // This will "read" the correct amount of data, but then we discard it.
            byte[] data = p.createByteArray();

            // Never called by real code in a platform, this binder API is implemented only in C++.
            return new Intent("<cannot deserialize IntentProto>");
        }
    }
}
+185 −9
Original line number Diff line number Diff line
@@ -24,12 +24,16 @@ import android.content.Context;
import android.content.Intent;
import android.content.pm.ActivityInfo;
import android.os.IBinder;
import android.os.IBinder.DeathRecipient;
import android.os.Handler;
import android.os.Parcel;
import android.os.RemoteException;
import android.os.ServiceManager;
import android.os.SystemProperties;
import android.util.Log;

import com.android.internal.annotations.VisibleForTesting;
import com.android.server.IoThread;
import com.android.server.LocalServices;
import com.android.server.SystemService;
import com.android.server.wm.ActivityMetricsLaunchObserver;
@@ -43,10 +47,20 @@ import com.android.server.wm.ActivityTaskManagerInternal;
 */
public class IorapForwardingService extends SystemService {

    public static final boolean DEBUG = true; // TODO: read from a getprop?
    public static final String TAG = "IorapForwardingService";
    /** $> adb shell 'setprop log.tag.IorapdForwardingService VERBOSE' */
    public static final boolean DEBUG = Log.isLoggable(TAG, Log.DEBUG);
    /** $> adb shell 'setprop iorapd.enable true' */
    private static boolean IS_ENABLED = SystemProperties.getBoolean("iorapd.enable", true);
    /** $> adb shell 'setprop iorapd.forwarding_service.wtf_crash true' */
    private static boolean WTF_CRASH = SystemProperties.getBoolean(
            "iorapd.forwarding_service.wtf_crash", false);

    private IIorap mIorapRemote;
    private final Object mLock = new Object();
    /** Handle onBinderDeath by periodically trying to reconnect. */
    private final Handler mHandler =
            new BinderConnectionHandler(IoThread.getHandler().getLooper());

    /**
     * Initializes the system service.
@@ -78,12 +92,40 @@ public class IorapForwardingService extends SystemService {

    @VisibleForTesting
    protected IIorap provideIorapRemote() {
        IIorap iorap;
        try {
            return IIorap.Stub.asInterface(ServiceManager.getServiceOrThrow("iorapd"));
            iorap = IIorap.Stub.asInterface(ServiceManager.getServiceOrThrow("iorapd"));
        } catch (ServiceManager.ServiceNotFoundException e) {
            // TODO: how do we handle service being missing?
            throw new AssertionError(e);
            handleRemoteError(e);
            return null;
        }

        try {
            iorap.asBinder().linkToDeath(provideDeathRecipient(), /*flags*/0);
        } catch (RemoteException e) {
            handleRemoteError(e);
            return null;
        }

        return iorap;
    }

    @VisibleForTesting
    protected DeathRecipient provideDeathRecipient() {
        return new DeathRecipient() {
            @Override
            public void binderDied() {
                Log.w(TAG, "iorapd has died");
                retryConnectToRemoteAndConfigure(/*attempts*/0);
            }
        };
    }

    @VisibleForTesting
    protected boolean isIorapEnabled() {
        // Same as the property in iorapd.rc -- disabling this will mean the 'iorapd' binder process
        // never comes up, so all binder connections will fail indefinitely.
        return IS_ENABLED;
    }

    //</editor-fold>
@@ -94,15 +136,128 @@ public class IorapForwardingService extends SystemService {
            Log.v(TAG, "onStart");
        }

        retryConnectToRemoteAndConfigure(/*attempts*/0);
    }

    private class BinderConnectionHandler extends Handler {
        public BinderConnectionHandler(android.os.Looper looper) {
            super(looper);
        }

        public static final int MESSAGE_BINDER_CONNECT = 0;

        private int mAttempts = 0;

        @Override
        public void handleMessage(android.os.Message message) {
           switch (message.what) {
               case MESSAGE_BINDER_CONNECT:
                   if (!retryConnectToRemoteAndConfigure(mAttempts)) {
                       mAttempts++;
                   } else {
                       mAttempts = 0;
                   }
                   break;
               default:
                   throw new AssertionError("Unknown message: " + message.toString());
           }
        }
    }

    /**
     * Handle iorapd shutdowns and crashes, by attempting to reconnect
     * until the service is reached again.
     *
     * <p>The first connection attempt is synchronous,
     * subsequent attempts are done by posting delayed tasks to the IoThread.</p>
     *
     * @return true if connection succeeded now, or false if it failed now [and needs to requeue].
     */
    private boolean retryConnectToRemoteAndConfigure(int attempts) {
        final int sleepTime = 1000;  // ms

        if (DEBUG) {
            Log.v(TAG, "retryConnectToRemoteAndConfigure - attempt #" + attempts);
        }

        if (connectToRemoteAndConfigure()) {
            return true;
        }

        // Either 'iorapd' is stuck in a crash loop (ouch!!) or we manually
        // called 'adb shell stop iorapd' , which means this would loop until it comes back
        // up.
        //
        // TODO: it would be good to get nodified of 'adb shell stop iorapd' to avoid
        // printing this warning.
        Log.w(TAG, "Failed to connect to iorapd, is it down? Delay for " + sleepTime);

        // Use a handler instead of Thread#sleep to avoid backing up the binder thread
        // when this is called from the death recipient callback.
        mHandler.sendMessageDelayed(
                mHandler.obtainMessage(BinderConnectionHandler.MESSAGE_BINDER_CONNECT),
                sleepTime);

        return false;

        // Log.e(TAG, "Can't connect to iorapd - giving up after " + attempts + " attempts");
    }

    private boolean connectToRemoteAndConfigure() {
        synchronized (mLock) {
            // Synchronize against any concurrent calls to this via the DeathRecipient.
            return connectToRemoteAndConfigureLocked();
        }
    }

    private boolean connectToRemoteAndConfigureLocked() {
        if (!isIorapEnabled()) {
            if (DEBUG) {
                Log.v(TAG, "connectToRemoteAndConfigure - iorapd is disabled, skip rest of work");
            }
            // When we see that iorapd is disabled (when system server comes up),
            // it stays disabled permanently until the next system server reset.

            // TODO: consider listening to property changes as a callback, then we can
            // be more dynamic about handling enable/disable.
            return true;
        }

        // Connect to the native binder service.
        mIorapRemote = provideIorapRemote();
        if (mIorapRemote == null) {
            Log.e(TAG, "connectToRemoteAndConfigure - null iorap remote. check for Log.wtf?");
            return false;
        }
        invokeRemote( () -> mIorapRemote.setTaskListener(new RemoteTaskListener()) );
        registerInProcessListenersLocked();

        return true;
    }

    private final AppLaunchObserver mAppLaunchObserver = new AppLaunchObserver();
    private boolean mRegisteredListeners = false;

    private void registerInProcessListenersLocked() {
        if (mRegisteredListeners) {
            // Listeners are registered only once (idempotent operation).
            //
            // Today listeners are tolerant of the remote side going away
            // by handling remote errors.
            //
            // We could try to 'unregister' the listener when we get a binder disconnect,
            // but we'd still have to handle the case of encountering synchronous errors so
            // it really wouldn't be a win (other than having less log spew).
            return;
        }

        // Listen to App Launch Sequence events from ActivityTaskManager,
        // and forward them to the native binder service.
        ActivityMetricsLaunchObserverRegistry launchObserverRegistry =
                provideLaunchObserverRegistry();
        launchObserverRegistry.registerLaunchObserver(new AppLaunchObserver());
        launchObserverRegistry.registerLaunchObserver(mAppLaunchObserver);

        mRegisteredListeners = true;
    }

    private class AppLaunchObserver implements ActivityMetricsLaunchObserver {
@@ -110,6 +265,8 @@ public class IorapForwardingService extends SystemService {
        // launch sequences on the native side.
        private @AppLaunchEvent.SequenceId long mSequenceId = -1;

        // All callbacks occur on the same background thread. Don't synchronize explicitly.

        @Override
        public void onIntentStarted(@NonNull Intent intent) {
            // #onIntentStarted [is the only transition that] initiates a new launch sequence.
@@ -174,7 +331,7 @@ public class IorapForwardingService extends SystemService {

            invokeRemote(() ->
                mIorapRemote.onAppLaunchEvent(RequestId.nextValueForSequence(),
                        new AppLaunchEvent.ActivityLaunchCancelled(mSequenceId, activity))
                        new AppLaunchEvent.ActivityLaunchFinished(mSequenceId, activity))
            );
        }
    }
@@ -201,6 +358,7 @@ public class IorapForwardingService extends SystemService {
        }
    }

    /** Allow passing lambdas to #invokeRemote */
    private interface RemoteRunnable {
        void run() throws RemoteException;
    }
@@ -209,8 +367,26 @@ public class IorapForwardingService extends SystemService {
       try {
           r.run();
       } catch (RemoteException e) {
           // TODO: what do we do with exceptions?
           throw new AssertionError("not implemented", e);
           // This could be a logic error (remote side returning error), which we need to fix.
           //
           // This could also be a DeadObjectException in which case its probably just iorapd
           // being manually restarted.
           //
           // Don't make any assumption, since DeadObjectException could also mean iorapd crashed
           // unexpectedly.
           //
           // DeadObjectExceptions are recovered from using DeathRecipient and #linkToDeath.
           handleRemoteError(e);
       }
    }

    private static void handleRemoteError(Throwable t) {
        if (WTF_CRASH) {
            // In development modes, we just want to crash.
            throw new AssertionError("unexpected remote error", t);
        } else {
            // Log to wtf which gets sent to dropbox, and in system_server this does not crash.
            Log.wtf(TAG, t);
        }
    }
}
+9 −0
Original line number Diff line number Diff line
@@ -33,6 +33,15 @@
    <target_preparer class="com.android.tradefed.targetprep.DisableSELinuxTargetPreparer">
    </target_preparer>

    <target_preparer
        class="com.android.tradefed.targetprep.DeviceSetup">
        <!-- Crash instead of using Log.wtf within the system_server iorap code. -->
        <option name="set-property" key="iorapd.forwarding_service.wtf_crash" value="true" />
        <!-- IIorapd has fake behavior: it doesn't do anything but reply with 'DONE' status -->
        <option name="set-property" key="iorapd.binder.fake" value="true" />
        <option name="restore-properties" value="true" />
    </target_preparer>

    <test class="com.android.tradefed.testtype.AndroidJUnitTest" >
        <option name="package" value="com.google.android.startop.iorap.tests" />
        <option name="runner" value="android.support.test.runner.AndroidJUnitRunner" />