binder: Add fault injection hooksunit tests for BinderClientTransport (#10928)

binder/src/androidTest/java/io/grpc/binder/internal/BinderClientTransportTest.java

@@ -19,7 +19,9 @@ package io.grpc.binder.internal;
 import static com.google.common.truth.Truth.assertThat;
 
 import android.content.Context;
+import android.os.DeadObjectException;
 import android.os.Parcel;
+import android.os.RemoteException;
 import androidx.core.content.ContextCompat;
 import androidx.test.core.app.ApplicationProvider;
 import androidx.test.ext.junit.runners.AndroidJUnit4;
@@ -41,13 +43,14 @@ import io.grpc.binder.HostServices;
 import io.grpc.binder.InboundParcelablePolicy;
 import io.grpc.binder.SecurityPolicies;
 import io.grpc.binder.SecurityPolicy;
+import io.grpc.binder.internal.OneWayBinderProxies.BlockingBinderDecorator;
+import io.grpc.binder.internal.OneWayBinderProxies.ThrowingOneWayBinderProxy;
 import io.grpc.internal.ClientStream;
 import io.grpc.internal.ClientStreamListener;
 import io.grpc.internal.FixedObjectPool;
 import io.grpc.internal.ManagedClientTransport;
 import io.grpc.internal.ObjectPool;
 import io.grpc.internal.StreamListener;
-import io.grpc.internal.StreamListener.MessageProducer;
 import io.grpc.protobuf.lite.ProtoLiteUtils;
 import io.grpc.stub.ServerCalls;
 import java.io.IOException;
@@ -140,12 +143,19 @@ public final class BinderClientTransportTest {
 
   private class BinderClientTransportBuilder {
     private SecurityPolicy securityPolicy = SecurityPolicies.internalOnly();
+    private OneWayBinderProxy.Decorator binderDecorator = OneWayBinderProxy.IDENTITY_DECORATOR;
 
     public BinderClientTransportBuilder setSecurityPolicy(SecurityPolicy securityPolicy) {
       this.securityPolicy = securityPolicy;
       return this;
     }
 
+    public BinderClientTransportBuilder setBinderDecorator(
+        OneWayBinderProxy.Decorator binderDecorator) {
+      this.binderDecorator = binderDecorator;
+      return this;
+    }
+
     public BinderTransport.BinderClientTransport build() {
       return new BinderTransport.BinderClientTransport(
           appContext,
@@ -158,6 +168,7 @@ public final class BinderClientTransportTest {
           executorServicePool,
           securityPolicy,
           InboundParcelablePolicy.DEFAULT,
+          binderDecorator,
           Attributes.EMPTY);
     }
   }
@@ -273,6 +284,62 @@ public final class BinderClientTransportTest {
     transportListener.awaitReady();
   }
 
+  @Test
+  public void testTxnFailureDuringSetup() throws InterruptedException {
+    BlockingBinderDecorator<ThrowingOneWayBinderProxy> decorator = new BlockingBinderDecorator<>();
+    transport = new BinderClientTransportBuilder()
+        .setBinderDecorator(decorator)
+        .build();
+    transport.start(transportListener).run();
+    ThrowingOneWayBinderProxy endpointBinder = new ThrowingOneWayBinderProxy(
+        decorator.takeNextRequest());
+    DeadObjectException doe = new DeadObjectException("ouch");
+    endpointBinder.setRemoteException(doe);
+    decorator.putNextResult(endpointBinder);
+
+    Status shutdownStatus = transportListener.awaitShutdown();
+    assertThat(shutdownStatus.getCode()).isEqualTo(Code.UNAVAILABLE);
+    assertThat(shutdownStatus.getCause()).isInstanceOf(RemoteException.class);
+    transportListener.awaitTermination();
+
+    ClientStream stream =
+        transport.newStream(streamingMethodDesc, new Metadata(), CallOptions.DEFAULT, tracers);
+    stream.start(streamListener);
+
+    Status streamStatus = streamListener.awaitClose();
+    assertThat(streamStatus.getCode()).isEqualTo(Code.UNAVAILABLE);
+    assertThat(streamStatus.getCause()).isSameInstanceAs(doe);
+  }
+
+  @Test
+  public void testTxnFailurePostSetup() throws InterruptedException {
+    BlockingBinderDecorator<ThrowingOneWayBinderProxy> decorator = new BlockingBinderDecorator<>();
+    transport = new BinderClientTransportBuilder()
+        .setBinderDecorator(decorator)
+        .build();
+    transport.start(transportListener).run();
+    ThrowingOneWayBinderProxy endpointBinder = new ThrowingOneWayBinderProxy(
+        decorator.takeNextRequest());
+    decorator.putNextResult(endpointBinder);
+    ThrowingOneWayBinderProxy serverBinder = new ThrowingOneWayBinderProxy(
+        decorator.takeNextRequest());
+    DeadObjectException doe = new DeadObjectException("ouch");
+    serverBinder.setRemoteException(doe);
+    decorator.putNextResult(serverBinder);
+    transportListener.awaitReady();
+
+    ClientStream stream =
+        transport.newStream(streamingMethodDesc, new Metadata(), CallOptions.DEFAULT, tracers);
+    stream.start(streamListener);
+    stream.writeMessage(marshaller.stream(Empty.getDefaultInstance()));
+    stream.halfClose();
+    stream.request(1);
+
+    Status streamStatus = streamListener.awaitClose();
+    assertThat(streamStatus.getCode()).isEqualTo(Code.UNAVAILABLE);
+    assertThat(streamStatus.getCause()).isSameInstanceAs(doe);
+  }
+
   private static void startAndAwaitReady(
       BinderTransport.BinderClientTransport transport, TestTransportListener transportListener) {
     transport.start(transportListener).run();
@@ -288,13 +355,28 @@ public final class BinderClientTransportTest {
     public boolean terminated;
 
     @Override
-    public void transportShutdown(Status shutdownStatus) {
+    public synchronized void transportShutdown(Status shutdownStatus) {
       this.shutdownStatus = shutdownStatus;
+      notifyAll();
+    }
+
+    public synchronized Status awaitShutdown() throws InterruptedException {
+      while (shutdownStatus == null) {
+        wait();
+      }
+      return shutdownStatus;
     }
 
     @Override
-    public void transportTerminated() {
+    public synchronized void transportTerminated() {
       terminated = true;
+      notifyAll();
+    }
+
+    public synchronized void awaitTermination() throws InterruptedException {
+      while (!terminated) {
+        wait();
+      }
     }
 
     @Override

binder/src/androidTest/java/io/grpc/binder/internal/BinderTransportTest.java

@@ -108,6 +108,7 @@ public final class BinderTransportTest extends AbstractTransportTest {
         offloadExecutorPool,
         SecurityPolicies.internalOnly(),
         InboundParcelablePolicy.DEFAULT,
+        OneWayBinderProxy.IDENTITY_DECORATOR,
         eagAttrs());
   }
 

binder/src/androidTest/java/io/grpc/binder/internal/OneWayBinderProxies.java

@@ -0,0 +1,100 @@
+/*
+ * Copyright 2024 The gRPC Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.grpc.binder.internal;
+
+import android.os.RemoteException;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.LinkedBlockingQueue;
+import javax.annotation.Nullable;
+
+/**
+ * A collection of {@link OneWayBinderProxy}-related test helpers.
+ */
+public final class OneWayBinderProxies {
+  /**
+   * A {@link OneWayBinderProxy.Decorator} that blocks calling threads while an (external) test
+   * provides the actual decoration.
+   */
+  public static final class BlockingBinderDecorator<T extends OneWayBinderProxy> implements
+      OneWayBinderProxy.Decorator {
+    private final BlockingQueue<OneWayBinderProxy> requests = new LinkedBlockingQueue<>();
+    private final BlockingQueue<T> results = new LinkedBlockingQueue<>();
+
+    /**
+     * Returns the next {@link OneWayBinderProxy} that needs decorating, blocking if it hasn't yet
+     * been provided to {@link #decorate}.
+     *
+     * <p>Follow this with a call to {@link #putNextResult(OneWayBinderProxy)} to provide
+     * the result of {@link #decorate} and unblock the waiting caller.
+     */
+    public OneWayBinderProxy takeNextRequest() throws InterruptedException {
+      return requests.take();
+    }
+
+    /**
+     * Provides the next value to return from {@link #decorate}.
+     */
+    public void putNextResult(T next) throws InterruptedException {
+      results.put(next);
+    }
+
+    @Override
+    public OneWayBinderProxy decorate(OneWayBinderProxy in) {
+      try {
+        requests.put(in);
+        return results.take();
+      } catch (InterruptedException e) {
+        Thread.currentThread().interrupt();
+        throw new RuntimeException(e);
+      }
+    }
+  }
+
+  /**
+   * A {@link OneWayBinderProxy} decorator whose transact method can artificially throw.
+   */
+  public static final class ThrowingOneWayBinderProxy extends OneWayBinderProxy {
+    private final OneWayBinderProxy wrapped;
+    @Nullable
+    private RemoteException remoteException;
+
+    ThrowingOneWayBinderProxy(OneWayBinderProxy wrapped) {
+      super(wrapped.getDelegate());
+      this.wrapped = wrapped;
+    }
+
+    /**
+     * Causes all future invocations of transact to throw `remoteException`.
+     *
+     * <p>Users are responsible for ensuring their calls "happen-before" the relevant calls to
+     * {@link #transact(int, ParcelHolder)}.
+     */
+    public void setRemoteException(RemoteException remoteException) {
+      this.remoteException = remoteException;
+    }
+
+    @Override
+    public void transact(int code, ParcelHolder data) throws RemoteException {
+      if (remoteException != null) {
+        throw remoteException;
+      }
+      wrapped.transact(code, data);
+    }
+  }
+
+  // Cannot be instantiated.
+  private OneWayBinderProxies() {};
+}

binder/src/main/java/io/grpc/binder/BinderChannelBuilder.java

@@ -30,6 +30,7 @@ import io.grpc.ForwardingChannelBuilder;
 import io.grpc.ManagedChannel;
 import io.grpc.ManagedChannelBuilder;
 import io.grpc.binder.internal.BinderTransport;
+import io.grpc.binder.internal.OneWayBinderProxy;
 import io.grpc.internal.ClientTransportFactory;
 import io.grpc.internal.ConnectionClientTransport;
 import io.grpc.internal.FixedObjectPool;
@@ -384,6 +385,7 @@ public final class BinderChannelBuilder
           offloadExecutorPool,
           securityPolicy,
           inboundParcelablePolicy,
+          OneWayBinderProxy.IDENTITY_DECORATOR,
           options.getEagAttributes());
     }
 

binder/src/main/java/io/grpc/binder/internal/BinderServer.java

@@ -162,7 +162,9 @@ public final class BinderServer implements InternalServer, LeakSafeOneWayBinder.
           // Create a new transport and let our listener know about it.
           BinderTransport.BinderServerTransport transport =
               new BinderTransport.BinderServerTransport(
-                  executorServicePool, attrsBuilder.build(), streamTracerFactories, callbackBinder);
+                  executorServicePool, attrsBuilder.build(), streamTracerFactories,
+                  OneWayBinderProxy.IDENTITY_DECORATOR,
+                  callbackBinder);
           transport.setServerTransportListener(listener.transportCreated(transport));
           return true;
         }

binder/src/main/java/io/grpc/binder/internal/BinderTransport.java

@@ -191,6 +191,7 @@ public abstract class BinderTransport
   private final LeakSafeOneWayBinder incomingBinder;
 
   protected final ConcurrentHashMap<Integer, Inbound<?>> ongoingCalls;
+  protected final OneWayBinderProxy.Decorator binderDecorator;
 
   @GuardedBy("this")
   private final LinkedHashSet<Integer> callIdsToNotifyWhenReady = new LinkedHashSet<>();
@@ -218,7 +219,9 @@ public abstract class BinderTransport
   private BinderTransport(
       ObjectPool<ScheduledExecutorService> executorServicePool,
       Attributes attributes,
+      OneWayBinderProxy.Decorator binderDecorator,
       InternalLogId logId) {
+    this.binderDecorator = binderDecorator;
     this.executorServicePool = executorServicePool;
     this.attributes = attributes;
     this.logId = logId;
@@ -283,6 +286,7 @@ public abstract class BinderTransport
 
   @GuardedBy("this")
   protected boolean setOutgoingBinder(OneWayBinderProxy binder) {
+    binder = binderDecorator.decorate(binder);
     this.outgoingBinder = binder;
     try {
       binder.getDelegate().linkToDeath(this, 0);
@@ -566,6 +570,12 @@ public abstract class BinderTransport
     @GuardedBy("this")
     private int latestCallId = FIRST_CALL_ID;
 
+    /**
+     * Constructs a new transport instance.
+     *
+     * @param binderDecorator used to decorate both the "endpoint" and "server" binders, for fault
+     *     injection.
+     */
     public BinderClientTransport(
         Context sourceContext,
         BinderChannelCredentials channelCredentials,
@@ -577,10 +587,12 @@ public abstract class BinderTransport
         ObjectPool<? extends Executor> offloadExecutorPool,
         SecurityPolicy securityPolicy,
         InboundParcelablePolicy inboundParcelablePolicy,
+        OneWayBinderProxy.Decorator binderDecorator,
         Attributes eagAttrs) {
       super(
           executorServicePool,
           buildClientAttributes(eagAttrs, sourceContext, targetAddress, inboundParcelablePolicy),
+          binderDecorator,
           buildLogId(sourceContext, targetAddress));
       this.offloadExecutorPool = offloadExecutorPool;
       this.securityPolicy = securityPolicy;
@@ -607,7 +619,7 @@ public abstract class BinderTransport
 
     @Override
     public synchronized void onBound(IBinder binder) {
-      sendSetupTransaction(OneWayBinderProxy.wrap(binder, offloadExecutor));
+      sendSetupTransaction(binderDecorator.decorate(OneWayBinderProxy.wrap(binder, offloadExecutor)));
     }
 
     @Override
@@ -819,12 +831,18 @@ public abstract class BinderTransport
     private final List<ServerStreamTracer.Factory> streamTracerFactories;
     @Nullable private ServerTransportListener serverTransportListener;
 
+    /**
+     * Constructs a new transport instance.
+     *
+     * @param binderDecorator used to decorate 'callbackBinder', for fault injection.
+     */
     public BinderServerTransport(
         ObjectPool<ScheduledExecutorService> executorServicePool,
         Attributes attributes,
         List<ServerStreamTracer.Factory> streamTracerFactories,
+        OneWayBinderProxy.Decorator binderDecorator,
         IBinder callbackBinder) {
-      super(executorServicePool, attributes, buildLogId(attributes));
+      super(executorServicePool, attributes, binderDecorator, buildLogId(attributes));
       this.streamTracerFactories = streamTracerFactories;
       // TODO(jdcormie): Plumb in the Server's executor() and use it here instead.
       setOutgoingBinder(OneWayBinderProxy.wrap(callbackBinder, getScheduledExecutorService()));

binder/src/main/java/io/grpc/binder/internal/OneWayBinderProxy.java

@@ -46,7 +46,7 @@ public abstract class OneWayBinderProxy {
   private static final Logger logger = Logger.getLogger(OneWayBinderProxy.class.getName());
   protected final IBinder delegate;
 
-  private OneWayBinderProxy(IBinder iBinder) {
+  protected OneWayBinderProxy(IBinder iBinder) {
     this.delegate = iBinder;
   }
 
@@ -64,6 +64,24 @@ public abstract class OneWayBinderProxy {
         : new OutOfProcessImpl(iBinder);
   }
 
+  /**
+   * An abstract function that decorates instances of {@link OneWayBinderProxy}.
+   *
+   * <p>See https://en.wikipedia.org/wiki/Decorator_pattern.
+   */
+  public interface Decorator {
+    /**
+     * Returns an instance of {@link OneWayBinderProxy} that decorates {@code input} with some
+     * new behavior.
+     */
+    OneWayBinderProxy decorate(OneWayBinderProxy input);
+  }
+
+  /**
+   * A {@link Decorator} that does nothing.
+   */
+  public static final Decorator IDENTITY_DECORATOR = (x) -> x;
+
   /**
    * Enqueues a transaction for the wrapped {@link IBinder} with guaranteed "oneway" semantics.
    *

binder/src/test/java/io/grpc/binder/internal/BinderServerTransportTest.java

@@ -71,6 +71,7 @@ public final class BinderServerTransportTest {
             new FixedObjectPool<>(executorService),
             Attributes.EMPTY,
             ImmutableList.of(),
+            OneWayBinderProxy.IDENTITY_DECORATOR,
             mockBinder);
   }