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.

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

survive bereaving #141

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
95 changes: 78 additions & 17 deletions dumb-init.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
#include <sys/types.h>
#include <sys/wait.h>
#include <unistd.h>
#include <dirent.h>
#include <ctype.h>
#include "VERSION.h"

#define PRINTERR(...) do { \
Expand All @@ -42,6 +44,7 @@ int signal_rewrite[MAXSIG + 1] = {[0 ... MAXSIG] = -1};
pid_t child_pid = -1;
char debug = 0;
char use_setsid = 1;
static char survive_bereaving = 0;

int translate_signal(int signum) {
if (signum <= 0 || signum > MAXSIG) {
Expand All @@ -67,6 +70,43 @@ void forward_signal(int signum) {
}
}

/*
* Read /proc and see if there are processes except init(PIDs)
*/
signed int process_count() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this isn't exactly returning the total process count, maybe it'd be better to rename this function to is_only_process_running?

DIR *dp;
struct dirent *ep;
char nonnumber;
signed int count = 0;

dp = opendir ("/proc");
if (dp != NULL)
{
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: formatting...

while ((ep = readdir (dp)) != NULL) {
nonnumber = 0;
for (int i = 0; ep->d_name[i] != 0; ++i) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I think it'd be slightly more clear to write ep->d_name[i] != '\0'

if (!isdigit(ep->d_name[i])) {
nonnumber = 1;
break;
}
}
if (!nonnumber) {
DEBUG("/proc/%s is a process\n", ep->d_name);
++count;
if (count > 1) {
closedir(dp);
return 2; //2 is enough, do not count further
}
}
}
closedir(dp);
} else {
PRINTERR("Could not open /proc.\n");
return -1;
}
return count;
}

/*
* The dumb-init signal handler.
*
Expand All @@ -88,6 +128,7 @@ void forward_signal(int signum) {
*
*/
void handle_signal(int signum) {
static char bereaved = 0;
DEBUG("Received signal %d.\n", signum);
if (signum == SIGCHLD) {
int status, exit_status;
Expand All @@ -103,11 +144,26 @@ void handle_signal(int signum) {
}

if (killed_pid == child_pid) {
forward_signal(SIGTERM); // send SIGTERM to any remaining children
DEBUG("Child exited with status %d. Goodbye.\n", exit_status);
bereaved = 1;
if (!survive_bereaving) {
forward_signal(SIGTERM); // send SIGTERM to any remaining children
DEBUG("Child exited with status %d. Goodbye.\n", exit_status);
exit(exit_status);
} else {
DEBUG("Child exited with status %d. Stay alive for your grandchildren.\n", exit_status);
}
}
}

if ((bereaved == 1) && survive_bereaving) {
signed int pc = process_count();
DEBUG("Process count: %d\n", pc);
if (pc <= 1) {
DEBUG("No process left, exitting.\n");
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: typo exitting -> exiting

exit(exit_status);
}
}

} else {
forward_signal(signum);
if (signum == SIGTSTP || signum == SIGTTOU || signum == SIGTTIN) {
Expand All @@ -126,15 +182,16 @@ void print_help(char *argv[]) {
"It is designed to run as PID1 in minimal container environments.\n"
"\n"
"Optional arguments:\n"
" -c, --single-child Run in single-child mode.\n"
" In this mode, signals are only proxied to the\n"
" direct child and not any of its descendants.\n"
" -r, --rewrite s:r Rewrite received signal s to new signal r before proxying.\n"
" To ignore (not proxy) a signal, rewrite it to 0.\n"
" This option can be specified multiple times.\n"
" -v, --verbose Print debugging information to stderr.\n"
" -h, --help Print this help message and exit.\n"
" -V, --version Print the current version and exit.\n"
" -c, --single-child Run in single-child mode.\n"
" In this mode, signals are only proxied to the\n"
" direct child and not any of its descendants.\n"
" -b, --survive-bereaving Do not quit when the direct child dies.\n"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it'd be helpful to add another line to clarify further when it actually does quit. Maybe a line like "In this mode, dumb-init will quit when there are no other processes running on the host. This mode requires dumb-init to be running as PID 1."?

" -r, --rewrite s:r Rewrite received signal s to new signal r before proxying.\n"
" To ignore (not proxy) a signal, rewrite it to 0.\n"
" This option can be specified multiple times.\n"
" -v, --verbose Print debugging information to stderr.\n"
" -h, --help Print this help message and exit.\n"
" -V, --version Print the current version and exit.\n"
"\n"
"Full help is available online at https://github.com/Yelp/dumb-init\n",
VERSION,
Expand Down Expand Up @@ -175,14 +232,15 @@ void set_rewrite_to_sigstop_if_not_defined(int signum) {
char **parse_command(int argc, char *argv[]) {
int opt;
struct option long_options[] = {
{"help", no_argument, NULL, 'h'},
{"single-child", no_argument, NULL, 'c'},
{"rewrite", required_argument, NULL, 'r'},
{"verbose", no_argument, NULL, 'v'},
{"version", no_argument, NULL, 'V'},
{"help", no_argument, NULL, 'h'},
{"single-child", no_argument, NULL, 'c'},
{"rewrite", required_argument, NULL, 'r'},
{"verbose", no_argument, NULL, 'v'},
{"version", no_argument, NULL, 'V'},
{"survive-bereaving",no_argument, NULL, 'b'},
{NULL, 0, NULL, 0},
};
while ((opt = getopt_long(argc, argv, "+hvVcr:", long_options, NULL)) != -1) {
while ((opt = getopt_long(argc, argv, "+hvVcbr:", long_options, NULL)) != -1) {
switch (opt) {
case 'h':
print_help(argv);
Expand All @@ -199,6 +257,9 @@ char **parse_command(int argc, char *argv[]) {
case 'r':
parse_rewrite_signum(optarg);
break;
case 'b':
survive_bereaving = 1;
break;
default:
exit(1);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Since survive_bereaving only works properly when dumb-init is running as PID 1, I think it'd be a good idea to add a check like:

if (survive_bereaving && getpid() != 1) {
    PRINTERR("--survive-bereaving is only valid when dumb-init is running as PID 1.\n");
    exit(1);
}

I think that would reduce confusion for anybody who tries to use the feature without being PID1.

Expand Down