Skip to content

Commit

Permalink
scion/ping: wait for send go routine to terminate (#4392)
Browse files Browse the repository at this point in the history
Wait for the ping sender go routine to terminate before we return the
stats. With bad luck, the receiver go routine receives all the replies
before the sender go routine has terminated. Because only the sender
go routine increments the stats.Sent count, we end up with wrong stats
that claim more packets were received then sent.

The race condition is that `WriteTo` in the last `pinger.send` succeeds, but then the writer go-routine is not scheduled to increment the `stats.Sent` counter before these stats are returned from `pinger.Ping`.
  • Loading branch information
oncilla authored Sep 18, 2023
1 parent 783896e commit d176fc0
Showing 1 changed file with 6 additions and 0 deletions.
6 changes: 6 additions & 0 deletions scion/ping/ping.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"encoding/binary"
"math/rand"
"net"
"sync"
"time"

"github.com/scionproto/scion/pkg/addr"
Expand Down Expand Up @@ -178,8 +179,12 @@ func (p *pinger) Ping(ctx context.Context, remote *snet.UDPAddr) (Stats, error)
p.drain(ctx)
}()

var wg sync.WaitGroup
wg.Add(1)

go func() {
defer log.HandlePanic()
defer wg.Done()
for i := uint16(0); i < p.attempts; i++ {
if err := p.send(remote); err != nil {
errSend <- serrors.WrapStr("sending", err)
Expand Down Expand Up @@ -210,6 +215,7 @@ func (p *pinger) Ping(ctx context.Context, remote *snet.UDPAddr) (Stats, error)
p.receive(reply)
}
}
wg.Wait()
return p.stats, nil
}

Expand Down

0 comments on commit d176fc0

Please sign in to comment.