zaccone / spf

Sender Policy Framework
MIT License
20 stars 6 forks source link

go test -race discovers possible race condition #23

Closed zaccone closed 7 years ago

zaccone commented 7 years ago

$ go test -v -race

=== RUN TestParseMX

WARNING: DATA RACE Read at 0x00c4202340a0 by goroutine 78: github.com/zaccone/spf.(parser).parseMX.func1() /Users/mde/go/src/github.com/zaccone/spf/parser.go:245 +0xc7 github.com/zaccone/spf.matchIP() /Users/mde/go/src/github.com/zaccone/spf/resolver_miekg.go:84 +0xad github.com/zaccone/spf.(MiekgDNSResolver).MatchIP.func1() /Users/mde/go/src/github.com/zaccone/spf/resolver_miekg.go:112 +0x198

Previous write at 0x00c4202340a0 by goroutine 65: github.com/zaccone/spf.TestParseMX() /Users/mde/go/src/github.com/zaccone/spf/parser_test.go:549 +0xf35 testing.tRunner() /usr/local/go/src/testing/testing.go:610 +0xc9

Goroutine 78 (running) created at: github.com/zaccone/spf.(MiekgDNSResolver).MatchIP() /Users/mde/go/src/github.com/zaccone/spf/resolver_miekg.go:116 +0x17d github.com/zaccone/spf.(MiekgDNSResolver).MatchMX.func1() /Users/mde/go/src/github.com/zaccone/spf/resolver_miekg.go:156 +0x60

Goroutine 65 (running) created at: testing.(T).Run() /usr/local/go/src/testing/testing.go:646 +0x52f testing.RunTests.func1() /usr/local/go/src/testing/testing.go:793 +0xb9 testing.tRunner() /usr/local/go/src/testing/testing.go:610 +0xc9 testing.RunTests() /usr/local/go/src/testing/testing.go:799 +0x4ea testing.(M).Run() /usr/local/go/src/testing/testing.go:743 +0x12f github.com/zaccone/spf.TestMain() /Users/mde/go/src/github.com/zaccone/spf/main_test.go:31 +0x1f1 main.main() github.com/zaccone/spf/_test/_testmain.go:106 +0x1b5

zaccone commented 7 years ago

Actually it's not a real bug, but race checked freak out as we iteratively assign new IP address and inside Parser.ParseMX() use its value as a closure. In my opinion its not a bug, however we must carefully check whether such closures will not lead to real data races.

One possible solution is to make matchIP Resolver's method and copy Parser.IP into Resolver.