Skip to content

Commit 350584c

Browse files
authored
fix(gitter): Better handling for context cancel (#4918)
Add proper handling for context cancellation in semaphore and our longer running git operations.
1 parent 3a27acc commit 350584c

2 files changed

Lines changed: 33 additions & 4 deletions

File tree

go/cmd/gitter/gitter.go

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,11 @@ func prepareCmd(ctx context.Context, dir string, env []string, name string, args
117117
}
118118
// Use SIGINT instead of SIGKILL for graceful shutdown of subprocesses
119119
cmd.Cancel = func() error {
120+
logger.DebugContext(ctx, "SIGINT sent to command", slog.String("cmd", name), slog.Any("args", args))
120121
return cmd.Process.Signal(syscall.SIGINT)
121122
}
123+
// Ensure it eventually dies if it ignores SIGINT
124+
cmd.WaitDelay = shutdownTimeout / 2
122125

123126
return cmd
124127
}
@@ -369,8 +372,15 @@ func gitHandler(w http.ResponseWriter, r *http.Request) {
369372
}
370373
}
371374

372-
semaphore <- struct{}{}
373-
defer func() { <-semaphore }()
375+
select {
376+
case semaphore <- struct{}{}:
377+
defer func() { <-semaphore }()
378+
case <-ctx.Done():
379+
logger.WarnContext(ctx, "Request cancelled while waiting for semaphore")
380+
http.Error(w, "Server context cancelled", http.StatusServiceUnavailable)
381+
382+
return
383+
}
374384
logger.DebugContext(ctx, "Concurrent requests", slog.Int("count", len(semaphore)))
375385

376386
// Fetch repo first
@@ -441,8 +451,15 @@ func cacheHandler(w http.ResponseWriter, r *http.Request) {
441451
ctx := context.WithValue(r.Context(), urlKey, url)
442452
logger.InfoContext(ctx, "Received request: /cache")
443453

444-
semaphore <- struct{}{}
445-
defer func() { <-semaphore }()
454+
select {
455+
case semaphore <- struct{}{}:
456+
defer func() { <-semaphore }()
457+
case <-ctx.Done():
458+
logger.WarnContext(ctx, "Request cancelled while waiting for semaphore")
459+
http.Error(w, "Server context cancelled", http.StatusServiceUnavailable)
460+
461+
return
462+
}
446463
logger.DebugContext(ctx, "Concurrent requests", slog.Int("count", len(semaphore)))
447464

448465
// Fetch repo if it's not fresh

go/cmd/gitter/repository.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,10 @@ func (r *Repository) buildCommitGraph(ctx context.Context, cache *pb.RepositoryC
146146

147147
scanner := bufio.NewScanner(file)
148148
for scanner.Scan() {
149+
// Handle context cancel within the loop to exit faster if we're processing a very large repo
150+
if ctx.Err() != nil {
151+
return nil, ctx.Err()
152+
}
149153
// Example of a line of git log output
150154
// 4c9bdbf0e2d45a5297cc080c3ebe809c0cca3581 bc0e4b4c4dbf7932fab7d264929d4d820e82c817 65be82edcdbc5aa6eeea23655cc96b5c84547d3b upstream/master, upstream/HEAD\n
151155
// Corresponds to: commit hash \t parent hashes (space delimited) \t refs (comma delimited)
@@ -316,6 +320,10 @@ func (r *Repository) calculatePatchIDsWorker(ctx context.Context, chunk []SHA1)
316320
go func() {
317321
defer in.Close()
318322
for _, hash := range chunk {
323+
// Handle context cancel
324+
if ctx.Err() != nil {
325+
return
326+
}
319327
fmt.Fprintf(in, "%s\n", hex.EncodeToString(hash[:]))
320328
}
321329
}()
@@ -330,6 +338,10 @@ func (r *Repository) calculatePatchIDsWorker(ctx context.Context, chunk []SHA1)
330338
// Read results from stdout of git patch-id
331339
scanner := bufio.NewScanner(out)
332340
for scanner.Scan() {
341+
// Handle context cancel to exit faster as this can be a long process
342+
if ctx.Err() != nil {
343+
return ctx.Err()
344+
}
333345
// Format of git patch-id result: <patch ID> <commit hash>
334346
line := scanner.Text()
335347

0 commit comments

Comments
 (0)