Skip to content

Commit 67c2c93

Browse files
committed
Don't set the ready timeout if we fail to resolve.
1 parent df0b824 commit 67c2c93

2 files changed

Lines changed: 33 additions & 38 deletions

File tree

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

Lines changed: 32 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -648,38 +648,33 @@ public synchronized void onUnbound(Status reason) {
648648
@Override
649649
public synchronized Runnable start(ManagedClientTransport.Listener clientTransportListener) {
650650
this.clientTransportListener = checkNotNull(clientTransportListener);
651-
return () -> {
652-
synchronized (BinderClientTransport.this) {
653-
if (inState(TransportState.NOT_STARTED)) {
654-
setState(TransportState.SETUP);
655-
if (preAuthorizeServer) {
656-
preAuthorizeServer();
657-
} else {
658-
serviceBinding.bind();
659-
}
660-
if (readyTimeoutMillis >= 0) {
661-
readyTimeoutFuture =
662-
getScheduledExecutorService()
663-
.schedule(
664-
BinderClientTransport.this::onReadyTimeout,
665-
readyTimeoutMillis,
666-
MILLISECONDS);
667-
}
651+
return this::startInternal;
652+
}
653+
654+
private synchronized void startInternal() {
655+
if (inState(TransportState.NOT_STARTED)) {
656+
setState(TransportState.SETUP);
657+
try {
658+
if (preAuthorizeServer) {
659+
preAuthorize(serviceBinding.resolve());
660+
} else {
661+
serviceBinding.bind();
668662
}
663+
} catch (StatusException e) {
664+
shutdownInternal(e.getStatus(), true);
665+
return;
669666
}
670-
};
667+
if (readyTimeoutMillis >= 0) {
668+
readyTimeoutFuture =
669+
getScheduledExecutorService()
670+
.schedule(
671+
BinderClientTransport.this::onReadyTimeout, readyTimeoutMillis, MILLISECONDS);
672+
}
673+
}
671674
}
672675

673676
@GuardedBy("this")
674-
private void preAuthorizeServer() {
675-
ServiceInfo serviceInfo;
676-
try {
677-
serviceInfo = serviceBinding.resolve();
678-
} catch (StatusException e) {
679-
shutdownInternal(e.getStatus(), true);
680-
return;
681-
}
682-
677+
private void preAuthorize(ServiceInfo serviceInfo) {
683678
// It's unlikely, but the identity/existence of this Service could change by the time we
684679
// actually connect. It doesn't matter though, because:
685680
// - If pre-auth fails (but would succeed against the server's new state), the grpc-core layer
@@ -705,6 +700,16 @@ public void onFailure(Throwable t) {
705700
offloadExecutor);
706701
}
707702

703+
private synchronized void handlePreAuthResult(Status authorization) {
704+
if (inState(TransportState.SETUP)) {
705+
if (!authorization.isOk()) {
706+
shutdownInternal(authorization, true);
707+
} else {
708+
serviceBinding.bind();
709+
}
710+
}
711+
}
712+
708713
private synchronized void onReadyTimeout() {
709714
if (inState(TransportState.SETUP)) {
710715
readyTimeoutFuture = null;
@@ -846,16 +851,6 @@ private ListenableFuture<Status> checkServerAuthorizationAsync(int remoteUid) {
846851
: Futures.submit(() -> securityPolicy.checkAuthorization(remoteUid), offloadExecutor);
847852
}
848853

849-
private synchronized void handlePreAuthResult(Status authorization) {
850-
if (inState(TransportState.SETUP)) {
851-
if (!authorization.isOk()) {
852-
shutdownInternal(authorization, true);
853-
} else {
854-
serviceBinding.bind();
855-
}
856-
}
857-
}
858-
859854
private synchronized void handleAuthResult(IBinder binder, Status authorization) {
860855
if (inState(TransportState.SETUP)) {
861856
if (!authorization.isOk()) {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ public void clientAuthorizesServerUidsInOrder() throws Exception {
205205
// test never exercises server SecurityPolicy.
206206
ShadowBinder.setCallingUid(11111); // UID of the server *process*.
207207

208-
serverPkgInfo.applicationInfo.uid = 22222; // UID of the server *app*, which can be different.
208+
serverPkgInfo.applicationInfo.uid = 22222; // UID of the server *app*, which can be different.
209209
shadowOf(application.getPackageManager()).installPackage(serverPkgInfo);
210210
shadowOf(application.getPackageManager()).addOrUpdateService(serviceInfo);
211211
server = newServer(Collections.emptyList());

0 commit comments

Comments
 (0)