Skip to content

Commit 4807f7b

Browse files
authored
Clarify the git command Stdin hanging problem (#26967)
1 parent bafe133 commit 4807f7b

File tree

2 files changed

+12
-17
lines changed

2 files changed

+12
-17
lines changed

modules/git/command.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,8 +221,18 @@ type RunOpts struct {
221221
Dir string
222222

223223
Stdout, Stderr io.Writer
224-
Stdin io.Reader
225-
PipelineFunc func(context.Context, context.CancelFunc) error
224+
225+
// Stdin is used for passing input to the command
226+
// The caller must make sure the Stdin writer is closed properly to finish the Run function.
227+
// 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.
228+
// Some common mistakes:
229+
// * `defer stdinWriter.Close()` then call `cmd.Run()`: the Run() would never return if the command is killed by timeout
230+
// * `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()
231+
// * `go { if stdoutReader.Read() err != nil: stdinWriter.Close() }` with `cmd.Run()`: the stdoutReader may never return error if the command is killed by timeout
232+
// 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.
233+
Stdin io.Reader
234+
235+
PipelineFunc func(context.Context, context.CancelFunc) error
226236
}
227237

228238
func commonBaseEnvs() []string {

modules/indexer/code/indexer.go

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -122,21 +122,6 @@ func Init() {
122122
indexer := *globalIndexer.Load()
123123
for _, indexerData := range items {
124124
log.Trace("IndexerData Process Repo: %d", indexerData.RepoID)
125-
126-
// FIXME: it seems there is a bug in `CatFileBatch` or `nio.Pipe`, which will cause the process to hang forever in rare cases
127-
/*
128-
sync.(*Cond).Wait(cond.go:70)
129-
github.com/djherbis/nio/v3.(*PipeReader).Read(sync.go:106)
130-
bufio.(*Reader).fill(bufio.go:106)
131-
bufio.(*Reader).ReadSlice(bufio.go:372)
132-
bufio.(*Reader).collectFragments(bufio.go:447)
133-
bufio.(*Reader).ReadString(bufio.go:494)
134-
code.gitea.io/gitea/modules/git.ReadBatchLine(batch_reader.go:149)
135-
code.gitea.io/gitea/modules/indexer/code.(*BleveIndexer).addUpdate(bleve.go:214)
136-
code.gitea.io/gitea/modules/indexer/code.(*BleveIndexer).Index(bleve.go:296)
137-
code.gitea.io/gitea/modules/indexer/code.(*wrappedIndexer).Index(wrapped.go:74)
138-
code.gitea.io/gitea/modules/indexer/code.index(indexer.go:105)
139-
*/
140125
if err := index(ctx, indexer, indexerData.RepoID); err != nil {
141126
unhandled = append(unhandled, indexerData)
142127
if !setting.IsInTesting {

0 commit comments

Comments
 (0)