Skip to content

Commit

Permalink
Don't lazy-init checker cache
Browse files Browse the repository at this point in the history
This caused a data race as multiple processes might try to create a new
cache and store it on the checker.
  • Loading branch information
vbrown608 committed Mar 1, 2019
1 parent 67a0f09 commit a3aa0c8
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 15 deletions.
8 changes: 0 additions & 8 deletions checker/checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ type Checker struct {
Timeout time.Duration

// Cache specifies the hostname scan cache store and expire time.
// Defaults to a 10-minute in-memory cache.
Cache *ScanCache

// lookupMX specifies an alternate function to retrieve hostnames for a given
Expand All @@ -33,10 +32,3 @@ func (c *Checker) timeout() time.Duration {
}
return 10 * time.Second
}

func (c *Checker) cache() *ScanCache {
if c.Cache == nil {
c.Cache = MakeSimpleCache(10 * time.Minute)
}
return c.Cache
}
7 changes: 1 addition & 6 deletions checker/domain.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,12 +120,7 @@ func (c *Checker) CheckDomain(domain string, expectedHostnames []string) DomainR
}
checkedHostnames := make([]string, 0)
for _, hostname := range hostnames {
cache := c.cache()
hostnameResult, err := cache.GetHostnameScan(hostname)
if err != nil {
hostnameResult = c.CheckHostname(domain, hostname)
cache.PutHostnameScan(hostname, hostnameResult)
}
hostnameResult := c.CheckHostnameWithCache(domain, hostname)
result.HostnameResults[hostname] = hostnameResult
if hostnameResult.couldConnect() {
checkedHostnames = append(checkedHostnames, hostname)
Expand Down
14 changes: 14 additions & 0 deletions checker/hostname.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,20 @@ func checkTLSVersion(client *smtp.Client, hostname string, timeout time.Duration
return result.Success()
}

// CheckHostnameWithCache returns the result of CheckHostname, using or
// updating the Checker's cache.
func (c *Checker) CheckHostnameWithCache(domain string, hostname string) HostnameResult {
if c.Cache == nil {
return c.CheckHostname(domain, hostname)
}
hostnameResult, err := c.Cache.GetHostnameScan(hostname)
if err != nil {
hostnameResult = c.CheckHostname(domain, hostname)
c.Cache.PutHostnameScan(hostname, hostnameResult)
}
return hostnameResult
}

// CheckHostname performs a series of checks against a hostname for an email domain.
// `domain` is the mail domain that this server serves email for.
// `hostname` is the hostname for this server.
Expand Down
4 changes: 3 additions & 1 deletion validator/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ func validateRegularly(v DomainPolicyStore, interval time.Duration,
// Hostname map. Interval specifies the interval to wait between each run.
// Failures are reported to Sentry.
func ValidateRegularly(v DomainPolicyStore, interval time.Duration) {
c := checker.Checker{}
c := checker.Checker{
Cache: checker.MakeSimpleCache(10 * time.Minute),
}
validateRegularly(v, interval, c.CheckDomain, reportToSentry)
}

0 comments on commit a3aa0c8

Please sign in to comment.