grpc
/
grpc-java
mirror of https://github.com/grpc/grpc-java.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

netty: Avoid volatile attributes on client-side 

6efa9ee3 added `volatile` to `attributes` after TSAN detected a data
race that was added in 91d15ce4. The race was because attributes may be
read from another thread after `transportReady()`, and the
post-filtering assignment occurred after `transportReady()`. The code
now filters the attributes separately so they are updated before calling
`transportReady()`.

Original TSAN failure:
```
  Read of size 4 at 0x0000cd44769c by thread T23:
    #0 io.grpc.netty.NettyClientHandler.getAttributes()Lio/grpc/Attributes; NettyClientHandler.java:327
    #1 io.grpc.netty.NettyClientTransport.getAttributes()Lio/grpc/Attributes; NettyClientTransport.java:363
    #2 io.grpc.netty.NettyClientTransport.newStream(Lio/grpc/MethodDescriptor;Lio/grpc/Metadata;Lio/grpc/CallOptions;[Lio/grpc/ClientStreamTracer;)Lio/grpc/internal/ClientStream; NettyClientTransport.java:183
    #3 io.grpc.internal.MetadataApplierImpl.apply(Lio/grpc/Metadata;)V MetadataApplierImpl.java:74
    #4 io.grpc.auth.GoogleAuthLibraryCallCredentials$1.onSuccess(Ljava/util/Map;)V GoogleAuthLibraryCallCredentials.java:141
    #5 com.google.auth.oauth2.OAuth2Credentials$FutureCallbackToMetadataCallbackAdapter.onSuccess(Lcom/google/auth/oauth2/OAuth2Credentials$OAuthValue;)V OAuth2Credentials.java:534
    #6 com.google.auth.oauth2.OAuth2Credentials$FutureCallbackToMetadataCallbackAdapter.onSuccess(Ljava/lang/Object;)V OAuth2Credentials.java:525
    ...

  Previous write of size 4 at 0x0000cd44769c by thread T24:
    #0 io.grpc.netty.NettyClientHandler$FrameListener.onSettingsRead(Lio/netty/channel/ChannelHandlerContext;Lio/netty/handler/codec/http2/Http2Settings;)V NettyClientHandler.java:920
    #1 io.netty.handler.codec.http2.DefaultHttp2ConnectionDecoder$FrameReadListener.onSettingsRead(Lio/netty/channel/ChannelHandlerContext;Lio/netty/handler/codec/http2/Http2Settings;)V DefaultHttp2ConnectionDecoder.java:515
    ...
```
This commit is contained in:
Eric Anderson 2024-01-11 15:31:27 -08:00
parent c346b41e22
commit 91cfa97c28
2 changed files with 11 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
netty/src/main/java/io/grpc/netty/ClientTransportLifecycleManager.java
View File

@ -37,14 +37,19 @@ final class ClientTransportLifecycleManager {
this.listener = listener; this.listener = listener;
} }
public Attributes notifyReady(Attributes attributes) { public Attributes filterAttributes(Attributes attributes) {
if (transportReady || transportShutdown) { if (transportReady || transportShutdown) {
return attributes; return attributes;
} }
return listener.filterTransport(attributes);
}
public void notifyReady() {
if (transportReady || transportShutdown) {
return;
}
transportReady = true; transportReady = true;
attributes = listener.filterTransport(attributes);
listener.transportReady(); listener.transportReady();
return attributes;
} }
/** /**

5
netty/src/main/java/io/grpc/netty/NettyClientHandler.java
View File

@ -131,7 +131,7 @@ class NettyClientHandler extends AbstractNettyHandler {
private WriteQueue clientWriteQueue; private WriteQueue clientWriteQueue;
private Http2Ping ping; private Http2Ping ping;
private volatile Attributes attributes; private Attributes attributes;
private InternalChannelz.Security securityInfo; private InternalChannelz.Security securityInfo;
private Status abruptGoAwayStatus; private Status abruptGoAwayStatus;
private Status channelInactiveReason; private Status channelInactiveReason;
@ -917,7 +917,8 @@ class NettyClientHandler extends AbstractNettyHandler {
public void onSettingsRead(ChannelHandlerContext ctx, Http2Settings settings) { public void onSettingsRead(ChannelHandlerContext ctx, Http2Settings settings) {
if (firstSettings) { if (firstSettings) {
firstSettings = false; firstSettings = false;
attributes = lifecycleManager.notifyReady(attributes); attributes = lifecycleManager.filterAttributes(attributes);
lifecycleManager.notifyReady();
} }
} }