Skip to content
Merged
Changes from all commits
Commits
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
36 changes: 30 additions & 6 deletions internal/jobcontainers/process.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,21 +92,39 @@ func (p *JobProcess) Signal(ctx context.Context, options interface{}) (bool, err
func (p *JobProcess) CloseStdin(ctx context.Context) error {
p.stdioLock.Lock()
defer p.stdioLock.Unlock()
return p.stdin.Close()
if p.stdin != nil {
if err := p.stdin.Close(); err != nil {
return errors.Wrap(err, "failed to close job container stdin")
}
p.stdin = nil
}
return nil
}

// CloseStdout closes the stdout pipe of the process.
func (p *JobProcess) CloseStdout(ctx context.Context) error {
p.stdioLock.Lock()
defer p.stdioLock.Unlock()
return p.stdout.Close()
if p.stdout != nil {
if err := p.stdout.Close(); err != nil {
return errors.Wrap(err, "failed to close job container stdout")
}
p.stdout = nil
}
return nil
}

// CloseStderr closes the stderr pipe of the process.
func (p *JobProcess) CloseStderr(ctx context.Context) error {
p.stdioLock.Lock()
defer p.stdioLock.Unlock()
return p.stderr.Close()
if p.stderr != nil {
if err := p.stderr.Close(); err != nil {
return errors.Wrap(err, "failed to close job container stderr")
}
p.stderr = nil
}
return nil
}

// Wait waits for the process to exit. If the process has already exited returns
Expand Down Expand Up @@ -217,7 +235,9 @@ func signalProcess(pid uint32, signal int) error {
if err != nil {
return errors.Wrap(err, "failed to open process")
}
defer windows.Close(hProc)
defer func() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These could just be left as the original defer windows.Close(hproc) with no functionality change.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think our linter was whining about it as we weren't handling the error, but begs the question of how it didn't whine the first time..

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Let me look what we can get away with

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

internal\jobcontainers\process.go:252:27: Error return value of windows.FreeLibrary is not checked (errcheck)
defer windows.FreeLibrary(k32)

Seems to only care about the freelibrary for some odd reason

_ = windows.Close(hProc)
}()

// We can't use GenerateConsoleCtrlEvent since that only supports CTRL_C_EVENT and CTRL_BREAK_EVENT.
// Instead, to handle an arbitrary signal we open a CtrlRoutine thread inside the target process and
Expand All @@ -231,7 +251,9 @@ func signalProcess(pid uint32, signal int) error {
if err != nil {
return errors.Wrap(err, "failed to load kernel32 library")
}
defer windows.Close(k32)
defer func() {
_ = windows.FreeLibrary(k32)
}()

proc, err := windows.GetProcAddress(k32, "CtrlRoutine")
if err != nil {
Expand All @@ -242,6 +264,8 @@ func signalProcess(pid uint32, signal int) error {
if err != nil {
return errors.Wrapf(err, "failed to open remote thread in target process %d", pid)
}
defer windows.Close(threadHandle)
defer func() {
_ = windows.Close(threadHandle)
}()
return nil
}