crypto/tls: data race accessing Conn.handshakeErr in QUICConn.NextEvent

[marten-seemann]

What version of Go are you using (go version)?

$ go version
go version go1.26rc2 darwin/arm64

Does this issue reproduce with the latest release?

No, it does not reproduce with go1.25.x.

What did you do?

A user observed a data race: quic-go/quic-go#5548

What did you expect to see?

No data race.

What did you see instead?

The handshake error variable (handshakeErr) is accessed without a mutex.

Code links:

• Read: https://github.com/golang/go/blob/go1.26rc2/src/crypto/tls/quic.go#L242
• Write: https://github.com/golang/go/blob/go1.26rc2/src/crypto/tls/conn.go#L1562

This bug was introduced in the CL that added QUICErrEvent: https://go-review.googlesource.com/c/go/+/719040

[cagedmantis]

cc @FiloSottile @rolandshoemaker @cpu @golang/security

[rbqvq]

I think add a TryLock are okay.

I think add a lock before NoEvent are okay.

I will get a CL for this.

[gopherbot]

Change https://go.dev/cl/739480 mentions this issue: crypto/tls: avoid data race accessing handshakeErr

[mark-rushakoff]

Cross-posting for visibility (quic-go/quic-go#5548 (comment)) I can confirm that with CL 739480 applied to 2d1f571 , I can no longer produce the data race in my test cases.

[rbqvq]

Thanks.

[gopherbot]

Change https://go.dev/cl/742561 mentions this issue: crypto/tls: avoid data race when canceling a QUICConn's Context

[neild]

The root problem is that with the introduction of QUICErrorEvent, canceling a connection's context now asynchronously generates a connection event. This breaks the QUICConn model of connection events always being generated by synchronous calls on the conn.

CL 742561 ensures that cancelled contexts are only handled as part of a synchronous call: Cancelling a connection's context doesn't generate an event on its own, but the next call to a method on the conn will process the cancellation.

FYI, CL 739480 stops the race detector from reporting a data race, but doesn't address the actual race condition.

[marten-seemann]

I believe the long-term fix for this would be not spawning a Goroutine in the first place, as I’ve suggested in #74922. I’d be happy to put some more work into the CL if there’s interested in getting this resolved for the 1.27 release.

[mark-rushakoff]

I've confirmed that with d4febb4 cherry-picked on top of go1.26rc3, I can no longer reproduce the data race.