From 5fffb6f3f6f5c687cbc3380a8fe32539ec2f1c41 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Fri, 8 Feb 2019 01:27:46 +0000 Subject: [PATCH] Fork as early as possible If fork is requested, we fork as early as possible, way before anything is initialized. This way, we don't need to do the gymnastics to make OpenGL work properly across fork. Signed-off-by: Yuxuan Shui --- src/compton.c | 131 ++++++++++++++++++++++++-------------------------- src/config.h | 2 - src/dbus.c | 1 - src/options.c | 20 ++++++-- src/options.h | 2 +- 5 files changed, 80 insertions(+), 76 deletions(-) diff --git a/src/compton.c b/src/compton.c index 9fdeb8f..822eee6 100644 --- a/src/compton.c +++ b/src/compton.c @@ -9,6 +9,7 @@ * */ +#include #include #include #include @@ -2028,49 +2029,6 @@ register_cm(session_t *ps) { return true; } -/** - * Fork program to background and disable all I/O streams. - */ -static inline bool -fork_after(session_t *ps) { - if (getppid() == 1) - return true; - -#ifdef CONFIG_OPENGL - // GLX context must be released and reattached on fork - if (glx_has_context(ps) && !glXMakeCurrent(ps->dpy, XCB_NONE, NULL)) { - log_fatal("Failed to detach GLX context."); - return false; - } -#endif - - int pid = fork(); - - if (-1 == pid) { - log_fatal("fork() failed."); - return false; - } - - if (pid > 0) _exit(0); - - setsid(); - -#ifdef CONFIG_OPENGL - if (glx_has_context(ps) - && !glXMakeCurrent(ps->dpy, get_tgt_window(ps), ps->psglx->context)) { - log_fatal("Failed to make GLX context current."); - return false; - } -#endif - - if (!freopen("/dev/null", "r", stdin)) { - log_fatal("freopen() failed."); - return false; - } - - return true; -} - /** * Write PID to a file. */ @@ -2485,14 +2443,16 @@ reset_enable(EV_P_ ev_signal *w, int revents) { /** * Initialize a session. * - * @param ps_old old session, from which the function will take the X - * connection, then free it * @param argc number of commandline arguments * @param argv commandline arguments + * @param dpy the X Display + * @param config_file the path to the config file + * @param all_xerros whether we should report all X errors + * @param fork whether we will fork after initialization */ static session_t * -session_init(int argc, char **argv, Display *dpy, bool all_xerrors, - const char *config_file) { +session_init(int argc, char **argv, Display *dpy, const char *config_file, + bool all_xerrors, bool fork) { static const session_t s_def = { .backend_data = NULL, .dpy = NULL, @@ -2518,7 +2478,6 @@ session_init(int argc, char **argv, Display *dpy, bool all_xerrors, .glx_no_stencil = false, .mark_wmwin_focused = false, .mark_ovredir_focused = false, - .fork_after_register = false, .detect_rounded_corners = false, .resize_damage = 0, .unredir_if_possible = false, @@ -2779,6 +2738,7 @@ session_init(int argc, char **argv, Display *dpy, bool all_xerrors, log_info("Switching to log file: %s", ps->o.logpath); log_remove_target_tls(stderr_logger); log_add_target_tls(l); + stderr_logger = NULL; } else { log_error("Failed to setup log file %s, I will keep using stderr", ps->o.logpath); } @@ -3023,23 +2983,12 @@ session_init(int argc, char **argv, Display *dpy, bool all_xerrors, free(e); } - // Fork to background, if asked - if (ps->o.fork_after_register) { - if (!fork_after(ps)) { - session_destroy(ps); - return NULL; - } - } - - // Redirect output stream - if (ps->o.fork_after_register) { - if (!freopen("/dev/null", "w", stdout) || !freopen("/dev/null", "w", stderr)) { - log_fatal("Failed to redirect stdout/stderr to /dev/null"); - exit(1); - } - } - write_pid(ps); + + if (fork && stderr_logger) { + // Remove the stderr logger if we will fork + log_remove_target_tls(stderr_logger); + } return ps; } @@ -3232,11 +3181,41 @@ main(int argc, char **argv) { int exit_code; char *config_file = NULL; - bool all_xerrors = 0; - if (get_early_config(argc, argv, &config_file, &all_xerrors, &exit_code)) { + bool all_xerrors = false, need_fork = false; + if (get_early_config(argc, argv, &config_file, &all_xerrors, &need_fork, &exit_code)) { return exit_code; } + int pfds[2]; + if (need_fork) { + if (pipe2(pfds, O_CLOEXEC)) { + perror("pipe2"); + return 1; + } + auto pid = fork(); + if (pid < 0) { + perror("fork"); + return 1; + } + if (pid > 0) { + // We are the parent + close(pfds[1]); + // We wait for the child to tell us it has finished initialization + // by sending us something via the pipe. + int tmp; + if (read(pfds[0], &tmp, sizeof tmp) <= 0) { + // Failed to read, the child has most likely died + // We can probably waitpid() here. + return 1; + } else { + // We are done + return 0; + } + } + // We are the child + close(pfds[0]); + } + sigset_t sigmask; sigemptyset(&sigmask); const struct sigaction int_action = { @@ -3256,11 +3235,29 @@ main(int argc, char **argv) { XSetEventQueueOwner(dpy, XCBOwnsEventQueue); do { - ps_g = session_init(argc, argv, dpy, all_xerrors, config_file); + ps_g = session_init(argc, argv, dpy, config_file, all_xerrors, need_fork); if (!ps_g) { log_fatal("Failed to create new compton session."); return 1; } + if (need_fork) { + // Finishing up daemonization + // Close files + if (fclose(stdout) || fclose(stderr) || fclose(stdin)) { + log_fatal("Failed to close standard input/output"); + return 1; + } + // Make us the session and process group leader so we don't get killed when + // our parent die. + setsid(); + // Notify the parent that we are done. This might cause the parent to quit, + // so only do this after setsid() + int tmp = 1; + write(pfds[1], &tmp, sizeof tmp); + close(pfds[1]); + // We only do this once + need_fork = false; + } session_run(ps_g); quit = ps_g->quit; session_destroy(ps_g); diff --git a/src/config.h b/src/config.h index d87a1b5..785d153 100644 --- a/src/config.h +++ b/src/config.h @@ -96,8 +96,6 @@ typedef struct options_t { bool glx_use_gpushader4; /// Custom fragment shader for painting windows, as a string. char *glx_fshader_win_str; - /// Whether to fork to background. - bool fork_after_register; /// Whether to detect rounded corners. bool detect_rounded_corners; /// Force painting of window content with blending. diff --git a/src/dbus.c b/src/dbus.c index 6df7315..f5ea614 100644 --- a/src/dbus.c +++ b/src/dbus.c @@ -1029,7 +1029,6 @@ cdbus_process_opts_get(session_t *ps, DBusMessage *msg) { cdbus_m_opts_get_do(write_pid_path, cdbus_reply_string); cdbus_m_opts_get_do(mark_wmwin_focused, cdbus_reply_bool); cdbus_m_opts_get_do(mark_ovredir_focused, cdbus_reply_bool); - cdbus_m_opts_get_do(fork_after_register, cdbus_reply_bool); cdbus_m_opts_get_do(detect_rounded_corners, cdbus_reply_bool); cdbus_m_opts_get_stub(paint_on_overlay, cdbus_reply_bool, ps->overlay != XCB_NONE); // paint_on_overlay_id: Get ID of the X composite overlay window diff --git a/src/options.c b/src/options.c index d1925e3..d1b0378 100644 --- a/src/options.c +++ b/src/options.c @@ -470,7 +470,8 @@ static const struct option longopts[] = { /// Get config options that are needed to parse the rest of the options /// Return true if we should quit -bool get_early_config(int argc, char *const *argv, char **config_file, bool *all_xerrors, int *exit_code) { +bool get_early_config(int argc, char *const *argv, char **config_file, bool *all_xerrors, + bool *fork, int *exit_code) { int o = 0, longopt_idx = -1; // Pre-parse the commandline arguments to check for --config and invalid @@ -486,6 +487,9 @@ bool get_early_config(int argc, char *const *argv, char **config_file, bool *all } else if (o == 'h') { usage(0); return true; + + } else if (o == 'b') { + *fork = true; } else if (o == 'd') { log_warn("-d will be ignored, please use the DISPLAY " "environment variable"); @@ -548,12 +552,19 @@ void get_cfg(options_t *opt, int argc, char *const *argv, bool shadow_enable, // clang-format off // Short options - case 'h': usage(0); break; + case 318: + case 'h': + // These options should cause compton to exit early, + // so assert(false) here + assert(false); + break; case 'd': + case 'b': case 'S': case 314: - case 318: - case 320: break; + case 320: + // These options are handled by get_early_config() + break; P_CASELONG('D', fade_delta); case 'I': opt->fade_in_step = normalize_d(atof(optarg)) * OPAQUE; break; case 'O': opt->fade_out_step = normalize_d(atof(optarg)) * OPAQUE; break; @@ -599,7 +610,6 @@ void get_cfg(options_t *opt, int argc, char *const *argv, bool shadow_enable, case 's': log_error("-n, -a, and -s have been removed."); break; - P_CASEBOOL('b', fork_after_register); // Long options case 256: // --config diff --git a/src/options.h b/src/options.h index 732837a..144820e 100644 --- a/src/options.h +++ b/src/options.h @@ -17,7 +17,7 @@ typedef struct session session_t; /// Get config options that are needed to parse the rest of the options /// Return true if we should quit bool get_early_config(int argc, char *const *argv, char **config_file, bool *all_xerrors, - int *exit_code); + bool *fork, int *exit_code); /** * Process arguments and configuration files.