Skip to content

Commit

Permalink
avoid "too many open files" error when loading module info
Browse files Browse the repository at this point in the history
We would call "go mod edit -json" for each Go file we formatted,
as each file may be in a different directory,
and thus inside a different module.

This could cause us to run into open file limits,
because spawning a child process and grabbing its output opens files of
its own such as named pipes.
The added test shows this with a limit of 256 and 10k tiny Go files:

	--- FAIL: TestWithLowOpenFileLimit (0.30s)
		ulimit_unix_test.go:82: writing 10000 tiny Go files
		ulimit_unix_test.go:104: running with 1 paths
		ulimit_unix_test.go:104: running with 10000 paths
		ulimit_unix_test.go:112:
			error:
			  got non-nil error
			comment:
			  stderr:
			  open /tmp/TestWithLowOpenFileLimit2753748366/001/p003/014.go: too many open files
			  open /tmp/TestWithLowOpenFileLimit2753748366/001/p003/017.go: too many open files
			  open /tmp/TestWithLowOpenFileLimit2753748366/001/p004/000.go: too many open files
			  open /tmp/TestWithLowOpenFileLimit2753748366/001/p004/019.go: too many open files

Instead, only call "go mod edit -json" once per directory,
and do it in the main thread to reduce its parallelism.
Also make it grab fdSem as well, for good measure.

This may not be a complete fix, as we're not sure how many files are
open by an exec.Command.Output call. However, we are no longer able to
reproduce a failure, so leave that as a TODO.

Fixes #208.
  • Loading branch information
mvdan committed Mar 18, 2022
1 parent 9eee203 commit 9d4d28a
Show file tree
Hide file tree
Showing 2 changed files with 135 additions and 9 deletions.
56 changes: 47 additions & 9 deletions gofmt.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"runtime"
"runtime/pprof"
"strings"
"sync"

"golang.org/x/sync/semaphore"
exec "golang.org/x/sys/execabs"
Expand Down Expand Up @@ -288,15 +289,10 @@ func processFile(filename string, info fs.FileInfo, in io.Reader, r *reporter, e

// If either -lang or -modpath aren't set, fetch them from go.mod.
if *langVersion == "" || *modulePath == "" {
out, err := exec.Command("go", "mod", "edit", "-json").Output()
if err == nil && len(out) > 0 {
var mod struct {
Go string
Module struct {
Path string
}
}
_ = json.Unmarshal(out, &mod)
dir := filepath.Dir(filename)
mod, ok := moduleCacheByDir.Load(dir)
if ok && mod != nil {
mod := mod.(*module)
if *langVersion == "" {
*langVersion = mod.Go
}
Expand Down Expand Up @@ -532,7 +528,49 @@ func gofmtMain(s *sequencer) {
}
}

type module struct {
Go string
Module struct {
Path string
}
}

func loadModuleInfo(dir string) interface{} {
// Spawning "go mod edit" will open files by design,
// such as the named pipe to obtain stdout.
// TODO(mvdan): if we run into "too many open files" errors again in the
// future, we probably need to turn fdSem into a weighted semaphore so this
// operation can acquire a weight larger than 1.
fdSem <- true
out, err := exec.Command("go", "mod", "edit", "-json").Output()
defer func() { <-fdSem }()
if err != nil || len(out) == 0 {
return nil
}
mod := new(module)
if err := json.Unmarshal(out, mod); err != nil {
return nil
}
if *langVersion == "" {
*langVersion = mod.Go
}
if *modulePath == "" {
*modulePath = mod.Module.Path
}
return mod
}

// Written to by fileWeight, read from fileWeight and processFile.
// A present but nil value means that loading the module info failed.
// Note that we don't require the keys to be absolute directories,
// so duplicates are possible. The same can happen with symlinks.
var moduleCacheByDir sync.Map // map[dirString]*module

func fileWeight(path string, info fs.FileInfo) int64 {
dir := filepath.Dir(path)
if _, ok := moduleCacheByDir.Load(dir); !ok {
moduleCacheByDir.Store(dir, loadModuleInfo(dir))
}
if info == nil {
return exclusive
}
Expand Down
88 changes: 88 additions & 0 deletions ulimit_unix_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
// Copyright (c) 2019, Daniel Martí <mvdan@mvdan.cc>
// See LICENSE for licensing information

// TODO: replace with the unix build tag once we require Go 1.19 or later
//go:build linux

package main

import (
"bytes"
"fmt"
"os"
"os/exec"
"path/filepath"
"strconv"
"testing"

qt "github.com/frankban/quicktest"
"golang.org/x/sys/unix"
)

func init() {
// Here rather than in TestMain, to reuse the unix build tag.
if limit := os.Getenv("TEST_WITH_FILE_LIMIT"); limit != "" {
n, err := strconv.ParseUint(limit, 10, 64)
if err != nil {
panic(err)
}
rlimit := unix.Rlimit{Cur: n, Max: n}
if err := unix.Setrlimit(unix.RLIMIT_NOFILE, &rlimit); err != nil {
panic(err)
}
os.Exit(main1())
}
}

func TestWithLowOpenFileLimit(t *testing.T) {
// Safe to run in parallel, as we only change the limit for child processes.
t.Parallel()

tempDir := t.TempDir()
testBinary, err := os.Executable()
qt.Assert(t, err, qt.IsNil)

const (
// Enough directories to run into the ulimit.
// Enough number of files in total to run into the ulimit.
numberDirs = 500
numberFilesPerDir = 20
numberFilesTotal = numberDirs * numberFilesPerDir
)
t.Logf("writing %d tiny Go files", numberFilesTotal)
var allGoFiles []string
for i := 0; i < numberDirs; i++ {
// Prefix "p", so the package name is a valid identifier.
dirName := fmt.Sprintf("p%03d", i)
dirPath := filepath.Join(tempDir, dirName)
err := os.MkdirAll(dirPath, 0o777)
qt.Assert(t, err, qt.IsNil)
for j := 0; j < numberFilesPerDir; j++ {
filePath := filepath.Join(dirPath, fmt.Sprintf("%03d.go", j))
err := os.WriteFile(filePath,
// Extra newlines so that "-l" prints all paths.
[]byte(fmt.Sprintf("package %s\n\n\n", dirName)),
0o666)
qt.Assert(t, err, qt.IsNil)
allGoFiles = append(allGoFiles, filePath)
}
}
if len(allGoFiles) != numberFilesTotal {
panic("allGoFiles doesn't have the expected number of files?")
}
runGofmt := func(paths ...string) {
t.Logf("running with %d paths", len(paths))
cmd := exec.Command(testBinary, append([]string{"-l"}, paths...)...)
// 256 is a relatively common low limit, e.g. on Mac.
cmd.Env = append(os.Environ(), "TEST_WITH_FILE_LIMIT=256")
out, err := cmd.Output()
var stderr []byte
if err, _ := err.(*exec.ExitError); err != nil {
stderr = err.Stderr
}
qt.Assert(t, err, qt.IsNil, qt.Commentf("stderr:\n%s", stderr))
qt.Assert(t, bytes.Count(out, []byte("\n")), qt.Equals, len(allGoFiles))
}
runGofmt(tempDir)
runGofmt(allGoFiles...)
}

0 comments on commit 9d4d28a

Please sign in to comment.