Goroutine in for loop causes unexpected behavior

I was doing the Web Crawler Exercise in A Tour of Go.

I was trying to use concurrent Mutex to solve the question, based on a solution found here. I modified it to fit the pre-defined signatures in the original question. However, the crawler stops at the second level of the URL tree. During debugging, the different behaviors of the print statements completely confused me:

	var done sync.WaitGroup
	for _, u := range urls {
		done.Add(1)
		fmt.Printf("enter: %s\n", u) // here
		go func(url string) {
			defer done.Done()
			Crawl(u, depth-1, fetcher, f)
		}(u)
	}
	done.Wait()

If I put the print statement outside the goroutine, outputs are expected. But I didn't know why it stops there.

enter: https://golang.org/pkg/
enter: https://golang.org/cmd/

But if I put the print statement inside the goroutine, that is

	var done sync.WaitGroup
for _, u := range urls {
done.Add(1)
go func(url string) {
defer done.Done()
fmt.Printf("enter: %s\n", u) // here
Crawl(u, depth-1, fetcher, f)
}(u)
}
done.Wait()

The output becomes

enter: https://golang.org/cmd/
enter: https://golang.org/cmd/

I have two questions:

1. In the second case, why enter: https://golang.org/cmd/ gets printed twice?
2. Why does the Crawl function stop at an error, instead of keeping traversing the URL tree?

PS: the second question might be related to the first one. I intentionally made u instead of url inside the goroutine to reproduce the bug that confused me.

Below is my modified solution

package main

import (
	"fmt"
	"sync"
)

type Fetcher interface {
	// Fetch returns the body of URL and
	// a slice of URLs found on that page.
	Fetch(url string) (body string, urls []string, err error)
}

type fetchState struct {
	mu sync.Mutex
	fetched map[string]bool
}

// Crawl uses fetcher to recursively crawl
// pages starting with url, to a maximum of depth.
func Crawl(url string, depth int, fetcher Fetcher, f *fetchState) {
	// TODO: Fetch URLs in parallel.
	// TODO: Don't fetch the same URL twice.
	// This implementation doesn't do either:
	f.mu.Lock()
	already := f.fetched[url]
	f.fetched[url] = true
	f.mu.Unlock()
	
	if already {
		return
	}
	
	if depth <= 0 {
		return
	}
	body, urls, err := fetcher.Fetch(url)
	if err != nil {
		fmt.Println(err)
		return
	}
	fmt.Printf("found: %s %q\n", url, body)
	
	var done sync.WaitGroup
	for _, u := range urls {
		done.Add(1)
		go func(url string) {
			defer done.Done()
			fmt.Printf("enter: %s\n", u)
			Crawl(u, depth-1, fetcher, f)
		}(u)
	}
	done.Wait()
	
	return
}

func makeState() *fetchState{
	f := &fetchState{}
	f.fetched = make(map[string]bool)
	return f
}

func main() {
	Crawl("https://golang.org/", 4, fetcher, makeState())
}

// fakeFetcher is Fetcher that returns canned results.
type fakeFetcher map[string]*fakeResult

type fakeResult struct {
	body string
	urls []string
}

func (f fakeFetcher) Fetch(url string) (string, []string, error) {
	if res, ok := f[url]; ok {
		return res.body, res.urls, nil
	}
	return "", nil, fmt.Errorf("not found: %s", url)
}

// fetcher is a populated fakeFetcher.
var fetcher = fakeFetcher{
	"https://golang.org/": &fakeResult{
		"The Go Programming Language",
		[]string{
			"https://golang.org/pkg/",
			"https://golang.org/cmd/",
		},
	},
	"https://golang.org/pkg/": &fakeResult{
		"Packages",
		[]string{
			"https://golang.org/",
			"https://golang.org/cmd/",
			"https://golang.org/pkg/fmt/",
			"https://golang.org/pkg/os/",
		},
	},
	"https://golang.org/pkg/fmt/": &fakeResult{
		"Package fmt",
		[]string{
			"https://golang.org/",
			"https://golang.org/pkg/",
		},
	},
	"https://golang.org/pkg/os/": &fakeResult{
		"Package os",
		[]string{
			"https://golang.org/",
			"https://golang.org/pkg/",
		},
	},
}

Welcome to Stack Overflow!

In you function, you defined url as the parameter, but kept using u inside of it.
The loop variable u captured by func literal.

Try doing this:

	var done sync.WaitGroup
for _, u := range urls {
done.Add(1)
go func(url string) {
defer done.Done()
fmt.Printf("enter: %s\n", url)  // <- check the difference
Crawl(url, depth-1, fetcher, f) // <- check the difference
}(u)
}
done.Wait()

For why the same value was being printed with the u variable, this is a very common mistake: https://github.com/golang/go/wiki/CommonMistakes#using-goroutines-on-loop-iterator-variables

In short, the go is passing a single variable by reference to the goroutines. When they execute, they are probably going to find the last value of the iteration in it.

I found this neat article that explains it in detail: https://eli.thegreenplace.net/2019/go-internals-capturing-loop-variables-in-closures/