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 b1ae344d5f9..96685a2f8bd 100644 --- a/binder/src/main/java/io/grpc/binder/internal/BinderServer.java +++ b/binder/src/main/java/io/grpc/binder/internal/BinderServer.java @@ -179,7 +179,7 @@ public synchronized boolean handleTransaction(int code, Parcel parcel) { checkNotNull(executor, "Not started?")); // Create a new transport and let our listener know about it. BinderServerTransport transport = - new BinderServerTransport( + BinderServerTransport.create( executorServicePool, attrsBuilder.build(), streamTracerFactories, diff --git a/binder/src/main/java/io/grpc/binder/internal/BinderServerTransport.java b/binder/src/main/java/io/grpc/binder/internal/BinderServerTransport.java index 44909f9c0bc..a57f89e72ac 100644 --- a/binder/src/main/java/io/grpc/binder/internal/BinderServerTransport.java +++ b/binder/src/main/java/io/grpc/binder/internal/BinderServerTransport.java @@ -42,21 +42,35 @@ public final class BinderServerTransport extends BinderTransport implements Serv @GuardedBy("this") private final SimplePromise listenerPromise = new SimplePromise<>(); + private BinderServerTransport( + ObjectPool executorServicePool, + Attributes attributes, + List streamTracerFactories, + OneWayBinderProxy.Decorator binderDecorator) { + super(executorServicePool, attributes, binderDecorator, buildLogId(attributes)); + this.streamTracerFactories = streamTracerFactories; + } + /** * Constructs a new transport instance. * * @param binderDecorator used to decorate 'callbackBinder', for fault injection. */ - public BinderServerTransport( + public static BinderServerTransport create( ObjectPool executorServicePool, Attributes attributes, List streamTracerFactories, OneWayBinderProxy.Decorator binderDecorator, IBinder callbackBinder) { - super(executorServicePool, attributes, binderDecorator, buildLogId(attributes)); - this.streamTracerFactories = streamTracerFactories; + BinderServerTransport transport = + new BinderServerTransport( + executorServicePool, attributes, streamTracerFactories, binderDecorator); // TODO(jdcormie): Plumb in the Server's executor() and use it here instead. - setOutgoingBinder(OneWayBinderProxy.wrap(callbackBinder, getScheduledExecutorService())); + // No need to handle failure here because if 'callbackBinder' is already dead, we'll notice it + // again in start() when we send the first transaction. + transport.setOutgoingBinder( + OneWayBinderProxy.wrap(callbackBinder, transport.getScheduledExecutorService())); + return transport; } /** 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 a5b8d94ba0e..1592f6977df 100644 --- a/binder/src/main/java/io/grpc/binder/internal/BinderTransport.java +++ b/binder/src/main/java/io/grpc/binder/internal/BinderTransport.java @@ -159,6 +159,7 @@ protected enum TransportState { private final ObjectPool executorServicePool; private final ScheduledExecutorService scheduledExecutorService; private final InternalLogId logId; + @GuardedBy("this") private final LeakSafeOneWayBinder incomingBinder; @@ -277,6 +278,14 @@ final void setState(TransportState newState) { transportState = newState; } + /** + * Sets the binder to use for sending subsequent transactions to our peer. + * + *

Subclasses should call this as early as possible but not from a constructor. + * + *

Returns true for success, false if the process hosting 'binder' is already dead. Callers are + * responsible for handling this. + */ @GuardedBy("this") protected boolean setOutgoingBinder(OneWayBinderProxy binder) { binder = binderDecorator.decorate(binder); 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 12416922fc9..d261ce43c8c 100644 --- a/binder/src/test/java/io/grpc/binder/internal/BinderServerTransportTest.java +++ b/binder/src/test/java/io/grpc/binder/internal/BinderServerTransportTest.java @@ -21,10 +21,8 @@ import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.doThrow; -import static org.mockito.Mockito.when; import static org.robolectric.Shadows.shadowOf; -import android.os.DeadObjectException; import android.os.IBinder; import android.os.Looper; import android.os.Parcel; @@ -96,6 +94,17 @@ public void testSetupTransactionFailureReportsMultipleTerminations_b153460678() assertThat(transportListener.isTerminated()).isTrue(); } + @Test + public void testClientBinderIsDeadOnArrival() throws Exception { + transport = newBinderServerTransportBuilder() + .setCallbackBinder(new FakeDeadBinder()) + .build(); + transport.start(transportListener); + shadowOf(Looper.getMainLooper()).idle(); + + assertThat(transportListener.isTerminated()).isTrue(); + } + @Test public void testStartAfterShutdownAndIdle() throws Exception { transport = newBinderServerTransportBuilder().build(); @@ -125,7 +134,7 @@ static class BinderServerTransportBuilder { IBinder callbackBinder; public BinderServerTransport build() { - return new BinderServerTransport( + return BinderServerTransport.create( executorServicePool, attributes, streamTracerFactories, binderDecorator, callbackBinder); }