From 219d25edf6fb3fe814818c9b8aaa5e47b4f17d8c Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Thu, 23 Jun 2022 10:27:20 +0200 Subject: [PATCH 1/3] Cancel reading from stdin on Ctrl-C This is essentially a copy-paste implementation of the ideas presented in this comment here: https://github.com/golang/go/issues/20280#issuecomment-655588450 It fixes #775 and helps with the issue described in https://github.com/sourcegraph/src-cli/pull/793#issuecomment-1163310725. Not sure if it has unintended side-effects. --- cmd/src/batch_common.go | 29 +++++++++++++++++++++++++---- cmd/src/batch_remote.go | 2 +- cmd/src/batch_repositories.go | 2 +- cmd/src/batch_validate.go | 2 +- 4 files changed, 28 insertions(+), 7 deletions(-) diff --git a/cmd/src/batch_common.go b/cmd/src/batch_common.go index 7b0a9004c5..0eaa394178 100644 --- a/cmd/src/batch_common.go +++ b/cmd/src/batch_common.go @@ -12,6 +12,7 @@ import ( "path/filepath" "runtime" "strings" + "syscall" "time" "github.com/mattn/go-isatty" @@ -220,7 +221,7 @@ func batchDefaultTempDirPrefix() string { return os.TempDir() } -func batchOpenFileFlag(flag string) (io.ReadCloser, error) { +func batchOpenFileFlag(flag string) (*os.File, error) { if flag == "" || flag == "-" { if flag != "-" { // If the flag wasn't set, we want to check stdin. If it's not a TTY, @@ -238,8 +239,13 @@ func batchOpenFileFlag(flag string) (io.ReadCloser, error) { } } } + // https://github.com/golang/go/issues/24842 + if err := syscall.SetNonblock(0, true); err != nil { + panic(err) + } + stdin := os.NewFile(0, "stdin") - return os.Stdin, nil + return stdin, nil } file, err := os.Open(flag) @@ -292,7 +298,7 @@ func executeBatchSpec(ctx context.Context, ui ui.ExecUI, opts executeBatchSpecOp // Parse flags and build up our service and executor options. ui.ParsingBatchSpec() - batchSpec, rawSpec, err := parseBatchSpec(opts.file, svc, false) + batchSpec, rawSpec, err := parseBatchSpec(ctx, opts.file, svc, false) if err != nil { var multiErr errors.MultiError if errors.As(err, &multiErr) { @@ -481,18 +487,33 @@ func executeBatchSpec(ctx context.Context, ui ui.ExecUI, opts executeBatchSpecOp return nil } +type ReadDeadliner interface { + SetReadDeadline(t time.Time) error +} + +func SetReadDeadlineOnCancel(ctx context.Context, d ReadDeadliner) { + go func() { + <-ctx.Done() + d.SetReadDeadline(time.Now()) + }() +} + // parseBatchSpec parses and validates the given batch spec. If the spec has // validation errors, they are returned. // // isRemote argument is a temporary argument used to determine if the batch spec is being parsed for remote // (server-side) processing. Remote processing does not support mounts yet. -func parseBatchSpec(file string, svc *service.Service, isRemote bool) (*batcheslib.BatchSpec, string, error) { +func parseBatchSpec(ctx context.Context, file string, svc *service.Service, isRemote bool) (*batcheslib.BatchSpec, string, error) { f, err := batchOpenFileFlag(file) if err != nil { return nil, "", err } defer f.Close() + ctx, cancel := context.WithCancel(ctx) + defer cancel() + SetReadDeadlineOnCancel(ctx, f) + data, err := io.ReadAll(f) if err != nil { return nil, "", errors.Wrap(err, "reading batch spec") diff --git a/cmd/src/batch_remote.go b/cmd/src/batch_remote.go index 9d542ae9b5..261831c11c 100644 --- a/cmd/src/batch_remote.go +++ b/cmd/src/batch_remote.go @@ -63,7 +63,7 @@ Examples: // may as well validate it at the same time so we don't even have to go to // the backend if it's invalid. ui.ParsingBatchSpec() - spec, raw, err := parseBatchSpec(file, svc, true) + spec, raw, err := parseBatchSpec(ctx, file, svc, true) if err != nil { ui.ParsingBatchSpecFailure(err) return err diff --git a/cmd/src/batch_repositories.go b/cmd/src/batch_repositories.go index 83beb40915..8adbfd9643 100644 --- a/cmd/src/batch_repositories.go +++ b/cmd/src/batch_repositories.go @@ -75,7 +75,7 @@ Examples: } out := output.NewOutput(flagSet.Output(), output.OutputOpts{Verbose: *verbose}) - spec, _, err := parseBatchSpec(file, svc, false) + spec, _, err := parseBatchSpec(ctx, file, svc, false) if err != nil { ui := &ui.TUI{Out: out} ui.ParsingBatchSpecFailure(err) diff --git a/cmd/src/batch_validate.go b/cmd/src/batch_validate.go index 3d40a8caca..db0e9d73e4 100644 --- a/cmd/src/batch_validate.go +++ b/cmd/src/batch_validate.go @@ -75,7 +75,7 @@ Examples: return err } - if _, _, err := parseBatchSpec(file, svc, false); err != nil { + if _, _, err := parseBatchSpec(ctx, file, svc, false); err != nil { ui.ParsingBatchSpecFailure(err) return err } From c0d370452ae97dcfea2f3d376cd770598a30736e Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Thu, 7 Jul 2022 14:58:00 +0200 Subject: [PATCH 2/3] Add comments and remove interface --- cmd/src/batch_common.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/cmd/src/batch_common.go b/cmd/src/batch_common.go index 0eaa394178..7b47d0556d 100644 --- a/cmd/src/batch_common.go +++ b/cmd/src/batch_common.go @@ -487,14 +487,12 @@ func executeBatchSpec(ctx context.Context, ui ui.ExecUI, opts executeBatchSpecOp return nil } -type ReadDeadliner interface { - SetReadDeadline(t time.Time) error -} - -func SetReadDeadlineOnCancel(ctx context.Context, d ReadDeadliner) { +func setReadDeadlineOnCancel(ctx context.Context, f *os.File) { go func() { + // When user cancels, we set the read deadline to now() so the runtime + // cancels the read and we don't block. <-ctx.Done() - d.SetReadDeadline(time.Now()) + f.SetReadDeadline(time.Now()) }() } @@ -510,9 +508,11 @@ func parseBatchSpec(ctx context.Context, file string, svc *service.Service, isRe } defer f.Close() + // Create new ctx so we ensure that the goroutine in + // setReadDeadlineOnCancel exits at end of function. ctx, cancel := context.WithCancel(ctx) defer cancel() - SetReadDeadlineOnCancel(ctx, f) + setReadDeadlineOnCancel(ctx, f) data, err := io.ReadAll(f) if err != nil { From 964c1183ac61462dcb1896c1839e9f9b0caa5838 Mon Sep 17 00:00:00 2001 From: BolajiOlajide <25608335+BolajiOlajide@users.noreply.github.com> Date: Thu, 7 Jul 2022 15:54:06 +0100 Subject: [PATCH 3/3] update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 161fede045..28a2ed9933 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,8 @@ All notable changes to `src-cli` are documented in this file. ### Fixed +- Handle SIGINT interrupt when reading from Stdin. [#794](https://github.com/sourcegraph/src-cli/pull/794) + ### Removed ## 3.41.0