mirror of
https://github.com/hanzoai/email-verifier.git
synced 2026-10-30 15:04:29 +00:00
Jason MaandGitHub
2d03a7d19e
AddDisposableDomains is reachable from whatever goroutine the caller likes.
EnableAutoUpdateDisposable runs updateDisposableDomains on a schedule
goroutine. Both touched additionalDisposableDomains, a plain map, with nothing
between them:
verifier.go additionalDisposableDomains[d] = true
handler.go for d := range additionalDisposableDomains { ... }
The Go runtime calls that `fatal error: concurrent map iteration and map
write`. It is not a panic a caller can defend against -- it takes the process
down. Reported by @hodge1997 while reviewing #209.
Reproduced on main as well as on #209, so it predates both:
WARNING: DATA RACE
Read at ... updateDisposableDomains()
Previous write at ... (*Verifier).AddDisposableDomains()
fatal error: concurrent map iteration and map write
sync.Map, which is what disposableSyncDomains already uses for the same reason,
and the declaration moves next to it so the two sit under one comment
explaining why both need it.
Why no test caught it: this is the first test in the package to run anything
concurrently. `make test` does pass -race, but the detector reports only
conflicting accesses that actually execute, and every existing test drives both
functions from the one goroutine. EnableAutoUpdateDisposable does start a
schedule, but its period is 24h, so inside a test that goroutine never fires --
the only updateDisposableDomains call it makes is the synchronous one before
the schedule starts.
TestAddDisposableDomainsIsConcurrentSafe fails 5 times out of 5 without the
change, reporting both the race and the fatal error, and passes 5 out of 5 with
it. Two details make it both reliable and cheap, and the first version had
neither:
- Both goroutines run to a deadline rather than a count. Bounding them
separately let one finish before the other started, and that version passed
on the unfixed code -- useless as a regression test.
- The adder rewrites one key instead of adding new ones. Storing an existing
key is still a map write, so the race is unchanged, but neither map grows.
An adder that keeps adding makes every refresh walk a longer map: quadratic,
and the cost lands only once the bug is fixed and nothing crashes early,
which is why the first version ran for over 100s while appearing fast
against the unfixed code.
It also empties disposableSyncDomains first, since walking all 75k of it is
beside the point here, and restoreDisposableDomains puts it back. 2.3s under
-race; the suite stays at 23s.
require goes after wg.Wait rather than inside the goroutine -- it calls
t.FailNow, which only works on the goroutine running the test, and testifylint
flags it.
35 lines
1.0 KiB
Go
35 lines
1.0 KiB
Go
package emailverifier
|
|
|
|
import (
|
|
"strings"
|
|
"sync"
|
|
)
|
|
|
|
var (
|
|
disposableSyncDomains sync.Map // concurrent safe map to store disposable domains data
|
|
|
|
// additional list of disposable domains set via users of this library.
|
|
// Concurrent safe for the same reason as the map above: it is written by
|
|
// AddDisposableDomains, which callers reach from whatever goroutine they
|
|
// like, and read by updateDisposableDomains on the schedule goroutine that
|
|
// EnableAutoUpdateDisposable starts.
|
|
additionalDisposableDomains sync.Map
|
|
)
|
|
|
|
// IsRoleAccount checks if username is a role-based account
|
|
func (v *Verifier) IsRoleAccount(username string) bool {
|
|
return roleAccounts[strings.ToLower(username)]
|
|
}
|
|
|
|
// IsFreeDomain checks if domain is a free domain
|
|
func (v *Verifier) IsFreeDomain(domain string) bool {
|
|
return freeDomains[domain]
|
|
}
|
|
|
|
// IsDisposable checks if domain is a disposable domain
|
|
func (v *Verifier) IsDisposable(domain string) bool {
|
|
domain = domainToASCII(domain)
|
|
_, found := disposableSyncDomains.Load(domain)
|
|
return found
|
|
}
|