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

Jump to bottom

fix: actually turn off child subreaping in reaper.Stop #454

Merged
merged 2 commits into from
Jul 19, 2024
Merged

fix: actually turn off child subreaping in reaper.Stop #454

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5c87e47
fix: actually turn off child subreaping in reaper.Stop
benhoyt Jul 17, 2024
1b23fd1
Disable receiving of SIGCHLD before we stop the loop that handles them
benhoyt Jul 19, 2024
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
34 changes: 19 additions & 15 deletions internals/reaper/reaper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package reaper

import (
"bytes"
"errors"
"fmt"
"os"
"os/exec"
Expand Down Expand Up @@ -45,12 +46,9 @@ func Start() error {
return nil // already started
}

isSubreaper, err := setChildSubreaper()
err := setChildSubreaper(1)
if err != nil {
return fmt.Errorf("cannot set child subreaper: %w", err)
}
if !isSubreaper {
return fmt.Errorf("child subreaping unavailable on this platform")
return err
}

started = true
Expand All @@ -75,22 +73,28 @@ func Stop() error {
started = false
mutex.Unlock()

err := setChildSubreaper(0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be theoretically safer to disable receiving unix.SIGCHLD signals, before we terminate the processing loop that deals with them ?

if err != nil {
return err
}

return nil
}

// setChildSubreaper sets the current process as a "child subreaper" so we
// become the parent of dead child processes rather than PID 1. This allows us
// to wait for processes that are started by a Pebble service but then die, to
// "reap" them (see https://unix.stackexchange.com/a/250156/73491).
// setChildSubreaper sets the "child subreaper" attribute of the current
// process, turning it on if the argument is nonzero, off otherwise.
//
// The function returns true if sub-reaping is available (Linux 3.4+) along
// with an error if it's available but can't be set.
func setChildSubreaper() (bool, error) {
err := unix.Prctl(unix.PR_SET_CHILD_SUBREAPER, 1, 0, 0, 0)
// If turning it on, we become the parent of dead child processes rather than
// PID 1. This allows us to wait for processes that are started by a Pebble
// service but then die, to "reap" them (see
// https://unix.stackexchange.com/a/250156/73491).
func setChildSubreaper(set int) error {
err := unix.Prctl(unix.PR_SET_CHILD_SUBREAPER, uintptr(set), 0, 0, 0)
if err == unix.EINVAL {
return false, nil
// Not available in kernels before Linux 3.4.
return errors.New("child subreaping unavailable on this platform")
}
return true, err
return err
}

// reapChildren "reaps" (waits for) child processes whose parents didn't
Expand Down
Loading