Prevent dangling cat-file calls (goroutine alternative)

[zeripath]

If an os/exec.Command is passed non *os.File as an input/output, go
will create os.Pipes and wait for their closure in cmd.Wait(). If
the code following this is responsible for closing io.Pipes or other
handlers then on process death from context cancellation the Wait can
hang.

There are two possible solutions:

1. use os.Pipe as the input/output as cmd.Wait does not wait for these.
2. create a goroutine waiting on the context cancellation that will close the inputs.

This PR provides the second option - which is a simpler change that can
be more easily backported.

Reference #19448
Fix #16113

Signed-off-by: Andrew Thornton [email protected]

[Gusted]

I'm in favor of #19448 as it avoids the goroutine is more correct, however if that PR introduces performance problems we should revert back to this PR in which it still will use buffered I/O.

[6543]

I'm in favor of #19448 as it avoids the goroutine is more correct, however if that PR introduces performance problems we should revert back to this PR in which it still will use buffered I/O.

I'd like to retrive some metrics ... - but we should backport THIS

[zeripath]

I vote we should do this and in the meantime I can work on #19448 and try to make it cleaner.

[zeripath]

OK ... one thing we might want to do which will prevent danglers for all calls to git:

diff --git a/modules/git/command.go b/modules/git/command.go
index 3dd12e421..acc1ed66a 100644
--- a/modules/git/command.go
+++ b/modules/git/command.go
@@ -165,6 +165,30 @@ func (c *Command) Run(opts *RunOpts) error {
 		return err
 	}
 
+	closers := make([]io.Closer, 0, 3)
+	for _, pipe := range []interface{}{cmd.Stdout, cmd.Stdin, cmd.Stderr} {
+		if pipe == nil {
+			continue
+		}
+		if _, ok := pipe.(*os.File); ok {
+			continue
+		}
+
+		if closer, ok := pipe.(io.Closer); ok {
+			closers = append(closers, closer)
+		}
+	}
+
+	if len(closers) > 0 {
+		go func() {
+			<-ctx.Done()
+			cancel()
+			for _, closer := range closers {
+				_ = closer.Close()
+			}
+		}()
+	}
+
 	if opts.PipelineFunc != nil {
 		err := opts.PipelineFunc(ctx, cancel)
 		if err != nil {

[6543]

ack for hotfix :)

[6543]

-> #19466