commit 06298eec730aa2664bb61d4cce4ef56dfce91ee3 Author: Cecylia Bocovich <cohosh@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