Skip to content

Commit

Permalink
binder: Add fault injection hooksunit tests for BinderClientTransport (
Browse files Browse the repository at this point in the history
  • Loading branch information
jdcormie committed Feb 27, 2024
1 parent af117e9 commit a654d2e
Show file tree
Hide file tree
Showing 8 changed files with 231 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@
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;
Expand All @@ -41,13 +43,14 @@
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;
Expand Down Expand Up @@ -140,12 +143,19 @@ public void setUp() throws Exception {

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,
Expand All @@ -158,6 +168,7 @@ public BinderTransport.BinderClientTransport build() {
executorServicePool,
securityPolicy,
InboundParcelablePolicy.DEFAULT,
binderDecorator,
Attributes.EMPTY);
}
}
Expand Down Expand Up @@ -273,6 +284,62 @@ public void testNewStreamBeforeTransportReadyFails() throws InterruptedException
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();
Expand All @@ -288,13 +355,28 @@ private static final class TestTransportListener implements ManagedClientTranspo
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ protected ManagedClientTransport newClientTransport(InternalServer server) {
offloadExecutorPool,
SecurityPolicies.internalOnly(),
InboundParcelablePolicy.DEFAULT,
OneWayBinderProxy.IDENTITY_DECORATOR,
eagAttrs());
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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() {};
}
2 changes: 2 additions & 0 deletions binder/src/main/java/io/grpc/binder/BinderChannelBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
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;
Expand Down Expand Up @@ -384,6 +385,7 @@ public ConnectionClientTransport newClientTransport(
offloadExecutorPool,
securityPolicy,
inboundParcelablePolicy,
OneWayBinderProxy.IDENTITY_DECORATOR,
options.getEagAttributes());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,9 @@ public synchronized boolean handleTransaction(int code, Parcel parcel) {
// 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;
}
Expand Down
22 changes: 20 additions & 2 deletions binder/src/main/java/io/grpc/binder/internal/BinderTransport.java
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ protected enum TransportState {
private final LeakSafeOneWayBinder incomingBinder;

protected final ConcurrentHashMap<Integer, Inbound<?>> ongoingCalls;
protected final OneWayBinderProxy.Decorator binderDecorator;

@GuardedBy("this")
private final LinkedHashSet<Integer> callIdsToNotifyWhenReady = new LinkedHashSet<>();
Expand Down Expand Up @@ -218,7 +219,9 @@ protected enum TransportState {
private BinderTransport(
ObjectPool<ScheduledExecutorService> executorServicePool,
Attributes attributes,
OneWayBinderProxy.Decorator binderDecorator,
InternalLogId logId) {
this.binderDecorator = binderDecorator;
this.executorServicePool = executorServicePool;
this.attributes = attributes;
this.logId = logId;
Expand Down Expand Up @@ -283,6 +286,7 @@ final void setState(TransportState newState) {

@GuardedBy("this")
protected boolean setOutgoingBinder(OneWayBinderProxy binder) {
binder = binderDecorator.decorate(binder);
this.outgoingBinder = binder;
try {
binder.getDelegate().linkToDeath(this, 0);
Expand Down Expand Up @@ -566,6 +570,12 @@ public static final class BinderClientTransport extends 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,
Expand All @@ -577,10 +587,12 @@ public BinderClientTransport(
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;
Expand All @@ -607,7 +619,7 @@ void releaseExecutors() {

@Override
public synchronized void onBound(IBinder binder) {
sendSetupTransaction(OneWayBinderProxy.wrap(binder, offloadExecutor));
sendSetupTransaction(binderDecorator.decorate(OneWayBinderProxy.wrap(binder, offloadExecutor)));
}

@Override
Expand Down Expand Up @@ -819,12 +831,18 @@ public static final class BinderServerTransport extends BinderTransport implemen
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()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -64,6 +64,24 @@ public static OneWayBinderProxy wrap(IBinder iBinder, Executor inProcessThreadHo
: 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.
*
Expand Down

0 comments on commit a654d2e

Please sign in to comment.