-
-
Notifications
You must be signed in to change notification settings - Fork 678
gazelle extension calls cmd.Wait() too eagerly #1546
Description
🐞 bug report
Affected Rule
Is this a regression?
No
Description
os.Stdout and os.Stdin are assigned to global mutable variables in this file. Then, they are initialized in this function.
When running gazelle at Uber, the process can sometimes take 10-15 minutes as we traverse 100ks of directories with Gazelle. To help make this more familiar, we tried to implement metrics and progress logging. One of the metrics emitters uses a call to aos/exec.Cmd. We noticed that when this is added, we experience intermittent failures that look like this:
gazelle: failed to wait for std_modules: signal: killed
🔬 Minimal Reproduction
Add this patch to gazelle:
diff --git a/cmd/gazelle/fix-update.go b/cmd/gazelle/fix-update.go
index cd20b0d..95eb59a 100644
--- a/cmd/gazelle/fix-update.go
+++ b/cmd/gazelle/fix-update.go
@@ -16,6 +16,7 @@ limitations under the License.
package main
import (
+ "os/exec"
"bytes"
"context"
"errors"
@@ -251,6 +252,7 @@ var genericLoads = []rule.LoadInfo{
}
func runFixUpdate(wd string, cmd command, args []string) (err error) {
+ defer exec.Command("ls", "-l").Run()
cexts := make([]config.Configurer, 0, len(languages)+4)
cexts = append(cexts,
&config.CommonConfigurer{},
Run gazelle on a small directory with the python gazelle plugin installed:
tfrench@tfrench-go ~/go-code % bin/gazelle some/dir
/home/user/go-code/tools/bazel run //:gazelle --build_event_binary_file /tmp/tmp8hg5rjv4/events.dat -- --index=false --resolveGen=true /home/user/go-code/some/dir
INFO: Invocation ID: 1cf28ae6-ba46-459c-8deb-e5d377cb7712
INFO: Analyzed target //:gazelle (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //:gazelle up-to-date:
bazel-bin/gazelle-runner.bash
bazel-bin/gazelle
INFO: Elapsed time: 0.418s, Critical Path: 0.01s
INFO: 1 process: 1 internal.
INFO: Running command line: bazel-bin/gazelle '--index=false' '--resolveGen=true' /home/user/go-code/some/dir
INFO: Build Event Protocol files produced successfully.
INFO: Build completed successfully, 1 total action
gazelle: failed to wait for std_modules: signal: killed
🔥 Exception or Error
gazelle: failed to wait for std_modules: signal: killed
This seems to be caused by calling cmd.Wait https://pkg.go.dev/os/exec#Cmd.StdoutPipe
Wait will close the pipe after seeing the command exit, so most callers need not close the pipe themselves. It is thus incorrect to call Wait before all reads from the pipe have completed.
🌍 Your Environment
Operating System:
linux_x86_64
Output of bazel version:
6.4.0
Rules_python version:
go_repository(
name = "com_github_bazelbuild_rules_python_gazelle",
build_extra_args = ["-go_naming_convention_external=go_default_library"],
build_file_generation = "on",
build_file_proto_mode = "disable",
importpath = "github.com/bazelbuild/rules_python/gazelle",
sum = "h1:Xf++ptT/cHFAS8FvSCQgxDwGNOTgBkBgiTcFYoctWxU=",
version = "v0.0.0-20230823134539-e3449dc0ee23",
)
Anything else relevant?
Gazelle version 0.33.0