Skip to content

Commit

Permalink
Lock to OS thread when Pdeathsig is set
Browse files Browse the repository at this point in the history 
See golang/go#27505 for context. Pdeathsig
isn't safe to set without locking to the current OS thread, because
otherwise thread termination will send the signal, which isn't the
desired behavior.

I discovered this while troubleshooting a problem that turned out to be
unrelated, but I think it's necessary for correctness.

Signed-off-by: Aaron Lehmann <[email protected]>
  • Loading branch information
@aaronlehmann
aaronlehmann committed Jul 20, 2022
1 parent 2e979ca commit a9d1516
Showing 1 changed file with 54 additions and 14 deletions.
68 changes: 54 additions & 14 deletions runc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"os"
"os/exec"
"path/filepath"
"runtime"
"strconv"
"strings"
"syscall"
Expand Down Expand Up @@ -61,11 +62,18 @@ 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
// Pdeathsig sets a signal the child process will receive when the
// parent dies.
//
// When Pdeathsig is set, command invocations will call os.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
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 +91,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 +105,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 @@ -174,13 +182,17 @@ 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
}
if r.PdeathSignal != 0 {
runtime.LockOSThread()
defer runtime.UnlockOSThread()
}
ec, err := Monitor.Start(cmd)
if err != nil {
return err
Expand Down Expand Up @@ -263,13 +275,17 @@ 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
}
if r.PdeathSignal != 0 {
runtime.LockOSThread()
defer runtime.UnlockOSThread()
}
ec, err := Monitor.Start(cmd)
if err != nil {
return err
Expand Down Expand Up @@ -309,6 +325,10 @@ func (r *Runc) Run(context context.Context, id, bundle string, opts *CreateOpts)
if opts != nil && opts.IO != nil {
opts.Set(cmd)
}
if r.PdeathSignal != 0 {
runtime.LockOSThread()
defer runtime.UnlockOSThread()
}
ec, err := Monitor.Start(cmd)
if err != nil {
return -1, err
Expand Down Expand Up @@ -382,6 +402,10 @@ func (r *Runc) Stats(context context.Context, id string) (*Stats, error) {
if err != nil {
return nil, err
}
if r.PdeathSignal != 0 {
runtime.LockOSThread()
defer runtime.UnlockOSThread()
}
ec, err := Monitor.Start(cmd)
if err != nil {
return nil, err
Expand All @@ -404,6 +428,10 @@ func (r *Runc) Events(context context.Context, id string, interval time.Duration
if err != nil {
return nil, err
}
if r.PdeathSignal != 0 {
runtime.LockOSThread()
defer runtime.UnlockOSThread()
}
ec, err := Monitor.Start(cmd)
if err != nil {
rd.Close()
Expand Down Expand Up @@ -448,7 +476,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 +490,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,6 +675,10 @@ func (r *Runc) Restore(context context.Context, id, bundle string, opts *Restore
if opts != nil && opts.IO != nil {
opts.Set(cmd)
}
if r.PdeathSignal != 0 {
runtime.LockOSThread()
defer runtime.UnlockOSThread()
}
ec, err := Monitor.Start(cmd)
if err != nil {
return -1, err
Expand Down Expand Up @@ -691,7 +723,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,6 +785,10 @@ func (r *Runc) args() (out []string) {
// <stderr>
func (r *Runc) runOrError(cmd *exec.Cmd) error {
if cmd.Stdout != nil || cmd.Stderr != nil {
if r.PdeathSignal != 0 {
runtime.LockOSThread()
defer runtime.UnlockOSThread()
}
ec, err := Monitor.Start(cmd)
if err != nil {
return err
Expand All @@ -763,7 +799,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,13 +809,17 @@ 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
}
if r.PdeathSignal != 0 {
runtime.LockOSThread()
defer runtime.UnlockOSThread()
}
ec, err := Monitor.Start(cmd)
if err != nil {
return nil, err
Expand Down

0 comments on commit a9d1516

Please sign in to comment.