From 6170d43d195d6e955ce7d906da3908a689b573f6 Mon Sep 17 00:00:00 2001 From: Sanel Zukan Date: Mon, 24 Sep 2007 11:47:42 +0000 Subject: [PATCH] Reworked process handler due a large number of races. Now process reports should not interfere with popped up message boxes. Still there is a issue with childs during splash startup: when one of them crashes with core dump crash dialog will not be shown, but the main process handler correctly returns crash flag. --- evoke/EvokeService.cpp | 110 ++++++++++++++++++++++++++++++++--------- evoke/EvokeService.h | 22 +++++++-- evoke/Spawn.cpp | 28 +++++++++-- evoke/Spawn.h | 1 + evoke/Splash.cpp | 18 +++++-- evoke/Xsm.cpp | 3 -- evoke/evoke.cpp | 5 ++ 7 files changed, 151 insertions(+), 36 deletions(-) diff --git a/evoke/EvokeService.cpp b/evoke/EvokeService.cpp index a3a75aa..46336da 100644 --- a/evoke/EvokeService.cpp +++ b/evoke/EvokeService.cpp @@ -32,11 +32,14 @@ #include #include // getpid -#include // +#include // pipe +#include // fcntl #include // free #include // strdup, memset #include // error codes +#include + void resolve_path(const edelib::String& datadir, edelib::String& item, bool have_datadir) { if(item.empty()) return; @@ -195,8 +198,15 @@ void service_watcher_cb(int pid, int signum) { EvokeService::instance()->service_watcher(pid, signum); } +void wake_up_cb(int fd, void* v) { + EvokeService::instance()->wake_up(fd); +} + EvokeService::EvokeService() : is_running(0), logfile(NULL), xsm(NULL), pidfile(NULL), lockfile(NULL) { + + wake_up_pipe[0] = wake_up_pipe[1] = -1; + quit_child_pid = quit_child_ret = -1; } EvokeService::~EvokeService() { @@ -217,6 +227,11 @@ EvokeService::~EvokeService() { } processes.clear(); + + if(wake_up_pipe[0] != -1) + close(wake_up_pipe[0]); + if(wake_up_pipe[1] != -1) + close(wake_up_pipe[1]); } EvokeService* EvokeService::instance(void) { @@ -224,6 +239,15 @@ EvokeService* EvokeService::instance(void) { return &es; } +bool EvokeService::setup_channels(void) { + if(pipe(wake_up_pipe) != 0) + return false; + + fcntl(wake_up_pipe[1], F_SETFL, fcntl(wake_up_pipe[1], F_GETFL) | O_NONBLOCK); + Fl::add_fd(wake_up_pipe[0], FL_READ, wake_up_cb); + return true; +} + bool EvokeService::setup_logging(const char* file) { if(!file) logfile = new DummyLog(); @@ -541,46 +565,88 @@ void EvokeService::quit_x11(void) { } /* - * Monitor starting service and report if staring failed. Also if one of - * runned services crashed attach gdb on it pid and run backtrace. + * This is run each time when some of the managed childs quits. + * Instead directly running wake_up(), it will be notified wia + * wake_up_pipe[] channel, via add_fd() monitor. + * + * This workaround is due races. */ -void EvokeService::service_watcher(int pid, int signum) { - printf("got %i\n", signum); +void EvokeService::service_watcher(int pid, int ret) { + puts("=== service_watcher() ==="); + printf("got %i\n", ret); + Mutex mutex; - if(signum == SPAWN_CHILD_CRASHED) { + mutex.lock(); + quit_child_ret = ret; + quit_child_pid = pid; + mutex.unlock(); + + if(write(wake_up_pipe[1], "c", 1) != 1) + puts("error write"); +} + +void EvokeService::wake_up(int fd) { + puts("=== wake_up() ==="); + + char c; + if(read(wake_up_pipe[0], &c, 1) != 1 || c != 'c') { + puts("unable to read from channel"); + return; + } + + Mutex mutex; + + mutex.lock(); + int child_ret = quit_child_ret; + int child_pid = quit_child_pid; + mutex.unlock(); + + //EASSERT(child_ret != -1 && child_pid != -1); + + mutex.lock(); + quit_child_ret = quit_child_pid = -1; + mutex.unlock(); + + if(child_ret == SPAWN_CHILD_CRASHED) { EvokeProcess pc; bool ret; mutex.lock(); - ret = find_and_unregister_process(pid, pc); + ret = find_and_unregister_process(child_pid, pc); mutex.unlock(); if(ret) { printf("%s crashed with core dump\n", pc.cmd.c_str()); - CrashDialog cdialog; cdialog.set_data(pc.cmd.c_str()); cdialog.run(); } - return; - } + } else { - mutex.lock(); - unregister_process(pid); - mutex.unlock(); - - if(signum == SPAWN_CHILD_KILLED) { - printf("child %i killed\n", pid); - } else if(signum == 127) { - edelib::alert(_("Program not found")); - } else if(signum == 126) { - edelib::alert(_("Program not executable")); - } else { - printf("child %i exited with %i\n", pid, signum); + mutex.lock(); + unregister_process(child_pid); + mutex.unlock(); + + switch(child_ret) { + case SPAWN_CHILD_KILLED: + printf("child %i killed\n", child_pid); + break; + case 127: + edelib::alert(_("Program not found")); + break; + case 126: + edelib::alert(_("Program not executable")); + break; + default: + printf("child %i exited with %i\n", child_pid, child_ret); + break; + } } + } + /* * Execute program. It's return status * will be reported via service_watcher() diff --git a/evoke/EvokeService.h b/evoke/EvokeService.h index 5e918bd..efe4aa9 100644 --- a/evoke/EvokeService.h +++ b/evoke/EvokeService.h @@ -33,15 +33,23 @@ struct EvokeProcess { pid_t pid; }; -typedef edelib::list ClientList; +struct QueuedSignal { + pid_t pid; + int signum; +}; + +typedef edelib::list ClientList; typedef edelib::list::iterator ClientListIter; -typedef edelib::list StringList; +typedef edelib::list StringList; typedef edelib::list::iterator StringListIter; -typedef edelib::list ProcessList; +typedef edelib::list ProcessList; typedef edelib::list::iterator ProcessListIter; +typedef edelib::list SignalQueue; +typedef edelib::list::iterator SignalQueueIter; + class Fl_Double_Window; class EvokeService { @@ -58,6 +66,10 @@ class EvokeService { ClientList clients; ProcessList processes; + int quit_child_pid; + int quit_child_ret; + int wake_up_pipe[2]; + public: EvokeService(); @@ -68,6 +80,7 @@ class EvokeService { void stop(void) { is_running = false; } bool running(void) { return is_running; } + bool setup_channels(void); bool setup_logging(const char* file); bool setup_pid(const char* file, const char* lock); void setup_atoms(Display* d); @@ -82,8 +95,9 @@ class EvokeService { Log* log(void) { return logfile; } void service_watcher(int pid, int signum); + void wake_up(int fd); + void run_program(const char* cmd, bool enable_vars = 1); - //void heuristic_run_program(const char* cmd); void register_process(const char* cmd, pid_t pid); void unregister_process(pid_t pid); bool find_and_unregister_process(pid_t pid, EvokeProcess& pc); diff --git a/evoke/Spawn.cpp b/evoke/Spawn.cpp index a70ebca..f4a145d 100644 --- a/evoke/Spawn.cpp +++ b/evoke/Spawn.cpp @@ -26,14 +26,13 @@ #include // extern char** environ; -// FIXME: is this safe ??? (or to use somehow sig_atomic_t) SignalWatch* global_watch = 0; void sigchld_handler(int sig) { int pid, status; do { errno = 0; - pid = waitpid(WAIT_ANY, &status, WNOHANG); + pid = waitpid(WAIT_ANY, &status, WNOHANG | WUNTRACED); if(global_watch != 0) { if(WIFEXITED(status)) @@ -56,12 +55,28 @@ int spawn_program(const char* cmd, SignalWatch wf, pid_t* child_pid_ret, const c int nulldev = -1; int status_ret = 0; +#if 0 struct sigaction sa; + sa.sa_handler = sigchld_handler; + sa.sa_flags = SA_NOCLDSTOP; + /*sa.sa_flags = SA_NOCLDWAIT;*/ + sigemptyset(&sa.sa_mask); + sigaction(SIGCHLD, &sa, (struct sigaction*)0); +#endif + + struct sigaction sa; + sigset_t chld_mask, old_mask; + sa.sa_handler = sigchld_handler; sa.sa_flags = SA_NOCLDSTOP; sigemptyset(&sa.sa_mask); sigaction(SIGCHLD, &sa, (struct sigaction*)0); + // block SIGCHLD until we set up stuff + sigemptyset(&chld_mask); + sigaddset(&chld_mask, SIGCHLD); + sigprocmask(SIG_BLOCK, &chld_mask, &old_mask); + global_watch = wf; pid_t pid = fork(); @@ -69,7 +84,7 @@ int spawn_program(const char* cmd, SignalWatch wf, pid_t* child_pid_ret, const c return SPAWN_FORK_FAILED; if(pid == 0) { - // this is child + // child char* argv[4]; argv[0] = "/bin/sh"; argv[1] = "-c"; @@ -95,12 +110,12 @@ int spawn_program(const char* cmd, SignalWatch wf, pid_t* child_pid_ret, const c errno = 0; if(execve(argv[0], argv, environ) == -1) { - close(nulldev); // should not get here return SPAWN_EXECVE_FAILED; } } + // parent if(nulldev != -1) close(nulldev); /* @@ -111,6 +126,9 @@ int spawn_program(const char* cmd, SignalWatch wf, pid_t* child_pid_ret, const c if(child_pid_ret) *child_pid_ret = pid; + // unblock SIGCHLD + sigprocmask(SIG_SETMASK, &old_mask, NULL); + return status_ret; } @@ -139,6 +157,8 @@ int spawn_backtrace(const char* gdb_path, const char* program, const char* core, const char* gdb_script = "bt\nquit\n"; const int gdb_script_len = 8; + //signal(SIGCHLD, SIG_DFL); + // file with gdb commands int sfd = open(script, O_WRONLY | O_TRUNC | O_CREAT, 0770); if(sfd == -1) diff --git a/evoke/Spawn.h b/evoke/Spawn.h index 39a53eb..70f4a26 100644 --- a/evoke/Spawn.h +++ b/evoke/Spawn.h @@ -27,6 +27,7 @@ #define SPAWN_EMPTY 2 #define SPAWN_EXECVE_FAILED 3 #define SPAWN_OPEN_FAILED 4 +#define SPAWN_PIPE_FAILED 5 #define SPAWN_CHILD_CRASHED -2 #define SPAWN_CHILD_KILLED -3 diff --git a/evoke/Splash.cpp b/evoke/Splash.cpp index 1f4d060..b7a862d 100644 --- a/evoke/Splash.cpp +++ b/evoke/Splash.cpp @@ -28,9 +28,15 @@ extern void service_watcher_cb(int pid, int signum); - Fl_Double_Window* splash_win = 0; +class AutoSound { + public: + AutoSound() { edelib::SoundSystem::init(); } + ~AutoSound() { if(edelib::SoundSystem::inited()) edelib::SoundSystem::shutdown(); } + void play(const char* file) { if(edelib::SoundSystem::inited()) edelib::SoundSystem::play(file, 0); } +}; + int splash_xmessage_handler(int e) { if(fl_xevent->type == MapNotify || fl_xevent->type == ConfigureNotify) { if(splash_win) { @@ -186,9 +192,12 @@ void Splash::run(void) { int sh = DisplayHeight(fl_display, fl_screen); position(sw/2 - w()/2, sh/2 - h()/2); +#if 0 bool sound_loaded = false; if(sound && !sound->empty()) sound_loaded = edelib::SoundSystem::init(); +#endif + AutoSound sound_play; show(); @@ -203,17 +212,20 @@ void Splash::run(void) { //XSelectInput(fl_display, RootWindow(fl_display, fl_screen), SubstructureNotifyMask); //Fl::add_handler(splash_xmessage_handler); - +#if 0 if(sound_loaded) edelib::SoundSystem::play(sound->c_str(), 0); +#endif + sound_play.play(sound->c_str()); while(shown()) Fl::wait(); - +#if 0 if(sound_loaded) { edelib::SoundSystem::stop(); edelib::SoundSystem::shutdown(); } +#endif splash_win = 0; } diff --git a/evoke/Xsm.cpp b/evoke/Xsm.cpp index bae9476..ff5a308 100644 --- a/evoke/Xsm.cpp +++ b/evoke/Xsm.cpp @@ -191,9 +191,6 @@ void setting_store(const XsettingsSetting* setting, XsettingsBuffer* buffer) { } } - - - Xsm::Xsm() : data(NULL) { } Xsm::~Xsm() { diff --git a/evoke/evoke.cpp b/evoke/evoke.cpp index 951ab9d..b9bb705 100644 --- a/evoke/evoke.cpp +++ b/evoke/evoke.cpp @@ -146,6 +146,11 @@ int main(int argc, char** argv) { return 1; } + if(!service->setup_channels()) { + printf("Can't setup internal channels\n"); + return 1; + } + EVOKE_LOG("= "APPNAME" started =\n"); if(!pid_file)