search

newrelic / go-agent

New Relic Go Agent
Apache License 2.0
775 stars 295 forks source link

3.23 Regression: parallel NewGoroutine calls result in data races #748

Closed adomaskizogian closed 1 year ago

adomaskizogian commented 1 year ago

We have observed that running application with race detector enabled (-race) it detects that parallel calls to newrelic.FromContext(ctx).NewGoroutine() results in data races.

No issues with 3.22

sample code that calls NewGoroutine

func NewNewrelicGoroutineTransaction(ctx context.Context) (context.Context, *newrelic.Transaction){
   txn := newrelic.FromContext(ctx)
   if txn != nil {
    gTxn := txn.NewGoroutine()  
    ctx = newrelic.NewContext(ctx, gTxn)
    return  ctx, gTxn
  }
  return ctx, nil
}
WARNING: DATA RACE
Write at 0x00c0010cc6d8 by goroutine 2026:
  github.com/newrelic/go-agent/v3/newrelic.(*Transaction).NewGoroutine()
      /home/adomas.kizogian/go/pkg/mod/github.com/newrelic/go-agent/v3@v3.23.0/newrelic/transaction.go:502 +0x1db
rittneje commented 1 year ago

@iamemilio This bug prevents us from upgrading from 3.22.1. Can you please resolve it? Also, this library ought to run its unit tests with the race detector, in order to detect such bugs during development.

nr-swilloughby commented 1 year ago

We're working to release a fix for this ASAP.

nr-swilloughby commented 1 year ago

Fix released

adomaskizogian commented 1 year ago

Thank you guys!

rittneje commented 1 year ago

@nr-swilloughby In the future it would be nice to include fixes like this in the release notes.

nr-swilloughby commented 1 year ago

@rittneje sorry about that. The release notes mentioned the fix that was made, which happened to be the cause of both a performance issue and the race condition, but neglected to actually say that it fixed the race condition as well. I'll make a note about that in the next release.