this repo has no description
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

cmd/cue: do not defer closing files in fmt's loop

Otherwise, when formatting a thousand files in a single call,
we would delay closing a thousand files until the very end.
This caused "too many open files" errors on Mac with Go 1.18,
before Go started bumping the open file soft limit in Go 1.19.

We don't add a test, because it's not easy:

* Go always lifts the soft limit as much as it can now, so we would have
to lower the hard limit, something which cannot be undone in the
process without superuser privileges.

* Open file limits are set for the entire process, so a test with a
lower limit would have to be awkwardly run via os/exec to prevent
altering all other tests.

* Go and CUE open dozens of files as part of their normal setup,
so we would need a file limit of at least a hundred,
meaning that the test would need to produce and format hundreds of
files to run into "too many open files".

I did manually test the fix; in the same scenario described in the
previous CL, when trying to format 500 CUE files with an open file limit
of 256, instead of an error like:

$ cue fmt *.cue
open [...]/250.cue: too many open files

we now properly format all 500 files without any error.
I also verified via `strace --trace=file,close cue fmt` that we no
longer finish with hundreds of consecutive `close` calls.

The fix is fairly clear, and I'm adding an inline note about why it's
written the way it is now, so I believe that should be enough to make
sure we don't regress.

Fixes #1791.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Change-Id: I159e06ab6c74489c47ee92a7ead475e8f8059b48
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/552625
Unity-Result: CUEcueckoo <cueckoo@cuelang.org>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Reviewed-by: Paul Jolly <paul@myitcv.io>

+4 -1
+4 -1
cmd/cue/cmd/fmt.go
··· 69 69 for _, file := range inst.BuildFiles { 70 70 files := []*ast.File{} 71 71 d := encoding.NewDecoder(file, &cfg) 72 - defer d.Close() 73 72 for ; !d.Done(); d.Next() { 74 73 f := d.File() 75 74 ··· 79 78 80 79 files = append(files, f) 81 80 } 81 + // Do not defer this Close call, as we are looping over builds, 82 + // and otherwise we would hold all of their files open at once. 83 + // Note that we always call Close after NewDecoder as well. 84 + d.Close() 82 85 if err := d.Err(); err != nil { 83 86 exitOnErr(cmd, err, true) 84 87 }