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 authored and Sean-Der committed Feb 14, 2025
1 parent 31282d5 commit 4941636
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 8 deletions.
8 changes: 4 additions & 4 deletions internal/client/tcp_conn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,12 @@ 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, buildErr := stun.Build(
stun.TransactionID,
stun.NewType(stun.MethodConnect, stun.ClassErrorResponse),
stun.ErrorCodeAttribute{Code: stun.CodeBadRequest},
)
assert.NoError(t, err)
assert.NoError(t, buildErr)

return TransactionResult{Msg: msg}, nil
}
Expand Down Expand Up @@ -174,12 +174,12 @@ func TestTCPConn(t *testing.T) {
typ = stun.NewType(stun.MethodCreatePermission, stun.ClassSuccessResponse)
}

msg, err = stun.Build(
msg, buildErr := stun.Build(
stun.TransactionID,
typ,
cid,
)
assert.NoError(t, err)
assert.NoError(t, buildErr)

return TransactionResult{Msg: msg}, nil
},
Expand Down
5 changes: 2 additions & 3 deletions internal/client/udp_conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,9 +240,8 @@ 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)
if err != nil {
c.log.Warnf("Failed to bind() for refresh: %s", err)
if bindErr := c.bind(bound); bindErr != nil {
c.log.Warnf("Failed to bind() for refresh: %s", bindErr)

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

View check run for this annotation

Codecov / codecov/patch

internal/client/udp_conn.go#L243-L244

Added lines #L243 - L244 were not covered by tests
bound.setState(bindingStateFailed)
// Keep going...
} else {
Expand Down
2 changes: 1 addition & 1 deletion lt_cred.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

package turn

import ( //nolint:gci
import (
"crypto/hmac"
"crypto/sha1" //nolint:gosec,gci
"encoding/base64"
Expand Down

0 comments on commit 4941636

Please sign in to comment.