gazelle extension calls cmd.Wait() too eagerly

[tyler-french]

🐞 bug report

Affected Rule

gazelle/python/std_modules.go

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

[sbalabanov]

At the very least, Wait() needs to be in shutdownStdModuleProcess after stdin is closed.
https://github.com/bazelbuild/rules_python/blob/main/gazelle/python/std_modules.go#L75C1-L75C1

[aignas]

@tyler-french, @sbalabanov, let me know if #1550 is something what you are looking for.

[sbalabanov]

lgtm