Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Communicate process kill signals without being blocked by stdout/err reads #472

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+70 −15
Open

Communicate process kill signals without being blocked by stdout/err reads #472

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
02c1fab
use channels to communicate sig kill/term w/o potential blocking read
0div Mar 19, 2025
1d8842f
make local envd more similar to what provision.sh does in template-ma…
0div Mar 19, 2025
6583c37
Merge branch 'main' into killing-command-by-pid-may-not-work-as-expec…
0div Mar 19, 2025
2274e65
Merge branch 'main' into killing-command-by-pid-may-not-work-as-expec…
0div Mar 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion packages/envd/debug.Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,22 @@ FROM golang:1.23
ARG DEBIAN_FRONTEND=noninteractive

RUN apt-get update
RUN apt-get install systemd ca-certificates make python-is-python3 python3 nodejs chrony sudo -y
RUN apt-get install systemd ca-certificates make python-is-python3 python3 nodejs npm chrony sudo -y

RUN update-ca-certificates

# Remove password for root.
RUN passwd -d root

# Create default user.
RUN useradd -ms /bin/bash user
RUN usermod -aG sudo user
RUN passwd -d user
RUN echo "user ALL=(ALL:ALL) NOPASSWD: ALL" >>/etc/sudoers

RUN mkdir -p /home/user
RUN chmod 777 -R /home/user
RUN chmod 777 -R /usr/local
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

chmod 777 seems excessive, but i'm low context, you might be better of with chown -R user /home/user and chown -R user /user/local

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just mirroring what is done in provision script used in template manager, doesn't invalidate your comment tho—here's it's solely for local dev, however in template manager it's used for templates


RUN go install github.com/go-delve/delve/cmd/dlv@latest

Expand Down
67 changes: 55 additions & 12 deletions packages/envd/internal/services/process/handler/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ type Handler struct {
cmd *exec.Cmd
tty *os.File

SignalChan chan syscall.Signal

outWg *sync.WaitGroup
stdin io.WriteCloser

Expand All @@ -63,6 +65,17 @@ func New(user *user.User, req *rpc.StartRequest, logger *zerolog.Logger, envVars
return nil, connect.NewError(connect.CodeInternal, err)
}

signalChan := make(chan syscall.Signal, 1)
shutdownChan := make(chan bool)

go func() {
signal := <-signalChan
if signal == syscall.SIGKILL || signal == syscall.SIGTERM {
// closing the shutdownChan allows us to broadcast the signal to all the read loops below
close(shutdownChan)
}
}()

cmd.SysProcAttr = &syscall.SysProcAttr{}
cmd.SysProcAttr.Credential = &syscall.Credential{
Uid: uid,
Expand Down Expand Up @@ -172,15 +185,27 @@ func New(user *user.User, req *rpc.StartRequest, logger *zerolog.Logger, envVars

outWg.Add(1)
go func() {
defer outWg.Done()

stdoutLogs := make(chan []byte, outputBufferSize)
defer close(stdoutLogs)

readOver := make(chan bool, 1)
stdoutLogger := logger.With().Str("event_type", "stdout").Logger()

go logs.LogBufferedDataEvents(stdoutLogs, &stdoutLogger, "data")

go func() {
select {
case <-readOver:
outWg.Done()
break
case _, ok := <-shutdownChan:
if !ok {
outWg.Done()
}
break
}
}()

for {
buf := make([]byte, stdChunkSize)

Expand Down Expand Up @@ -208,6 +233,8 @@ func New(user *user.User, req *rpc.StartRequest, logger *zerolog.Logger, envVars
break
}
}

readOver <- true
}()

stderr, err := cmd.StderrPipe()
Expand All @@ -217,15 +244,28 @@ func New(user *user.User, req *rpc.StartRequest, logger *zerolog.Logger, envVars

outWg.Add(1)
go func() {
defer outWg.Done()

stderrLogs := make(chan []byte, outputBufferSize)
defer close(stderrLogs)

readOver := make(chan bool, 1)

stderrLogger := logger.With().Str("event_type", "stderr").Logger()

go logs.LogBufferedDataEvents(stderrLogs, &stderrLogger, "data")

go func() {
select {
case <-readOver:
outWg.Done()
break
case _, ok := <-shutdownChan:
if !ok {
outWg.Done()
}
break
}
}()

for {
buf := make([]byte, stdChunkSize)

Expand Down Expand Up @@ -253,17 +293,20 @@ func New(user *user.User, req *rpc.StartRequest, logger *zerolog.Logger, envVars
break
}
}

readOver <- true
}()

return &Handler{
Config: req.GetProcess(),
cmd: cmd,
stdin: stdin,
Tag: req.Tag,
DataEvent: outMultiplex,
outWg: &outWg,
EndEvent: NewMultiplexedChannel[rpc.ProcessEvent_End](0),
logger: logger,
Config: req.GetProcess(),
cmd: cmd,
stdin: stdin,
Tag: req.Tag,
DataEvent: outMultiplex,
outWg: &outWg,
EndEvent: NewMultiplexedChannel[rpc.ProcessEvent_End](0),
SignalChan: signalChan,
logger: logger,
}, nil
}

Expand Down
5 changes: 3 additions & 2 deletions packages/envd/internal/services/process/signal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
)

func (s *Service) SendSignal(ctx context.Context, req *connect.Request[rpc.SendSignalRequest]) (*connect.Response[rpc.SendSignalResponse], error) {
proc, err := s.getProcess(req.Msg.Process)
handler, err := s.getProcess(req.Msg.Process)
if err != nil {
return nil, err
}
Expand All @@ -26,7 +26,8 @@ func (s *Service) SendSignal(ctx context.Context, req *connect.Request[rpc.SendS
return nil, connect.NewError(connect.CodeUnimplemented, fmt.Errorf("invalid signal: %s", req.Msg.GetSignal()))
}

err = proc.SendSignal(signal)
handler.SignalChan <- signal
err = handler.SendSignal(signal)
if err != nil {
return nil, connect.NewError(connect.CodeInternal, fmt.Errorf("error sending signal: %w", err))
}
Expand Down
Loading