Skip to content

Commit

Permalink
Fix data race in WriteTo
Browse files Browse the repository at this point in the history
There is no harm in shadowing `err` in the goroutine since we do not
need it outside of the routine.

I also discovered data races in tcp_conn_test.go while fixing this bug
and have included them in this PR.
  • Loading branch information
jaystenmark committed Feb 13, 2025
1 parent 31282d5 commit 8389183
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 3 deletions.
4 changes: 2 additions & 2 deletions internal/client/tcp_conn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func TestTCPConn(t *testing.T) {
client = &mockClient{
performTransaction: func(msg *stun.Message, _ net.Addr, _ bool) (TransactionResult, error) {
if msg.Type.Class == stun.ClassRequest && msg.Type.Method == stun.MethodConnect {
msg, err = stun.Build(
msg, err := stun.Build(

Check failure on line 95 in internal/client/tcp_conn_test.go

View workflow job for this annotation

GitHub Actions / lint / Go

shadow: declaration of "err" shadows declaration at line 88 (govet)
stun.TransactionID,
stun.NewType(stun.MethodConnect, stun.ClassErrorResponse),
stun.ErrorCodeAttribute{Code: stun.CodeBadRequest},
Expand Down Expand Up @@ -174,7 +174,7 @@ func TestTCPConn(t *testing.T) {
typ = stun.NewType(stun.MethodCreatePermission, stun.ClassSuccessResponse)
}

msg, err = stun.Build(
msg, err := stun.Build(

Check failure on line 177 in internal/client/tcp_conn_test.go

View workflow job for this annotation

GitHub Actions / lint / Go

shadow: declaration of "err" shadows declaration at line 165 (govet)
stun.TransactionID,
typ,
cid,
Expand Down
2 changes: 1 addition & 1 deletion internal/client/udp_conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ func (c *UDPConn) WriteTo(payload []byte, addr net.Addr) (int, error) { //nolint
if bound.state() == bindingStateReady && time.Since(bound.refreshedAt()) > 5*time.Minute {
bound.setState(bindingStateRefresh)
go func() {
err = c.bind(bound)
err := c.bind(bound)

Check warning on line 243 in internal/client/udp_conn.go

View check run for this annotation

Codecov / codecov/patch

internal/client/udp_conn.go#L243

Added line #L243 was not covered by tests

Check failure on line 243 in internal/client/udp_conn.go

View workflow job for this annotation

GitHub Actions / lint / Go

shadow: declaration of "err" shadows declaration at line 153 (govet)
if err != nil {
c.log.Warnf("Failed to bind() for refresh: %s", err)
bound.setState(bindingStateFailed)
Expand Down

0 comments on commit 8389183

Please sign in to comment.