Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Data Race: concurrent access of error state during duplexHTTPCall #171

Closed
rodaine opened this issue May 31, 2024 · 2 comments
Closed

Data Race: concurrent access of error state during duplexHTTPCall #171

rodaine opened this issue May 31, 2024 · 2 comments

Comments

@rodaine
Copy link
Member

rodaine commented May 31, 2024

I'm having difficulty determining the provenance of this particular race, but adjacent to the closeSpan call alongside a send func (similar to #170):

WARNING: DATA RACE
Read at 0x00c00fc5b408 by goroutine 34631:
  connectrpc.com/connect.(*Error).Unwrap()
      /Users/rodaine/pkg/mod/connectrpc.com/connect@v1.16.1/error.go:210 +0x38
  errors.is()
      /Users/rodaine/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.3.darwin-arm64/src/errors/wrap.go:63 +0x24c
  errors.Is()
      /Users/rodaine/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.3.darwin-arm64/src/errors/wrap.go:50 +0xfc
  connectrpc.com/connect.IsNotModifiedError()
      /Users/rodaine/pkg/mod/connectrpc.com/connect@v1.16.1/error.go:264 +0x68
  connectrpc.com/otelconnect.statusCodeAttribute()
      /Users/rodaine/pkg/mod/connectrpc.com/otelconnect@v0.7.0/attributes.go:110 +0x5e4
  connectrpc.com/otelconnect.(*Interceptor).WrapStreamingClient.func1.2()
      /Users/rodaine/pkg/mod/connectrpc.com/otelconnect@v0.7.0/interceptor.go:234 +0x120

Previous write at 0x00c00fc5b408 by goroutine 34378:
  connectrpc.com/connect.NewError()
      /Users/rodaine/pkg/mod/connectrpc.com/connect@v1.16.1/error.go:133 +0xcc
  connectrpc.com/connect.wrapIfContextError()
      /Users/rodaine/pkg/mod/connectrpc.com/connect@v1.16.1/error.go:306 +0x114
  connectrpc.com/connect.(*duplexHTTPCall).makeRequest()
      /Users/rodaine/pkg/mod/connectrpc.com/connect@v1.16.1/duplex_http_call.go:310 +0x2f8
  connectrpc.com/connect.(*duplexHTTPCall).Send.gowrap1()
      /Users/rodaine/pkg/mod/connectrpc.com/connect@v1.16.1/duplex_http_call.go:108 +0x38

Goroutine 34631 (running) created at:
  context.(*afterFuncCtx).cancel.func1()
      /Users/rodaine/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.3.darwin-arm64/src/context/context.go:348 +0x44
  sync.(*Once).doSlow()
      /Users/rodaine/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.3.darwin-arm64/src/sync/once.go:74 +0x150
  sync.(*Once).Do()
      /Users/rodaine/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.3.darwin-arm64/src/sync/once.go:65 +0x58
  context.(*afterFuncCtx).cancel()
      /Users/rodaine/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.3.darwin-arm64/src/context/context.go:347 +0x174
  context.(*cancelCtx).cancel()
      /Users/rodaine/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.3.darwin-arm64/src/context/context.go:558 +0x348
  context.(*cancelCtx).cancel()
      /Users/rodaine/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.3.darwin-arm64/src/context/context.go:558 +0x348
  context.(*cancelCtx).cancel()
      /Users/rodaine/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.3.darwin-arm64/src/context/context.go:558 +0x348
  context.WithCancel.func1()
      /Users/rodaine/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.3.darwin-arm64/src/context/context.go:237 +0x78
  main.run.func1()
      main.go:53 +0x20c

Goroutine 34378 (finished) created at:
  connectrpc.com/connect.(*duplexHTTPCall).Send()
      /Users/rodaine/pkg/mod/connectrpc.com/connect@v1.16.1/duplex_http_call.go:108 +0x33c
  connectrpc.com/connect.(*envelopeWriter).write()
      /Users/rodaine/pkg/mod/connectrpc.com/connect@v1.16.1/envelope.go:212 +0x88
  connectrpc.com/connect.(*envelopeWriter).Write()
      /Users/rodaine/pkg/mod/connectrpc.com/connect@v1.16.1/envelope.go:157 +0x37c
  connectrpc.com/connect.(*envelopeWriter).marshalAppend()
      /Users/rodaine/pkg/mod/connectrpc.com/connect@v1.16.1/envelope.go:195 +0x564
  connectrpc.com/connect.(*envelopeWriter).Marshal()
      /Users/rodaine/pkg/mod/connectrpc.com/connect@v1.16.1/envelope.go:143 +0x114
  connectrpc.com/connect.(*connectStreamingClientConn).Send()
      /Users/rodaine/pkg/mod/connectrpc.com/connect@v1.16.1/protocol_connect.go:589 +0x54
  connectrpc.com/connect.(*errorTranslatingClientConn).Send()
      /Users/rodaine/pkg/mod/connectrpc.com/connect@v1.16.1/protocol.go:206 +0x7c
  connectrpc.com/otelconnect.(*streamingState).send()
      /Users/rodaine/pkg/mod/connectrpc.com/otelconnect@v0.7.0/streaming.go:102 +0x68
  connectrpc.com/otelconnect.(*Interceptor).WrapStreamingClient.func1.5()
      /Users/rodaine/pkg/mod/connectrpc.com/otelconnect@v0.7.0/interceptor.go:262 +0x10c
  connectrpc.com/otelconnect.(*streamingClientInterceptor).Send()
      /Users/rodaine/pkg/mod/connectrpc.com/otelconnect@v0.7.0/payloadinterceptor.go:34 +0xa8
  connectrpc.com/connect.(*ClientStreamForClient[ ... ]).Send()
      /Users/rodaine/pkg/mod/connectrpc.com/connect@v1.16.1/client_stream.go:69 +0x110
@rodaine rodaine changed the title Data Race: concurrent access of error state during dupleHTTPCall Data Race: concurrent access of error state during duplexHTTPCall May 31, 2024
@emcfarlane
Copy link
Collaborator

I think this is the same race as #170 . Client stream on close accesses the state of the error here:

if statusCode, ok := statusCodeAttribute(protocol, state.error); ok {

Which is caused from a context cancellation that races with a receive of an error.

akshayjshah pushed a commit that referenced this issue Jun 24, 2024
Fix a data race for a streaming client on close when a the context is
cancelled. This was introduced in #152 where we changed the behaviour to
trigger a close on context cancellation. The state must be locked to
avoid a data race with Send/Receive calls.

Closes #170, #171.

---------

Signed-off-by: Edward McFarlane <emcfarlane@buf.build>
@jhump
Copy link
Member

jhump commented Jul 22, 2024

Resolved in #173, included in release v0.7.1.

@jhump jhump closed this as completed Jul 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants