From a6eaa450978791ccbdbc776d090e6eee118e024a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maximilian=20Pa=C3=9F?= <22845248+mpass99@users.noreply.github.com> Date: Tue, 23 Nov 2021 14:45:53 +0100 Subject: [PATCH] Stop stdout & stderr after timeout Co-authored-by: Sebastian Serth --- internal/api/websocket.go | 33 +++++++++++++++++++++++---------- internal/runner/runner.go | 12 ++++++++---- 2 files changed, 31 insertions(+), 14 deletions(-) diff --git a/internal/api/websocket.go b/internal/api/websocket.go index 475447d..bffba0d 100644 --- a/internal/api/websocket.go +++ b/internal/api/websocket.go @@ -151,23 +151,28 @@ func (cr *codeOceanToRawReader) Write(p []byte) (n int, err error) { type rawToCodeOceanWriter struct { proxy *webSocketProxy outputType dto.WebSocketMessageType + stopped bool } // Write implements the io.Writer interface. // The passed data is forwarded to the WebSocket to CodeOcean. func (rc *rawToCodeOceanWriter) Write(p []byte) (int, error) { + if rc.stopped { + return 0, nil + } err := rc.proxy.sendToClient(dto.WebSocketMessage{Type: rc.outputType, Data: string(p)}) return len(p), err } // webSocketProxy is an encapsulation of logic for forwarding between Runners and CodeOcean. type webSocketProxy struct { - userExit chan bool - connection webSocketConnection - connectionMu sync.Mutex - Stdin WebSocketReader - Stdout io.Writer - Stderr io.Writer + userExit chan bool + connection webSocketConnection + connectionMu sync.Mutex + Stdin WebSocketReader + Stdout io.Writer + Stderr io.Writer + cancelWebSocketWrite func() } // upgradeConnection upgrades a connection to a websocket and returns a webSocketProxy for this connection. @@ -190,8 +195,14 @@ func newWebSocketProxy(connection webSocketConnection) (*webSocketProxy, error) Stdin: stdin, userExit: make(chan bool), } - proxy.Stdout = &rawToCodeOceanWriter{proxy: proxy, outputType: dto.WebSocketOutputStdout} - proxy.Stderr = &rawToCodeOceanWriter{proxy: proxy, outputType: dto.WebSocketOutputStderr} + stdOut := &rawToCodeOceanWriter{proxy: proxy, outputType: dto.WebSocketOutputStdout} + stdErr := &rawToCodeOceanWriter{proxy: proxy, outputType: dto.WebSocketOutputStderr} + proxy.cancelWebSocketWrite = func() { + stdOut.stopped = true + stdErr.stopped = true + } + proxy.Stdout = stdOut + proxy.Stderr = stdErr err := proxy.sendToClient(dto.WebSocketMessage{Type: dto.WebSocketMetaStart}) if err != nil { @@ -216,9 +227,11 @@ func (wp *webSocketProxy) waitForExit(exit <-chan runner.ExitInfo, cancelExecuti select { case exitInfo = <-exit: cancelInputLoop() + wp.cancelWebSocketWrite() log.Info("Execution returned") case <-wp.userExit: cancelInputLoop() + wp.cancelWebSocketWrite() cancelExecution() log.Info("Client closed the connection") return @@ -265,8 +278,8 @@ func (wp *webSocketProxy) sendToClient(message dto.WebSocketMessage) error { } err = wp.writeMessage(websocket.TextMessage, encodedMessage) if err != nil { - errorMessage := "Error writing the exit message" - log.WithError(err).Warn(errorMessage) + errorMessage := "Error writing the message" + log.WithField("message", message).WithError(err).Warn(errorMessage) wp.closeWithError(errorMessage) return fmt.Errorf("error writing WebSocket message: %w", err) } diff --git a/internal/runner/runner.go b/internal/runner/runner.go index 9a7ea9d..0b99e1b 100644 --- a/internal/runner/runner.go +++ b/internal/runner/runner.go @@ -224,10 +224,14 @@ func (r *NomadJob) handleExitOrContextDone(ctx context.Context, cancelExecute co // (tty has to be true) and converted to a SIGQUIT signal sent to the foreground process attached to the tty. // By default, SIGQUIT causes the process to terminate and produces a core dump. Processes can catch this signal // and ignore it, which is why we destroy the runner if the process does not terminate after a grace period. - n, err := stdin.Write([]byte{SIGQUIT}) - if n != 1 { - log.WithField("runner", r.id).Warn("Could not send SIGQUIT because nothing was written") - } + _, err := stdin.Write([]byte{SIGQUIT}) + // if n != 1 { + // The SIGQUIT is sent and correctly processed by the allocation. However, for an unknown + // reason, the number of bytes written is always zero even though the error is nil. + // Hence, we disabled this sanity check here. See the MR for more details: + // https://github.com/openHPI/poseidon/pull/45#discussion_r757029024 + // log.WithField("runner", r.id).Warn("Could not send SIGQUIT because nothing was written") + // } if err != nil { log.WithField("runner", r.id).WithError(err).Warn("Could not send SIGQUIT due to error") }