diff --git a/src/frontend/xcecli/cmd/jobs/submit.go b/src/frontend/xcecli/cmd/jobs/submit.go index 86134c46..5c15efba 100755 --- a/src/frontend/xcecli/cmd/jobs/submit.go +++ b/src/frontend/xcecli/cmd/jobs/submit.go @@ -102,9 +102,16 @@ func (js *JobSubmitter) ParseCmdArgs(cmd *cobra.Command, args []string) error { } } - // check if there is standard input redirection + // check if there is standard input redirection -- but only when the + // command line carries no command of its own. A piped stdin is the job + // script (bsub < script semantics, parsed as #xce job submit lines by + // parseConfigFile below); an explicit command on the command line wins + // and stdin must not be touched at all: reading it would block forever + // on a pipe that stays open without writing data (bug-1980 -- a hung + // submit observed in live use), and any stdin bytes would be silently + // reinterpreted as a config script next to the explicit command. fileInfo, err := os.Stdin.Stat() - if err == nil && (fileInfo.Mode()&os.ModeCharDevice) == 0 { + if err == nil && len(args) == 0 && (fileInfo.Mode()&os.ModeCharDevice) == 0 { if tmpFileName, err := js.handleStdin(); err != nil { errMsg := request.ExtractErrorMessage(err) return fmt.Errorf("error reading from stdin: [%s]", errMsg) diff --git a/src/frontend/xcecli/cmd/jobs/submit_test.go b/src/frontend/xcecli/cmd/jobs/submit_test.go index d2ac9466..fc83f823 100644 --- a/src/frontend/xcecli/cmd/jobs/submit_test.go +++ b/src/frontend/xcecli/cmd/jobs/submit_test.go @@ -376,3 +376,38 @@ func TestWaitForInteractiveURL_SuspendedJobKeepsWaiting(t *testing.T) { t.Errorf("url = %q, want http://h:9000", url) } } + +// bug-1980: `jobs submit ` with a piped stdin used to block forever +// when the pipe stayed open with no data (heredoc quoting accidents, generic +// tooling that keeps a stdin pipe around): ParseCmdArgs treated ANY +// non-terminal stdin as a potential job script and io.ReadAll'ed it before +// looking at the command args. LSF (bsub) semantics the CLI claims to follow +// are the reverse: stdin is the job script only when no command is given on +// the command line -- an explicit command must win and never touch stdin. +func TestParseCmdArgs_ExplicitCommandIgnoresHeldOpenStdin(t *testing.T) { + pr, pw, err := os.Pipe() + if err != nil { + t.Fatalf("pipe: %v", err) + } + defer pr.Close() //nolint:errcheck // read end: closed after Parse returns + t.Cleanup(func() { _ = pw.Close() }) + // Keep the write end open for the duration of the call: the pre-fix code + // blocks in io.ReadAll here; the post-fix code must not read stdin at all. + + oldStdin := os.Stdin + os.Stdin = pr + t.Cleanup(func() { os.Stdin = oldStdin }) + + js := NewJobSubmitter(nil) + cmd := NewSubmitJobsCmd(nil) + parseErr := js.ParseCmdArgs(cmd, []string{"sleep", "10"}) + if parseErr != nil { + t.Fatalf("ParseCmdArgs with explicit command and held-open stdin: %v", parseErr) + } + if js.command != "sleep 10" { + t.Errorf("command = %q, want %q", js.command, "sleep 10") + } + if strings.Contains(js.configFile, "xcecli_stdin_") { + t.Errorf("stdin was consumed as a config file (%s) despite an explicit command", js.configFile) + } +}