Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/tools/cmd/godoc: fails with race detector enabled & large GOPATH #22110

Closed
kevinburke opened this issue Oct 2, 2017 · 10 comments

Comments

Projects
None yet
3 participants
@kevinburke
Copy link
Contributor

commented Oct 2, 2017

If I build godoc commit golang/tools@68e087e with Go commit cb3dc8b and the -race flag, and then start godoc, I get the following error:

race: limit on 8192 simultaneously alive goroutines is exceeded, dying

If I set GOPATH=/tmp in my environment and run godoc, there are no problems, suggesting that a large number of directories in GOPATH may trigger the problem. So we may want to check if race is enabled and lower the number of concurrent goroutines in that case, or permanently lower the number of concurrent goroutines.

@gopherbot gopherbot added this to the Unreleased milestone Oct 2, 2017

@kevinburke kevinburke changed the title x/tools/cmd/godoc: fails when run with race detector enabled x/tools/cmd/godoc: fails with race detector enabled & large GOPATH Oct 2, 2017

@kevinburke

This comment has been minimized.

Copy link
Contributor Author

commented Oct 2, 2017

I suspect it's this code block in godoc/index.go:

	for dirname := range c.fsDirnames() {
		if c.IndexDirectory != nil && !c.IndexDirectory(dirname) {
			continue
		}
		dirGate <- true
		wg.Add(1)
		go func(dirname string) {
			defer func() { <-dirGate }()
			defer wg.Done()

			list, err := c.fs.ReadDir(dirname)
			if err != nil {
				log.Printf("ReadDir(%q): %v; skipping directory", dirname, err)
				return // ignore this directory
			}
			for _, fi := range list {
				wg.Add(1)
				go func(fi os.FileInfo) {
					defer wg.Done()
					x.visitFile(dirname, fi)
				}(fi)
			}
		}(dirname)
	}
	wg.Wait()
@kevinburke

This comment has been minimized.

Copy link
Contributor Author

commented Oct 2, 2017

Scratch that, I moved the file gate logic up above the goroutine and I'm still getting a crash.

@agnivade

This comment has been minimized.

Copy link
Member

commented Feb 15, 2018

@kevinburke - Can you try again with the latest master ? I swear I saw this issue a few weeks earlier. But now when I am trying to repro this, it's not happening. 😞

@kevinburke

This comment has been minimized.

Copy link
Contributor Author

commented Feb 15, 2018

Yep

$ godocdoc
race: limit on 8192 simultaneously alive goroutines is exceeded, dying
2018/02/14 20:12:54 exit status 66

godocdoc is https://github.com/kevinburke/godocdoc

@kevinburke

This comment has been minimized.

Copy link
Contributor Author

commented Feb 15, 2018

I have a really big source tree. Every GH checkout on my machine is in my GOPATH

@kevinburke

This comment has been minimized.

Copy link
Contributor Author

commented Feb 15, 2018

(Can also trigger it with godoc -http=':6060')

@kevinburke

This comment has been minimized.

Copy link
Contributor Author

commented Feb 15, 2018

@kevinburke

This comment has been minimized.

Copy link
Contributor Author

commented Feb 15, 2018

The problem is here:

func (b *treeBuilder) newDirTree(fset *token.FileSet, path, name string, depth int) *Directory {
	// ...
        for _, d := range list {
		filename := pathpkg.Join(path, d.Name())
		switch {
		case isPkgDir(d):
			ch := make(chan *Directory, 1)
			dirchs = append(dirchs, ch)
			name := d.Name()
			go func() {
				ch <- b.newDirTree(fset, filename, name, depth+1)
			}()

Using a gate the same way as e.g. ioGate to block before opening a new goroutine is tricky because this function calls itself recursively and concurrently, so you can deadlock pretty easily. I need to think through how to structure the lock to avoid a deadlock situation.

@agnivade

This comment has been minimized.

Copy link
Member

commented Feb 15, 2018

Ah nice. I also had a feeling that the issue would be there. 👍

@gopherbot

This comment has been minimized.

Copy link

commented Feb 18, 2018

Change https://golang.org/cl/94955 mentions this issue: godoc: fix runaway goroutine use

@golang golang locked and limited conversation to collaborators Mar 18, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.