Skip to content

Commit

Permalink
core: fix NPE in ConfigSelectingClientCall
Browse files Browse the repository at this point in the history
Fix the following bug:

ManagedChannelImpl.ConfigSelectingClientCall may return early in start() leaving delegate null, and fails request() method after start().

Currently the bug can only be triggered when using xDS.
  • Loading branch information
dapengzhang0 committed Apr 15, 2021
1 parent d4fa0ec commit d25ebaf
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 0 deletions.
25 changes: 25 additions & 0 deletions core/src/main/java/io/grpc/internal/ManagedChannelImpl.java
Expand Up @@ -1179,13 +1179,15 @@ protected ClientCall<ReqT, RespT> delegate() {
return delegate;
}

@SuppressWarnings("unchecked")
@Override
public void start(Listener<RespT> observer, Metadata headers) {
PickSubchannelArgs args = new PickSubchannelArgsImpl(method, headers, callOptions);
InternalConfigSelector.Result result = configSelector.selectConfig(args);
Status status = result.getStatus();
if (!status.isOk()) {
executeCloseObserverInContext(observer, status);
delegate = (ClientCall<ReqT, RespT>) NOOP_CALL;
return;
}
ClientInterceptor interceptor = result.getInterceptor();
Expand Down Expand Up @@ -1226,6 +1228,29 @@ public void cancel(@Nullable String message, @Nullable Throwable cause) {
}
}

private static final ClientCall<Object, Object> NOOP_CALL = new ClientCall<Object, Object>() {
@Override
public void start(Listener<Object> responseListener, Metadata headers) {}

@Override
public void request(int numMessages) {}

@Override
public void cancel(String message, Throwable cause) {}

@Override
public void halfClose() {}

@Override
public void sendMessage(Object message) {}

// Always returns {@code false}, since this is only used when the startup of the call fails.
@Override
public boolean isReady() {
return false;
}
};

/**
* Terminate the channel if termination conditions are met.
*/
Expand Down
Expand Up @@ -135,6 +135,9 @@ public Result selectConfig(PickSubchannelArgs args) {
ArgumentCaptor<Status> statusCaptor = ArgumentCaptor.forClass(null);
verify(callListener).onClose(statusCaptor.capture(), any(Metadata.class));
assertThat(statusCaptor.getValue().getCode()).isEqualTo(Status.Code.FAILED_PRECONDITION);

// The call should not delegate to null and fail methods with NPE.
configSelectingClientCall.request(1);
}

private final class TestChannel extends Channel {
Expand Down

0 comments on commit d25ebaf

Please sign in to comment.