diff options
author | Markos Fountoulakis <44345837+mfundul@users.noreply.github.com> | 2019-07-09 07:25:54 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-07-09 07:25:54 +0300 |
commit | 0707fbaaac974dc63f9d5a1b9298b0d09b1c95da (patch) | |
tree | 24973cacf063fcc09805f943a600152154f3f0f9 /libnetdata | |
parent | 43385d30c187ef463c4b7ef11bff710e30c49def (diff) |
Reimplemented mypopen() function family (#6339)
* Reimplementd mypopen() family based on posix_spawn() instead of fork()
and execl().
The problem with fork() is that if the parent process has a large address
space then the fork() may fail due to insufficient free memory in the
system if memory overcommit is not enabled.
posix_spawn() does not call fork() and does not suffer from this problem.
It is also more portable than vfork() which is deprecated and clone()
which is linux only.
* Removed dead code
Diffstat (limited to 'libnetdata')
-rw-r--r-- | libnetdata/libnetdata.h | 1 | ||||
-rw-r--r-- | libnetdata/popen/popen.c | 161 |
2 files changed, 72 insertions, 90 deletions
diff --git a/libnetdata/libnetdata.h b/libnetdata/libnetdata.h index 43dc1e04dc..1672ae3004 100644 --- a/libnetdata/libnetdata.h +++ b/libnetdata/libnetdata.h @@ -81,6 +81,7 @@ #include <time.h> #include <unistd.h> #include <uuid/uuid.h> +#include <spawn.h> #ifdef HAVE_NETINET_IN_H #include <netinet/in.h> diff --git a/libnetdata/popen/popen.c b/libnetdata/popen/popen.c index 845363fd22..177aebfc04 100644 --- a/libnetdata/popen/popen.c +++ b/libnetdata/popen/popen.c @@ -45,110 +45,91 @@ static void mypopen_del(FILE *fp) { #define PIPE_READ 0 #define PIPE_WRITE 1 -FILE *mypopen(const char *command, volatile pid_t *pidptr) -{ - int pipefd[2]; - - if(pipe(pipefd) == -1) return NULL; +static inline FILE *custom_popene(const char *command, volatile pid_t *pidptr, char **env) { + FILE *fp; + int pipefd[2], error; + pid_t pid; + char *const spawn_argv[] = { + "sh", + "-c", + (char *)command, + NULL + }; + posix_spawnattr_t attr; + posix_spawn_file_actions_t fa; - int pid = fork(); - if(pid == -1) { - close(pipefd[PIPE_READ]); - close(pipefd[PIPE_WRITE]); + if(pipe(pipefd) == -1) return NULL; + if ((fp = fdopen(pipefd[PIPE_READ], "r")) == NULL) { + goto error_after_pipe; } - if(pid != 0) { - // the parent - *pidptr = pid; - close(pipefd[PIPE_WRITE]); - FILE *fp = fdopen(pipefd[PIPE_READ], "r"); - /*mypopen_add(fp, pid);*/ - return(fp); - } - // the child - // close all files + // Mark all files to be closed by the exec() stage of posix_spawn() int i; for(i = (int) (sysconf(_SC_OPEN_MAX) - 1); i >= 0; i--) - if(i != STDIN_FILENO && i != STDERR_FILENO && i != pipefd[PIPE_WRITE]) close(i); - - // move the pipe to stdout - if(pipefd[PIPE_WRITE] != STDOUT_FILENO) { - dup2(pipefd[PIPE_WRITE], STDOUT_FILENO); - close(pipefd[PIPE_WRITE]); + if(i != STDIN_FILENO && i != STDERR_FILENO) + fcntl(i, F_SETFD, FD_CLOEXEC); + + if (!posix_spawn_file_actions_init(&fa)) { + // move the pipe to stdout in the child + if (posix_spawn_file_actions_adddup2(&fa, pipefd[PIPE_WRITE], STDOUT_FILENO)) { + error("posix_spawn_file_actions_adddup2() failed"); + goto error_after_posix_spawn_file_actions_init; + } + } else { + error("posix_spawn_file_actions_init() failed."); + goto error_after_pipe; } - -#ifdef DETACH_PLUGINS_FROM_NETDATA - // this was an attempt to detach the child and use the suspend mode charts.d - // unfortunatelly it does not work as expected. - - // fork again to become session leader - pid = fork(); - if(pid == -1) - error("pre-execution of command '%s' on pid %d: Cannot fork 2nd time.", command, getpid()); - - if(pid != 0) { - // the parent - exit(0); + if (!(error = posix_spawnattr_init(&attr))) { + // reset all signals in the child + sigset_t mask; + + if (posix_spawnattr_setflags(&attr, POSIX_SPAWN_SETSIGMASK | POSIX_SPAWN_SETSIGDEF)) + error("posix_spawnattr_setflags() failed."); + sigemptyset(&mask); + if (posix_spawnattr_setsigmask(&attr, &mask)) + error("posix_spawnattr_setsigmask() failed."); + } else { + error("posix_spawnattr_init() failed."); } - - // set a new process group id for just this child - if( setpgid(0, 0) != 0 ) - error("pre-execution of command '%s' on pid %d: Cannot set a new process group.", command, getpid()); - - if( getpgid(0) != getpid() ) - error("pre-execution of command '%s' on pid %d: Cannot set a new process group. Process group set is incorrect. Expected %d, found %d", command, getpid(), getpid(), getpgid(0)); - - if( setsid() != 0 ) - error("pre-execution of command '%s' on pid %d: Cannot set session id.", command, getpid()); - - fprintf(stdout, "MYPID %d\n", getpid()); - fflush(NULL); -#endif - - // reset all signals - signals_unblock(); - signals_reset(); - - debug(D_CHILDS, "executing command: '%s' on pid %d.", command, getpid()); - execl("/bin/sh", "sh", "-c", command, NULL); - exit(1); -} - -FILE *mypopene(const char *command, volatile pid_t *pidptr, char **env) { - int pipefd[2]; - - if(pipe(pipefd) == -1) - return NULL; - - int pid = fork(); - if(pid == -1) { + if (!posix_spawn(&pid, "/bin/sh", &fa, &attr, spawn_argv, env)) { + *pidptr = pid; + debug(D_CHILDS, "Spawned command: '%s' on pid %d from parent pid %d.", command, pid, getpid()); + } else { + error("Failed to spawn command: '%s' from parent pid %d.", command, getpid()); close(pipefd[PIPE_READ]); - close(pipefd[PIPE_WRITE]); - return NULL; + fp = NULL; } - if(pid != 0) { - // the parent - *pidptr = pid; - close(pipefd[PIPE_WRITE]); - FILE *fp = fdopen(pipefd[PIPE_READ], "r"); - return(fp); + close(pipefd[PIPE_WRITE]); + + if (!error) { + // posix_spawnattr_init() succeeded + if (posix_spawnattr_destroy(&attr)) + error("posix_spawnattr_destroy"); } - // the child + if (posix_spawn_file_actions_destroy(&fa)) + error("posix_spawn_file_actions_destroy"); + + return fp; + +error_after_posix_spawn_file_actions_init: + if (posix_spawn_file_actions_destroy(&fa)) + error("posix_spawn_file_actions_destroy"); +error_after_pipe: + close(pipefd[PIPE_READ]); + close(pipefd[PIPE_WRITE]); + return NULL; +} - // close all files - int i; - for(i = (int) (sysconf(_SC_OPEN_MAX) - 1); i >= 0; i--) - if(i != STDIN_FILENO && i != STDERR_FILENO && i != pipefd[PIPE_WRITE]) close(i); +// See man environ +extern char **environ; - // move the pipe to stdout - if(pipefd[PIPE_WRITE] != STDOUT_FILENO) { - dup2(pipefd[PIPE_WRITE], STDOUT_FILENO); - close(pipefd[PIPE_WRITE]); - } +FILE *mypopen(const char *command, volatile pid_t *pidptr) { + return custom_popene(command, pidptr, environ); +} - execle("/bin/sh", "sh", "-c", command, NULL, env); - exit(1); +FILE *mypopene(const char *command, volatile pid_t *pidptr, char **env) { + return custom_popene(command, pidptr, env); } int mypclose(FILE *fp, pid_t pid) { |