node/pkg/common: Fix race condition in PostObservationRequest

Any goroutine can push into a channel so the current implementation has
a race condition where the channel can become full immediately after the
length check, causing the subsequent send on the channel to block.

Fix this by wrapping the send on the channel with a select block.
Control will fall through to the default case only if the actual send
operation blocks, avoiding the potential race with other goroutines.
This commit is contained in:
Chirantan Ekbote 2022-08-17 15:06:28 +09:00 committed by Chirantan Ekbote
parent d724d42cf1
commit 4712a6f774
2 changed files with 10 additions and 10 deletions

View File

@ -1,19 +1,20 @@
package common
import (
"fmt"
"errors"
gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1"
)
const ObsvReqChannelSize = 50
const ObsvReqChannelFullError = "channel is full"
func PostObservationRequest(obsvReqSendC chan *gossipv1.ObservationRequest, req *gossipv1.ObservationRequest) error {
if len(obsvReqSendC) >= cap(obsvReqSendC) {
return fmt.Errorf(ObsvReqChannelFullError)
var ErrChanFull = errors.New("channel is full")
func PostObservationRequest(obsvReqSendC chan<- *gossipv1.ObservationRequest, req *gossipv1.ObservationRequest) error {
select {
case obsvReqSendC <- req:
return nil
default:
return ErrChanFull
}
obsvReqSendC <- req
return nil
}

View File

@ -30,8 +30,7 @@ func TestObsvReqSendLimitEnforced(t *testing.T) {
ChainId: uint32(vaa.ChainIDSolana),
}
err := PostObservationRequest(obsvReqSendC, req)
assert.NotNil(t, err)
assert.Equal(t, ObsvReqChannelFullError, err.Error())
assert.ErrorIs(t, err, ErrChanFull)
done = true
}()