diff --git a/binder/src/androidTest/java/io/grpc/binder/internal/BinderClientTransportTest.java b/binder/src/androidTest/java/io/grpc/binder/internal/BinderClientTransportTest.java index 66c3479bfc..7710924d8c 100644 --- a/binder/src/androidTest/java/io/grpc/binder/internal/BinderClientTransportTest.java +++ b/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 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 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 diff --git a/binder/src/androidTest/java/io/grpc/binder/internal/BinderTransportTest.java b/binder/src/androidTest/java/io/grpc/binder/internal/BinderTransportTest.java index 8276877bb0..84ff74b4f8 100644 --- a/binder/src/androidTest/java/io/grpc/binder/internal/BinderTransportTest.java +++ b/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()); } diff --git a/binder/src/androidTest/java/io/grpc/binder/internal/OneWayBinderProxies.java b/binder/src/androidTest/java/io/grpc/binder/internal/OneWayBinderProxies.java new file mode 100644 index 0000000000..229c942612 --- /dev/null +++ b/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 implements + OneWayBinderProxy.Decorator { + private final BlockingQueue requests = new LinkedBlockingQueue<>(); + private final BlockingQueue results = new LinkedBlockingQueue<>(); + + /** + * Returns the next {@link OneWayBinderProxy} that needs decorating, blocking if it hasn't yet + * been provided to {@link #decorate}. + * + *

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`. + * + *

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() {}; +} diff --git a/binder/src/main/java/io/grpc/binder/BinderChannelBuilder.java b/binder/src/main/java/io/grpc/binder/BinderChannelBuilder.java index 83eabf407e..2afd6ba207 100644 --- a/binder/src/main/java/io/grpc/binder/BinderChannelBuilder.java +++ b/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()); } diff --git a/binder/src/main/java/io/grpc/binder/internal/BinderServer.java b/binder/src/main/java/io/grpc/binder/internal/BinderServer.java index 8d6fe08fa9..6dd2f23165 100644 --- a/binder/src/main/java/io/grpc/binder/internal/BinderServer.java +++ b/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; } diff --git a/binder/src/main/java/io/grpc/binder/internal/BinderTransport.java b/binder/src/main/java/io/grpc/binder/internal/BinderTransport.java index b51fbac653..4a33adb215 100644 --- a/binder/src/main/java/io/grpc/binder/internal/BinderTransport.java +++ b/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> ongoingCalls; + protected final OneWayBinderProxy.Decorator binderDecorator; @GuardedBy("this") private final LinkedHashSet callIdsToNotifyWhenReady = new LinkedHashSet<>(); @@ -218,7 +219,9 @@ public abstract class BinderTransport private BinderTransport( ObjectPool 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 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 streamTracerFactories; @Nullable private ServerTransportListener serverTransportListener; + /** + * Constructs a new transport instance. + * + * @param binderDecorator used to decorate 'callbackBinder', for fault injection. + */ public BinderServerTransport( ObjectPool executorServicePool, Attributes attributes, List 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())); diff --git a/binder/src/main/java/io/grpc/binder/internal/OneWayBinderProxy.java b/binder/src/main/java/io/grpc/binder/internal/OneWayBinderProxy.java index 09b45dd993..82da825b97 100644 --- a/binder/src/main/java/io/grpc/binder/internal/OneWayBinderProxy.java +++ b/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}. + * + *

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. * diff --git a/binder/src/test/java/io/grpc/binder/internal/BinderServerTransportTest.java b/binder/src/test/java/io/grpc/binder/internal/BinderServerTransportTest.java index 76cf9b6a6a..f1e5c5a955 100644 --- a/binder/src/test/java/io/grpc/binder/internal/BinderServerTransportTest.java +++ b/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); }