F

stateful · Jul 24, 2024 · a5dca03 · a5dca03
1 parent 25c095a
commit a5dca03
Show file tree

Hide file tree

Showing 5 changed files with 19 additions and 24 deletions.
diff --git a/internal/command/command_unix.go b/internal/command/command_unix.go
@@ -12,13 +12,14 @@ import (
 	"golang.org/x/sys/unix"
 )
 
-// func setSysProcAttrCtty(cmd *exec.Cmd) {
-// 	if cmd.SysProcAttr == nil {
-// 		cmd.SysProcAttr = &syscall.SysProcAttr{}
-// 	}
-// 	cmd.SysProcAttr.Setctty = true
-// 	cmd.SysProcAttr.Setsid = true
-// }
+func setSysProcAttrCtty(cmd *exec.Cmd, tty int) {
+	if cmd.SysProcAttr == nil {
+		cmd.SysProcAttr = &syscall.SysProcAttr{}
+	}
+	cmd.SysProcAttr.Ctty = tty
+	cmd.SysProcAttr.Setctty = true
+	cmd.SysProcAttr.Setsid = true
+}
 
 func setSysProcAttrPgid(cmd *exec.Cmd) {
 	if cmd.SysProcAttr == nil {

diff --git a/internal/command/command_unix_test.go b/internal/command/command_unix_test.go
@@ -482,7 +482,7 @@ func TestCommand_SimulateCtrlC(t *testing.T) {
 		require.NoError(t, err)
 	}
 
-	require.EqualError(t, cmd.Wait(), "signal: killed")
+	require.EqualError(t, cmd.Wait(), "exit status 130")
 }
 
 func TestCommand_StopWithSignal(t *testing.T) {

diff --git a/internal/command/command_virtual.go b/internal/command/command_virtual.go
@@ -81,13 +81,11 @@ func (c *virtualCommand) Start(ctx context.Context) (err error) {
 	c.cmd.Stdout = c.tty
 	c.cmd.Stderr = c.tty
 
-	// TODO(adamb): creating a new session and setting a controling terminal
-	// might be required; find the use case and then implement it properly.
-	// The current implementation misses setting Ctty.
-	// setSysProcAttrCtty(c.cmd)
-	// Creating a new process group is required to properly replicate a behaviour
-	// similar to CTRL-C in the terminal, which sends a SIGINT to the whole group.
-	setSysProcAttrPgid(c.cmd)
+	// Create a new session and set the controlling terminal to tty.
+	// The new process group is created automatically so that sending
+	// a signal to the command will affect the whole group.
+	setSysProcAttrCtty(c.cmd, 3) // 3 is the index of the i-th element in ExtraFiles
+	c.cmd.ExtraFiles = []*os.File{c.tty}
 
 	c.logger.Info("starting a virtual command", zap.Any("config", redactConfig(c.ProgramConfig())))
 	if err := c.cmd.Start(); err != nil {

diff --git a/internal/command/command_windows.go b/internal/command/command_windows.go
@@ -9,7 +9,7 @@ import (
 	"github.com/pkg/errors"
 )
 
-// func setSysProcAttrCtty(cmd *exec.Cmd) {}
+func setSysProcAttrCtty(cmd *exec.Cmd, tty int) {}
 
 func setSysProcAttrPgid(cmd *exec.Cmd) {}
 

diff --git a/internal/runnerv2service/service_execute_test.go b/internal/runnerv2service/service_execute_test.go
@@ -710,22 +710,18 @@ func TestRunnerServiceServerExecute_WithInput(t *testing.T) {
 
 		// cancel sleep
 		time.Sleep(time.Millisecond * 500)
-		err = stream.Send(&runnerv2alpha1.ExecuteRequest{InputData: []byte{0x04}})
+		err = stream.Send(&runnerv2alpha1.ExecuteRequest{InputData: []byte{0x03}})
 		assert.NoError(t, err)
 
-		// terminate outer shell and its children
 		time.Sleep(time.Millisecond * 500)
-		err = stream.Send(&runnerv2alpha1.ExecuteRequest{
-			// TODO(adamb): figure out why SIGINT does not work
-			Stop: runnerv2alpha1.ExecuteStop_EXECUTE_STOP_KILL,
-		})
+		err = stream.Send(&runnerv2alpha1.ExecuteRequest{InputData: []byte{0x04}})
 		assert.NoError(t, err)
 
 		result := <-execResult
 
 		// TODO(adamb): This should be a specific gRPC error rather than Unknown.
-		assert.Contains(t, result.Err.Error(), "signal: killed")
-		assert.Equal(t, 137, result.ExitCode)
+		assert.Contains(t, result.Err.Error(), "exit status 130")
+		assert.Equal(t, 130, result.ExitCode)
 	})
 
 	t.Run("CloseSendDirection", func(t *testing.T) {