[tor-commits] [snowflake/master] Added another lock to protect broker stats

[cohosh at torproject.org]

commit 06298eec730aa2664bb61d4cce4ef56dfce91ee3
Author: Cecylia Bocovich <cohosh at torproject.org>
Date:   Mon Dec 2 13:22:05 2019 -0500

    Added another lock to protect broker stats
    
    Added another lock to the metrics struct to synchronize accesses to the
    broker stats. There's a possible race condition if stats are updated at
    the same time they are being logged.
---
 broker/broker.go  | 10 +++++++++-
 broker/metrics.go |  5 +++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/broker/broker.go b/broker/broker.go
index a5b0edf..17c677e 100644
--- a/broker/broker.go
+++ b/broker/broker.go
@@ -38,7 +38,7 @@ type BrokerContext struct {
 	// Map keeping track of snowflakeIDs required to match SDP answers from
 	// the second http POST.
 	idToSnowflake map[string]*Snowflake
-	// Synchronization for the
+	// Synchronization for the snowflake map and heap
 	snowflakeLock sync.Mutex
 	proxyPolls    chan *ProxyPoll
 	metrics       *Metrics
@@ -181,14 +181,18 @@ func proxyPolls(ctx *BrokerContext, w http.ResponseWriter, r *http.Request) {
 	if err != nil {
 		log.Println("Error processing proxy IP: ", err.Error())
 	} else {
+		ctx.metrics.lock.Lock()
 		ctx.metrics.UpdateCountryStats(remoteIP, proxyType)
+		ctx.metrics.lock.Unlock()
 	}
 
 	// Wait for a client to avail an offer to the snowflake, or timeout if nil.
 	offer := ctx.RequestOffer(sid, proxyType)
 	var b []byte
 	if nil == offer {
+		ctx.metrics.lock.Lock()
 		ctx.metrics.proxyIdleCount++
+		ctx.metrics.lock.Unlock()
 
 		b, err = messages.EncodePollResponse("", false)
 		if err != nil {
@@ -227,7 +231,9 @@ func clientOffers(ctx *BrokerContext, w http.ResponseWriter, r *http.Request) {
 	numSnowflakes := ctx.snowflakes.Len()
 	ctx.snowflakeLock.Unlock()
 	if numSnowflakes <= 0 {
+		ctx.metrics.lock.Lock()
 		ctx.metrics.clientDeniedCount++
+		ctx.metrics.lock.Unlock()
 		w.WriteHeader(http.StatusServiceUnavailable)
 		return
 	}
@@ -241,7 +247,9 @@ func clientOffers(ctx *BrokerContext, w http.ResponseWriter, r *http.Request) {
 	// Wait for the answer to be returned on the channel or timeout.
 	select {
 	case answer := <-snowflake.answerChannel:
+		ctx.metrics.lock.Lock()
 		ctx.metrics.clientProxyMatchCount++
+		ctx.metrics.lock.Unlock()
 		if _, err := w.Write(answer); err != nil {
 			log.Printf("unable to write answer with error: %v", err)
 		}
diff --git a/broker/metrics.go b/broker/metrics.go
index bf5ce29..ea4d220 100644
--- a/broker/metrics.go
+++ b/broker/metrics.go
@@ -39,6 +39,9 @@ type Metrics struct {
 	proxyIdleCount          uint
 	clientDeniedCount       uint
 	clientProxyMatchCount   uint
+
+	//synchronization for access to snowflake metrics
+	lock sync.Mutex
 }
 
 func (s CountryStats) Display() string {
@@ -161,6 +164,7 @@ func (m *Metrics) logMetrics() {
 }
 
 func (m *Metrics) printMetrics() {
+	m.lock.Lock()
 	m.logger.Println("snowflake-stats-end", time.Now().UTC().Format("2006-01-02 15:04:05"), fmt.Sprintf("(%d s)", int(metricsResolution.Seconds())))
 	m.logger.Println("snowflake-ips", m.countryStats.Display())
 	m.logger.Println("snowflake-ips-total", len(m.countryStats.standalone)+
@@ -171,6 +175,7 @@ func (m *Metrics) printMetrics() {
 	m.logger.Println("snowflake-idle-count", binCount(m.proxyIdleCount))
 	m.logger.Println("client-denied-count", binCount(m.clientDeniedCount))
 	m.logger.Println("client-snowflake-match-count", binCount(m.clientProxyMatchCount))
+	m.lock.Unlock()
 }
 
 // Restores all metrics to original values