#19625

Committed 07 Jan 2025 06:25PM UTC coverage: 88.546% (+0.002%) from 88.544%

Build # #19625

Build Type

push

github

Committed by

ejona86

Commit Message

netty: Fix getAttributes() data races in NettyClientTransportTest

Since approximately the LBv2 API (the current API) was introduced, gRPC
won't use a transport until it is ready. Long ago, transports could be
used before they were ready and these old tests were not waiting for the
negotiator to complete before starting. We need them to wait for the
handshake to complete to avoid a test-only data race in getAttributes()
noticed by TSAN.

Throwing away data frames in the Noop handshaker is necessary to act
like a normal handshaker; they don't allow data frames to pass until the
handshake is complete. Without the handling, it goes through invalid
code paths in NettyClientHandler where a terminated transport becomes
ready, and a similar data race.

```
  Write of size 4 at 0x00008db31e2c by thread T37:
    #0 io.grpc.netty.NettyClientHandler.handleProtocolNegotiationCompleted(Lio/grpc/Attributes;Lio/grpc/InternalChannelz$Security;)V NettyClientHandler.java:517
    #1 io.grpc.netty.ProtocolNegotiators$GrpcNegotiationHandler.userEventTriggered(Lio/netty/channel/ChannelHandlerContext;Ljava/lang/Object;)V ProtocolNegotiators.java:937
    #2 io.netty.channel.AbstractChannelHandlerContext.invokeUserEventTriggered(Ljava/lang/Object;)V AbstractChannelHandlerContext.java:398
    #3 io.netty.channel.AbstractChannelHandlerContext.invokeUserEventTriggered(Lio/netty/channel/AbstractChannelHandlerContext;Ljava/lang/Object;)V AbstractChannelHandlerContext.java:376
    #4 io.netty.channel.AbstractChannelHandlerContext.fireUserEventTriggered(Ljava/lang/Object;)Lio/netty/channel/ChannelHandlerContext; AbstractChannelHandlerContext.java:368
    #5 io.grpc.netty.ProtocolNegotiators$ProtocolNegotiationHandler.fireProtocolNegotiationEvent(Lio/netty/channel/ChannelHandlerContext;)V ProtocolNegotiators.java:1107
    #6 io.grpc.netty.ProtocolNegotiators$WaitUntilActiveHandler.channelActive(Lio/netty/channel/ChannelHandlerContext;)V ProtocolNegotiators.java:1011
    ...

  Previo... (continued)

Run Details

33676 of 38032 relevant lines covered (88.55%)

0.89 hits per line

Source File
Press 'n' to go to next uncovered line, 'b' for previous

47.62

/../core/src/main/java/io/grpc/internal/PickSubchannelArgsImpl.java

/*
 * Copyright 2016 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.internal;

import static com.google.common.base.Preconditions.checkNotNull;

import com.google.common.base.Objects;
import io.grpc.CallOptions;
import io.grpc.LoadBalancer.PickDetailsConsumer;
import io.grpc.LoadBalancer.PickSubchannelArgs;
import io.grpc.Metadata;
import io.grpc.MethodDescriptor;

/** Implementation of {@link PickSubchannelArgs}. */
public final class PickSubchannelArgsImpl extends PickSubchannelArgs {
  private final CallOptions callOptions;
  private final Metadata headers;
  private final MethodDescriptor<?, ?> method;
  private final PickDetailsConsumer pickDetailsConsumer;

  /**
   * Creates call args object for given method with its call options, metadata.
   */
  public PickSubchannelArgsImpl(
      MethodDescriptor<?, ?> method, Metadata headers, CallOptions callOptions,
      PickDetailsConsumer pickDetailsConsumer) {
    this.method = checkNotNull(method, "method");
    this.headers = checkNotNull(headers, "headers");
    this.callOptions = checkNotNull(callOptions, "callOptions");
    this.pickDetailsConsumer = checkNotNull(pickDetailsConsumer, "pickDetailsConsumer");
  }

  @Override
  public Metadata getHeaders() {
    return headers;
  }

  @Override
  public CallOptions getCallOptions() {
    return callOptions;
  }

  @Override
  public MethodDescriptor<?, ?> getMethodDescriptor() {
    return method;
  }

  @Override
  public PickDetailsConsumer getPickDetailsConsumer() {
    return pickDetailsConsumer;
  }

  @Override
  public boolean equals(Object o) {
    if (this == o) {
      return true;
    }
    if (o == null || getClass() != o.getClass()) {
      return false;
    }
    PickSubchannelArgsImpl that = (PickSubchannelArgsImpl) o;
    return Objects.equal(callOptions, that.callOptions)
        && Objects.equal(headers, that.headers)
        && Objects.equal(method, that.method)
        && Objects.equal(pickDetailsConsumer, that.pickDetailsConsumer);
  }

  @Override
  public int hashCode() {
    return Objects.hashCode(callOptions, headers, method, pickDetailsConsumer);
  }

  @Override
  public final String toString() {
    return "[method=" + method + " headers=" + headers + " callOptions=" + callOptions + "]";
  }
}

1	/*
2	* Copyright 2016 The gRPC Authors
3	*
4	* Licensed under the Apache License, Version 2.0 (the "License");
5	* you may not use this file except in compliance with the License.
6	* You may obtain a copy of the License at
7	*
8	* http://www.apache.org/licenses/LICENSE-2.0
9	*
10	* Unless required by applicable law or agreed to in writing, software
11	* distributed under the License is distributed on an "AS IS" BASIS,
12	* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13	* See the License for the specific language governing permissions and
14	* limitations under the License.
15	*/
16
17	package io.grpc.internal;
18
19	import static com.google.common.base.Preconditions.checkNotNull;
20
21	import com.google.common.base.Objects;
22	import io.grpc.CallOptions;
23	import io.grpc.LoadBalancer.PickDetailsConsumer;
24	import io.grpc.LoadBalancer.PickSubchannelArgs;
25	import io.grpc.Metadata;
26	import io.grpc.MethodDescriptor;
27
28	/** Implementation of {@link PickSubchannelArgs}. */
29	public final class PickSubchannelArgsImpl extends PickSubchannelArgs {
30	private final CallOptions callOptions;
31	private final Metadata headers;
32	private final MethodDescriptor<?, ?> method;
33	private final PickDetailsConsumer pickDetailsConsumer;
34
35	/**
36	* Creates call args object for given method with its call options, metadata.
37	*/
38	public PickSubchannelArgsImpl(
39	MethodDescriptor<?, ?> method, Metadata headers, CallOptions callOptions,
40	PickDetailsConsumer pickDetailsConsumer) {	1✔
41	this.method = checkNotNull(method, "method");	1✔
42	this.headers = checkNotNull(headers, "headers");	1✔
43	this.callOptions = checkNotNull(callOptions, "callOptions");	1✔
44	this.pickDetailsConsumer = checkNotNull(pickDetailsConsumer, "pickDetailsConsumer");	1✔
45	}	1✔
46
47	@Override
48	public Metadata getHeaders() {
49	return headers;	1✔
50	}
51
52	@Override
53	public CallOptions getCallOptions() {
54	return callOptions;	1✔
55	}
56
57	@Override
58	public MethodDescriptor<?, ?> getMethodDescriptor() {
59	return method;	1✔
60	}
61
62	@Override
63	public PickDetailsConsumer getPickDetailsConsumer() {
64	return pickDetailsConsumer;	1✔
65	}
66
67	@Override
68	public boolean equals(Object o) {
69	if (this == o) {	×
70	return true;	×
71	}
72	if (o == null \|\| getClass() != o.getClass()) {	×
73	return false;	×
74	}
75	PickSubchannelArgsImpl that = (PickSubchannelArgsImpl) o;	×
76	return Objects.equal(callOptions, that.callOptions)	×
77	&& Objects.equal(headers, that.headers)	×
78	&& Objects.equal(method, that.method)	×
79	&& Objects.equal(pickDetailsConsumer, that.pickDetailsConsumer);	×
80	}
81
82	@Override
83	public int hashCode() {
84	return Objects.hashCode(callOptions, headers, method, pickDetailsConsumer);	×
85	}
86
87	@Override
88	public final String toString() {
89	return "[method=" + method + " headers=" + headers + " callOptions=" + callOptions + "]";	×
90	}
91	}

grpc / grpc-java / #19625

Source File Press 'n' to go to next uncovered line, 'b' for previous

Source File
Press 'n' to go to next uncovered line, 'b' for previous