diff --git a/binder/src/main/java/io/grpc/binder/internal/BinderClientTransport.java b/binder/src/main/java/io/grpc/binder/internal/BinderClientTransport.java index 144ad56eec3..c7d954db1cc 100644 --- a/binder/src/main/java/io/grpc/binder/internal/BinderClientTransport.java +++ b/binder/src/main/java/io/grpc/binder/internal/BinderClientTransport.java @@ -25,6 +25,8 @@ import android.os.IBinder; import android.os.Parcel; import android.os.Process; +import androidx.annotation.BinderThread; +import androidx.annotation.MainThread; import com.google.common.base.Ticker; import com.google.common.util.concurrent.FutureCallback; import com.google.common.util.concurrent.Futures; @@ -72,6 +74,9 @@ public final class BinderClientTransport extends BinderTransport private final SecurityPolicy securityPolicy; private final Bindable serviceBinding; + @GuardedBy("this") + private final ClientHandshake handshake; + /** Number of ongoing calls which keep this transport "in-use". */ private final AtomicInteger numInUseStreams; @@ -122,6 +127,7 @@ public BinderClientTransport( Boolean preAuthServerOverride = options.getEagAttributes().get(PRE_AUTH_SERVER_OVERRIDE); this.preAuthorizeServer = preAuthServerOverride != null ? preAuthServerOverride : factory.preAuthorizeServers; + this.handshake = new LegacyClientHandshake(); numInUseStreams = new AtomicInteger(); pingTracker = new PingTracker(Ticker.systemTicker(), (id) -> sendPing(id)); @@ -146,7 +152,9 @@ void releaseExecutors() { @Override public synchronized void onBound(IBinder binder) { - sendSetupTransaction(binderDecorator.decorate(OneWayBinderProxy.wrap(binder, offloadExecutor))); + OneWayBinderProxy binderProxy = OneWayBinderProxy.wrap(binder, offloadExecutor); + binderProxy = binderDecorator.decorate(binderProxy); + handshake.onBound(binderProxy); } @Override @@ -329,7 +337,6 @@ void notifyTerminated() { @Override @GuardedBy("this") protected void handleSetupTransport(Parcel parcel) { - int remoteUid = Binder.getCallingUid(); if (inState(TransportState.SETUP)) { int version = parcel.readInt(); IBinder binder = parcel.readStrongBinder(); @@ -339,23 +346,8 @@ protected void handleSetupTransport(Parcel parcel) { shutdownInternal( Status.UNAVAILABLE.withDescription("Malformed SETUP_TRANSPORT data"), true); } else { - restrictIncomingBinderToCallsFrom(remoteUid); - attributes = setSecurityAttrs(attributes, remoteUid); - authResultFuture = checkServerAuthorizationAsync(remoteUid); - Futures.addCallback( - authResultFuture, - new FutureCallback() { - @Override - public void onSuccess(Status result) { - handleAuthResult(binder, result); - } - - @Override - public void onFailure(Throwable t) { - handleAuthResult(t); - } - }, - offloadExecutor); + OneWayBinderProxy binderProxy = OneWayBinderProxy.wrap(binder, offloadExecutor); + handshake.handleSetupTransport(binderProxy); } } } @@ -366,29 +358,70 @@ private ListenableFuture checkServerAuthorizationAsync(int remoteUid) { : Futures.submit(() -> securityPolicy.checkAuthorization(remoteUid), offloadExecutor); } - private synchronized void handleAuthResult(IBinder binder, Status authorization) { - if (inState(TransportState.SETUP)) { - if (!authorization.isOk()) { - shutdownInternal(authorization, true); - } else if (!setOutgoingBinder(OneWayBinderProxy.wrap(binder, offloadExecutor))) { - shutdownInternal( - Status.UNAVAILABLE.withDescription("Failed to observe outgoing binder"), true); - } else { - // Check state again, since a failure inside setOutgoingBinder (or a callback it - // triggers), could have shut us down. - if (!isShutdown()) { - setState(TransportState.READY); - attributes = clientTransportListener.filterTransport(attributes); - clientTransportListener.transportReady(); - if (readyTimeoutFuture != null) { - readyTimeoutFuture.cancel(false); - readyTimeoutFuture = null; + private final class LegacyClientHandshake implements ClientHandshake { + @Override + @MainThread + @GuardedBy("BinderClientTransport.this") // By way of @GuardedBy("this") `handshake` member. + public void onBound(OneWayBinderProxy binder) { + sendSetupTransaction(binder); + } + + @Override + @BinderThread + @GuardedBy("BinderClientTransport.this") // By way of @GuardedBy("this") `handshake` member. + public void handleSetupTransport(OneWayBinderProxy binder) { + int remoteUid = Binder.getCallingUid(); + restrictIncomingBinderToCallsFrom(remoteUid); + attributes = setSecurityAttrs(attributes, remoteUid); + authResultFuture = checkServerAuthorizationAsync(remoteUid); + Futures.addCallback( + authResultFuture, + new FutureCallback() { + @Override + public void onSuccess(Status result) { + synchronized (BinderClientTransport.this) { + handleAuthResult(binder, result); + } + } + + @Override + public void onFailure(Throwable t) { + BinderClientTransport.this.handleAuthResult(t); + } + }, + offloadExecutor); + } + + @GuardedBy("BinderClientTransport.this") + private void handleAuthResult(OneWayBinderProxy binder, Status authorization) { + if (inState(TransportState.SETUP)) { + if (!authorization.isOk()) { + shutdownInternal(authorization, true); + } else if (!setOutgoingBinder(binder)) { + shutdownInternal( + Status.UNAVAILABLE.withDescription("Failed to observe outgoing binder"), true); + } else { + // Check state again, since a failure inside setOutgoingBinder (or a callback it + // triggers), could have shut us down. + if (!isShutdown()) { + onHandshakeComplete(); } } } } } + @GuardedBy("this") + private void onHandshakeComplete() { + setState(TransportState.READY); + attributes = clientTransportListener.filterTransport(attributes); + clientTransportListener.transportReady(); + if (readyTimeoutFuture != null) { + readyTimeoutFuture.cancel(false); + readyTimeoutFuture = null; + } + } + private synchronized void handleAuthResult(Throwable t) { shutdownInternal( Status.INTERNAL.withDescription("Could not evaluate SecurityPolicy").withCause(t), true); @@ -400,6 +433,27 @@ protected void handlePingResponse(Parcel parcel) { pingTracker.onPingResponse(parcel.readInt()); } + /** + * A base for all implementations of the client handshake. + * + *

Supports a clean migration away from the legacy approach, one client at a time. + */ + private interface ClientHandshake { + /** + * Notifies the implementation that the binding has succeeded and we are now connected to the + * server's "endpoint" which can be reached at 'endpointBinder'. + */ + @MainThread + void onBound(OneWayBinderProxy endpointBinder); + + /** + * Notifies the implementation that we've received a valid SETUP_TRANSPORT transaction from a + * server that can be reached at 'serverBinder'. + */ + @BinderThread + void handleSetupTransport(OneWayBinderProxy serverBinder); + } + private static ClientStream newFailingClientStream( Status failure, Attributes attributes, Metadata headers, ClientStreamTracer[] tracers) { StatsTraceContext statsTraceContext =