From 4807f7be22c2cf5ceffab241cec24058f1628125 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 8 Sep 2023 21:20:38 +0800 Subject: [PATCH] Clarify the git command Stdin hanging problem (#26967) --- modules/git/command.go | 14 ++++++++++++-- modules/indexer/code/indexer.go | 15 --------------- 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/modules/git/command.go b/modules/git/command.go index c38fd04696..f095bb18be 100644 --- a/modules/git/command.go +++ b/modules/git/command.go @@ -221,8 +221,18 @@ type RunOpts struct { Dir string Stdout, Stderr io.Writer - Stdin io.Reader - PipelineFunc func(context.Context, context.CancelFunc) error + + // Stdin is used for passing input to the command + // The caller must make sure the Stdin writer is closed properly to finish the Run function. + // Otherwise, the Run function may hang for long time or forever, especially when the Git's context deadline is not the same as the caller's. + // Some common mistakes: + // * `defer stdinWriter.Close()` then call `cmd.Run()`: the Run() would never return if the command is killed by timeout + // * `go { case <- parentContext.Done(): stdinWriter.Close() }` with `cmd.Run(DefaultTimeout)`: the command would have been killed by timeout but the Run doesn't return until stdinWriter.Close() + // * `go { if stdoutReader.Read() err != nil: stdinWriter.Close() }` with `cmd.Run()`: the stdoutReader may never return error if the command is killed by timeout + // In the future, ideally the git module itself should have full control of the stdin, to avoid such problems and make it easier to refactor to a better architecture. + Stdin io.Reader + + PipelineFunc func(context.Context, context.CancelFunc) error } func commonBaseEnvs() []string { diff --git a/modules/indexer/code/indexer.go b/modules/indexer/code/indexer.go index 80bde7614f..019773fe51 100644 --- a/modules/indexer/code/indexer.go +++ b/modules/indexer/code/indexer.go @@ -122,21 +122,6 @@ func Init() { indexer := *globalIndexer.Load() for _, indexerData := range items { log.Trace("IndexerData Process Repo: %d", indexerData.RepoID) - - // FIXME: it seems there is a bug in `CatFileBatch` or `nio.Pipe`, which will cause the process to hang forever in rare cases - /* - sync.(*Cond).Wait(cond.go:70) - github.com/djherbis/nio/v3.(*PipeReader).Read(sync.go:106) - bufio.(*Reader).fill(bufio.go:106) - bufio.(*Reader).ReadSlice(bufio.go:372) - bufio.(*Reader).collectFragments(bufio.go:447) - bufio.(*Reader).ReadString(bufio.go:494) - code.gitea.io/gitea/modules/git.ReadBatchLine(batch_reader.go:149) - code.gitea.io/gitea/modules/indexer/code.(*BleveIndexer).addUpdate(bleve.go:214) - code.gitea.io/gitea/modules/indexer/code.(*BleveIndexer).Index(bleve.go:296) - code.gitea.io/gitea/modules/indexer/code.(*wrappedIndexer).Index(wrapped.go:74) - code.gitea.io/gitea/modules/indexer/code.index(indexer.go:105) - */ if err := index(ctx, indexer, indexerData.RepoID); err != nil { unhandled = append(unhandled, indexerData) if !setting.IsInTesting {