From d90f3778a8a395d0ef843c02753bd919e36b5b77 Mon Sep 17 00:00:00 2001
From: Jean Raby <jean@raby.sh>
Date: Fri, 12 Mar 2021 03:51:07 -0500
Subject: [PATCH] Protect stats with RWMutex  (#151)

* Protect stats with RWMutex

updateStats() now also updates rtts so it is protected by the
lock.
Statistics() should now be callable from other goroutines.

PacketsSent and PacketsRecvDuplicates can still be updated while
Statistics() is running, and thus could yield strange results
(sent<recv)

To fix this, atomics could be used, but it would require changing the
types of the fields from int to int64, thus changing the public api.

Signed-off-by: Jean Raby <jean@raby.sh>
---
 ping.go | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/ping.go b/ping.go
index fe51456..f2e23d1 100644
--- a/ping.go
+++ b/ping.go
@@ -143,6 +143,7 @@ type Pinger struct {
 	avgRtt    time.Duration
 	stdDevRtt time.Duration
 	stddevm2  time.Duration
+	statsMu   sync.RWMutex
 
 	// If true, keep a record of rtts of all received packets.
 	// Set to false to avoid memory bloat for long running pings.
@@ -255,7 +256,14 @@ type Statistics struct {
 }
 
 func (p *Pinger) updateStatistics(pkt *Packet) {
+	p.statsMu.Lock()
+	defer p.statsMu.Unlock()
+
 	p.PacketsRecv++
+	if p.RecordRtts {
+		p.rtts = append(p.rtts, pkt.Rtt)
+	}
+
 	if p.PacketsRecv == 1 || pkt.Rtt < p.minRtt {
 		p.minRtt = pkt.Rtt
 	}
@@ -451,9 +459,12 @@ func (p *Pinger) finish() {
 // pinger is running or after it is finished. OnFinish calls this function to
 // get it's finished statistics.
 func (p *Pinger) Statistics() *Statistics {
-	loss := float64(p.PacketsSent-p.PacketsRecv) / float64(p.PacketsSent) * 100
+	p.statsMu.RLock()
+	defer p.statsMu.RUnlock()
+	sent := p.PacketsSent
+	loss := float64(sent-p.PacketsRecv) / float64(sent) * 100
 	s := Statistics{
-		PacketsSent:           p.PacketsSent,
+		PacketsSent:           sent,
 		PacketsRecv:           p.PacketsRecv,
 		PacketsRecvDuplicates: p.PacketsRecvDuplicates,
 		PacketLoss:            loss,
@@ -582,9 +593,6 @@ func (p *Pinger) processPacket(recv *packet) error {
 		return fmt.Errorf("invalid ICMP echo reply; type: '%T', '%v'", pkt, pkt)
 	}
 
-	if p.RecordRtts {
-		p.rtts = append(p.rtts, inPkt.Rtt)
-	}
 	handler := p.OnRecv
 	if handler != nil {
 		handler(inPkt)