git » dnss » commit d19c4a3

dnstox: Remove answer race in the caching resolver

author Alberto Bertogli
2016-05-30 10:06:21 UTC
committer Alberto Bertogli
2016-05-30 10:33:26 UTC
parent 05af0de5493d702f8e5e92695904329bff7d1691
dnstox: Remove answer race in the caching resolver

The cache maintenance goroutine regularly modifies the cached answers,
updating their TTLs.

When responding to queries, we use those cached answers, but currently there
is a race because the answers we return could be modified by the maintenance
goroutine concurrently.

This patch fixes that race by never modifying the cached answers, but updating
copies while doing maintenance. This is much faster than copying on each cache
hit, but may extend the maintenance period.
internal/dnstox/resolver.go +14 -1 
diff --git a/internal/dnstox/resolver.go b/internal/dnstox/resolver.go
index 367f6cd..6b3cf49 100644
--- a/internal/dnstox/resolver.go
+++ b/internal/dnstox/resolver.go
@@ -406,7 +406,12 @@ func (c *cachingResolver) Maintain() {
 		for q, ans := range c.answer {
 			newTTL := getTTL(ans) - maintenancePeriod
 			if newTTL > 0 {
-				setTTL(ans, newTTL)
+				// Don't modify in place, create a copy and override.
+				// That way, we avoid races with users that have gotten a
+				// cached answer and are returning it.
+				newans := copyRRSlice(ans)
+				setTTL(newans, newTTL)
+				c.answer[q] = newans
 				continue
 			}
 
@@ -465,6 +470,14 @@ func setTTL(answer []dns.RR, newTTL time.Duration) {
 	}
 }
 
+func copyRRSlice(a []dns.RR) []dns.RR {
+	b := make([]dns.RR, 0, len(a))
+	for _, rr := range a {
+		b = append(b, dns.Copy(rr))
+	}
+	return b
+}
+
 func (c *cachingResolver) Query(r *dns.Msg, tr trace.Trace) (*dns.Msg, error) {
 	stats.cacheTotal.Add(1)