I have been following this solution. There is no race condition detected when i run race detecter. But when i run race detecter with my code it gives the following error:
================== WARNING: DATA RACE Read at 0x00c42006c1e0 by goroutine 6: main.Crawl.func1() /task2.go:50 +0x53
Previous write at 0x00c42006c1e0 by main goroutine: main.Crawl() /task2.go:48 +0x692 main.main() /task2.go:66 +0x8c
Goroutine 6 (running) created at: main.Crawl() /task2.go:49 +0x61e main.main() /task2.go:66 +0x8c ================== . . . ================== WARNING: DATA RACE Read at 0x00c420094070 by goroutine 8: main.Crawl.func1() /task2.go:50 +0x53
Previous write at 0x00c420094070 by goroutine 6: main.Crawl() /task2.go:48 +0x692 main.Crawl.func1() /task2.go:51 +0x240
Goroutine 8 (running) created at: main.Crawl() /task2.go:49 +0x61e main.Crawl.func1() /task2.go:51 +0x240
Goroutine 6 (running) created at: main.Crawl() /task2.go:49 +0x61e main.main()
/task2.go:66 +0x8c
Found 2 data race(s) exit status 66
Following is my code, can anyone please tell me where i'm going wrong. I have been trying to figure it out for so long but could not identify.
var visited = struct {
urls map[string]bool
sync.Mutex
}{urls: make(map[string]bool)}
func Crawl(url string, depth int, fetcher Fetcher) {
if depth <= 0 {
return
}
visited.Lock()
if visited.urls[url] && visited.urls[url] == true {
fmt.Println("already fetched: ", url)
visited.Unlock()
return
}
visited.urls[url] = true
visited.Unlock()
body, urls, err := fetcher.Fetch(url)
if err != nil {
fmt.Println(err)
return
}
done := make(chan bool)
for _, nestedUrl := range urls {
go func(url string, d int) {
fmt.Printf("-> Crawling child %v of %v with depth %v
", nestedUrl, url, depth)
Crawl(url, d, fetcher)
done <- true
}(nestedUrl, depth-1)
}
for i := range urls {
fmt.Printf("<- [%v] %v/%v Waiting for child %v.
", url, i, len(urls))
<-done
}
fmt.Printf("<- Done with %v
", url)
}
func main() {
Crawl("http://golang.org/", 4, fetcher)
fmt.Println("Fetching stats
--------------")
for url, err := range visited.urls {
if err != true {
fmt.Printf("%v failed: %v
", url, err)
} else {
fmt.Printf("%v was fetched
", url)
}
}
}
You’re calling Crawl, which fires off a go routine to recurse, then you’re accessing the protected map visited without a mutex in main which is executed before some crawls finish. A few points on style:
So start synchronous, then figure out how best to change to async. You can then make it a sync just by putting go in front of your synchronous crawl function. Looking at the original tour, it doesn't much resemble this solution, so I'm not sure this is a great model to follow. The caller should not have to lock or worry about races, so you need to redesign. I'd start again from the original tour exercise.
For the lock, I'd use
type T struct {
data map[string]bool
mu sync.Mutex // not just sync.Mutex
}
and the T decides when locking is required and has functions to adjust the state of data or search it. This makes it simpler to think about using the Lock and less likely you make a mistake.