Skip to content

Lock to OS thread when Pdeathsig is set #86

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

Merged
merged 1 commit into from
Oct 7, 2022
Merged

Lock to OS thread when Pdeathsig is set #86

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
49 changes: 45 additions & 4 deletions monitor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package runc

import (
"os/exec"
"runtime"
"syscall"
"time"
)
Expand All @@ -32,15 +33,18 @@ type Exit struct {
Status int
}

// ProcessMonitor is an interface for process monitoring
// ProcessMonitor is an interface for process monitoring.
//
// It allows daemons using go-runc to have a SIGCHLD handler
// to handle exits without introducing races between the handler
// and go's exec.Cmd
// These methods should match the methods exposed by exec.Cmd to provide
// a consistent experience for the caller
// and go's exec.Cmd.
//
// ProcessMonitor also provides a StartLocked method which is similar to
// Start, but locks the goroutine used to start the process to an OS thread
// (for example: when Pdeathsig is set).
type ProcessMonitor interface {
Start(*exec.Cmd) (chan Exit, error)
StartLocked(*exec.Cmd) (chan Exit, error)
Wait(*exec.Cmd, chan Exit) (int, error)
}

Expand Down Expand Up @@ -72,6 +76,43 @@ func (m *defaultMonitor) Start(c *exec.Cmd) (chan Exit, error) {
return ec, nil
}

// StartLocked is like Start, but locks the goroutine used to start the process to
// the OS thread for use-cases where the parent thread matters to the child process
// (for example: when Pdeathsig is set).
func (m *defaultMonitor) StartLocked(c *exec.Cmd) (chan Exit, error) {
started := make(chan error)
ec := make(chan Exit, 1)
go func() {
runtime.LockOSThread()
defer runtime.UnlockOSThread()

if err := c.Start(); err != nil {
started <- err
return
}
close(started)
var status int
if err := c.Wait(); err != nil {
status = 255
if exitErr, ok := err.(*exec.ExitError); ok {
if ws, ok := exitErr.Sys().(syscall.WaitStatus); ok {
status = ws.ExitStatus()
}
}
}
ec <- Exit{
Timestamp: time.Now(),
Pid: c.Process.Pid,
Status: status,
}
close(ec)
}()
if err := <-started; err != nil {
return nil, err
}
return ec, nil
}

func (m *defaultMonitor) Wait(c *exec.Cmd, ec chan Exit) (int, error) {
e := <-ec
return e.Status, nil
Expand Down
62 changes: 40 additions & 22 deletions runc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,22 @@ const (
type Runc struct {
// Command overrides the name of the runc binary. If empty, DefaultCommand
// is used.
Command string
Root string
Debug bool
Log string
LogFormat Format
Command string
Root string
Debug bool
Log string
LogFormat Format
// PdeathSignal sets a signal the child process will receive when the
// parent dies.
//
// When Pdeathsig is set, command invocations will call runtime.LockOSThread
// to prevent OS thread termination from spuriously triggering the
// signal. See https://github.com/golang/go/issues/27505 and
// https://github.com/golang/go/blob/126c22a09824a7b52c019ed9a1d198b4e7781676/src/syscall/exec_linux.go#L48-L51
//
// A program with GOMAXPROCS=1 might hang because of the use of
// runtime.LockOSThread. Callers should ensure they retain at least one
// unlocked thread.
PdeathSignal syscall.Signal // using syscall.Signal to allow compilation on non-unix (unix.Syscall is an alias for syscall.Signal)
Setpgid bool

Expand All @@ -83,7 +94,7 @@ type Runc struct {

// List returns all containers created inside the provided runc root directory
func (r *Runc) List(context context.Context) ([]*Container, error) {
data, err := cmdOutput(r.command(context, "list", "--format=json"), false, nil)
data, err := r.cmdOutput(r.command(context, "list", "--format=json"), false, nil)
defer putBuf(data)
if err != nil {
return nil, err
Expand All @@ -97,7 +108,7 @@ func (r *Runc) List(context context.Context) ([]*Container, error) {

// State returns the state for the container provided by id
func (r *Runc) State(context context.Context, id string) (*Container, error) {
data, err := cmdOutput(r.command(context, "state", id), true, nil)
data, err := r.cmdOutput(r.command(context, "state", id), true, nil)
defer putBuf(data)
if err != nil {
return nil, fmt.Errorf("%s: %s", err, data.String())
Expand Down Expand Up @@ -157,6 +168,13 @@ func (o *CreateOpts) args() (out []string, err error) {
return out, nil
}

func (r *Runc) startCommand(cmd *exec.Cmd) (chan Exit, error) {
if r.PdeathSignal != 0 {
return Monitor.StartLocked(cmd)
}
return Monitor.Start(cmd)
}

// Create creates a new container and returns its pid if it was created successfully
func (r *Runc) Create(context context.Context, id, bundle string, opts *CreateOpts) error {
args := []string{"create", "--bundle", bundle}
Expand All @@ -174,14 +192,14 @@ func (r *Runc) Create(context context.Context, id, bundle string, opts *CreateOp
cmd.ExtraFiles = opts.ExtraFiles

if cmd.Stdout == nil && cmd.Stderr == nil {
data, err := cmdOutput(cmd, true, nil)
data, err := r.cmdOutput(cmd, true, nil)
defer putBuf(data)
if err != nil {
return fmt.Errorf("%s: %s", err, data.String())
}
return nil
}
ec, err := Monitor.Start(cmd)
ec, err := r.startCommand(cmd)
if err != nil {
return err
}
Expand Down Expand Up @@ -263,14 +281,14 @@ func (r *Runc) Exec(context context.Context, id string, spec specs.Process, opts
opts.Set(cmd)
}
if cmd.Stdout == nil && cmd.Stderr == nil {
data, err := cmdOutput(cmd, true, opts.Started)
data, err := r.cmdOutput(cmd, true, opts.Started)
defer putBuf(data)
if err != nil {
return fmt.Errorf("%w: %s", err, data.String())
}
return nil
}
ec, err := Monitor.Start(cmd)
ec, err := r.startCommand(cmd)
if err != nil {
return err
}
Expand Down Expand Up @@ -309,7 +327,7 @@ func (r *Runc) Run(context context.Context, id, bundle string, opts *CreateOpts)
if opts != nil && opts.IO != nil {
opts.Set(cmd)
}
ec, err := Monitor.Start(cmd)
ec, err := r.startCommand(cmd)
if err != nil {
return -1, err
}
Expand Down Expand Up @@ -382,7 +400,7 @@ func (r *Runc) Stats(context context.Context, id string) (*Stats, error) {
if err != nil {
return nil, err
}
ec, err := Monitor.Start(cmd)
ec, err := r.startCommand(cmd)
if err != nil {
return nil, err
}
Expand All @@ -404,7 +422,7 @@ func (r *Runc) Events(context context.Context, id string, interval time.Duration
if err != nil {
return nil, err
}
ec, err := Monitor.Start(cmd)
ec, err := r.startCommand(cmd)
if err != nil {
rd.Close()
return nil, err
Expand Down Expand Up @@ -448,7 +466,7 @@ func (r *Runc) Resume(context context.Context, id string) error {

// Ps lists all the processes inside the container returning their pids
func (r *Runc) Ps(context context.Context, id string) ([]int, error) {
data, err := cmdOutput(r.command(context, "ps", "--format", "json", id), true, nil)
data, err := r.cmdOutput(r.command(context, "ps", "--format", "json", id), true, nil)
defer putBuf(data)
if err != nil {
return nil, fmt.Errorf("%s: %s", err, data.String())
Expand All @@ -462,7 +480,7 @@ func (r *Runc) Ps(context context.Context, id string) ([]int, error) {

// Top lists all the processes inside the container returning the full ps data
func (r *Runc) Top(context context.Context, id string, psOptions string) (*TopResults, error) {
data, err := cmdOutput(r.command(context, "ps", "--format", "table", id, psOptions), true, nil)
data, err := r.cmdOutput(r.command(context, "ps", "--format", "table", id, psOptions), true, nil)
defer putBuf(data)
if err != nil {
return nil, fmt.Errorf("%s: %s", err, data.String())
Expand Down Expand Up @@ -647,7 +665,7 @@ func (r *Runc) Restore(context context.Context, id, bundle string, opts *Restore
if opts != nil && opts.IO != nil {
opts.Set(cmd)
}
ec, err := Monitor.Start(cmd)
ec, err := r.startCommand(cmd)
if err != nil {
return -1, err
}
Expand Down Expand Up @@ -691,7 +709,7 @@ type Version struct {

// Version returns the runc and runtime-spec versions
func (r *Runc) Version(context context.Context) (Version, error) {
data, err := cmdOutput(r.command(context, "--version"), false, nil)
data, err := r.cmdOutput(r.command(context, "--version"), false, nil)
defer putBuf(data)
if err != nil {
return Version{}, err
Expand Down Expand Up @@ -753,7 +771,7 @@ func (r *Runc) args() (out []string) {
// <stderr>
func (r *Runc) runOrError(cmd *exec.Cmd) error {
if cmd.Stdout != nil || cmd.Stderr != nil {
ec, err := Monitor.Start(cmd)
ec, err := r.startCommand(cmd)
if err != nil {
return err
}
Expand All @@ -763,7 +781,7 @@ func (r *Runc) runOrError(cmd *exec.Cmd) error {
}
return err
}
data, err := cmdOutput(cmd, true, nil)
data, err := r.cmdOutput(cmd, true, nil)
defer putBuf(data)
if err != nil {
return fmt.Errorf("%s: %s", err, data.String())
Expand All @@ -773,14 +791,14 @@ func (r *Runc) runOrError(cmd *exec.Cmd) error {

// callers of cmdOutput are expected to call putBuf on the returned Buffer
// to ensure it is released back to the shared pool after use.
func cmdOutput(cmd *exec.Cmd, combined bool, started chan<- int) (*bytes.Buffer, error) {
func (r *Runc) cmdOutput(cmd *exec.Cmd, combined bool, started chan<- int) (*bytes.Buffer, error) {
b := getBuf()

cmd.Stdout = b
if combined {
cmd.Stderr = b
}
ec, err := Monitor.Start(cmd)
ec, err := r.startCommand(cmd)
if err != nil {
return nil, err
}
Expand Down