From 916a86b4c0bd1297bdb2b98f12927d82910097f8 Mon Sep 17 00:00:00 2001 From: root Date: Wed, 15 Jul 2009 12:26:26 +0200 Subject: test commit Signed-off-by: root --- src/Daemon/abrt.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/Daemon') diff --git a/src/Daemon/abrt.conf b/src/Daemon/abrt.conf index 4255e02..fa6a9b3 100644 --- a/src/Daemon/abrt.conf +++ b/src/Daemon/abrt.conf @@ -10,7 +10,7 @@ OpenGPGPublicKeys = /etc/pki/rpm-gpg/RPM-GPG-KEY-fedora BlackList = # enabled plugins # there has to be exactly one database plugin -EnabledPlugins = SQLite3, CCpp, Logger, Kerneloops, KerneloopsScanner, KerneloopsReporter #, Mailx +EnabledPlugins = SQLite3, CCpp, Logger, Kerneloops, KerneloopsScanner, KerneloopsReporter #, Mailx # Database Database = SQLite3 # max size for crash storage [MiB] -- cgit From 3431b2cee2ad71c0046bcef239ddd30539c202ae Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Wed, 15 Jul 2009 15:12:16 +0200 Subject: Restore /proc/sys/kernel/core_pattern on error exit. The bug was observed when dbus-abrt.conf is missing. Signed-off-by: Denys Vlasenko --- src/Daemon/CrashWatcher.cpp | 67 ++++++++++++++++++++++++++------------------- src/Daemon/Daemon.cpp | 20 ++++++++++++-- 2 files changed, 57 insertions(+), 30 deletions(-) (limited to 'src/Daemon') diff --git a/src/Daemon/CrashWatcher.cpp b/src/Daemon/CrashWatcher.cpp index 9a14967..4513309 100644 --- a/src/Daemon/CrashWatcher.cpp +++ b/src/Daemon/CrashWatcher.cpp @@ -380,27 +380,38 @@ CCrashWatcher::CCrashWatcher(const std::string& pPath) m_pMainloop = g_main_loop_new(NULL,FALSE); m_pMW = new CMiddleWare(PLUGINS_CONF_DIR,PLUGINS_LIB_DIR); - SetUpMW(); - SetUpCron(); - FindNewDumps(pPath); + + try + { + SetUpMW(); + SetUpCron(); + FindNewDumps(pPath); #ifdef ENABLE_DBUS - m_pCommLayer = new CCommLayerServerDBus(); + m_pCommLayer = new CCommLayerServerDBus(); #elif ENABLE_SOCKET - m_pCommLayer = new CCommLayerServerSocket(); + m_pCommLayer = new CCommLayerServerSocket(); #endif -// m_pCommLayer = new CCommLayerServerDBus(); -// m_pCommLayer = new CCommLayerServerSocket(); - m_pCommLayer->Attach(this); +// m_pCommLayer = new CCommLayerServerDBus(); +// m_pCommLayer = new CCommLayerServerSocket(); + m_pCommLayer->Attach(this); - if((m_nFd = inotify_init()) == -1) - { - throw CABRTException(EXCEP_FATAL, "CCrashWatcher::CCrashWatcher(): Init Failed"); + if((m_nFd = inotify_init()) == -1) + { + throw CABRTException(EXCEP_FATAL, "CCrashWatcher::CCrashWatcher(): Init Failed"); + } + if((watch = inotify_add_watch(m_nFd, pPath.c_str(), IN_CREATE)) == -1) + { + throw CABRTException(EXCEP_FATAL, "CCrashWatcher::CCrashWatcher(): Add watch failed:" + pPath); + } + m_pGio = g_io_channel_unix_new(m_nFd); } - if((watch = inotify_add_watch(m_nFd, pPath.c_str(), IN_CREATE)) == -1){ - - throw CABRTException(EXCEP_FATAL, "CCrashWatcher::CCrashWatcher(): Add watch failed:" + pPath); + catch (...) + { + /* This restores /proc/sys/kernel/core_pattern, among other things */ + delete m_pMW; + //too? delete m_pCommLayer; + throw; } - m_pGio = g_io_channel_unix_new(m_nFd); } CCrashWatcher::~CCrashWatcher() @@ -519,17 +530,17 @@ void CCrashWatcher::CreatePidFile() void CCrashWatcher::Lock() { int lfp = open(VAR_RUN_LOCK_FILE, O_RDWR|O_CREAT,0640); - if (lfp < 0) - { - throw CABRTException(EXCEP_FATAL, "CCrashWatcher::Lock(): can not open lock file"); - } - if (lockf(lfp,F_TLOCK,0) < 0) - { - throw CABRTException(EXCEP_FATAL, "CCrashWatcher::Lock(): cannot create lock on lockfile"); - } - /* only first instance continues */ - //sprintf(str,"%d\n",getpid()); - //write(lfp,str,strlen(str)); /* record pid to lockfile */ + if (lfp < 0) + { + throw CABRTException(EXCEP_FATAL, "CCrashWatcher::Lock(): can not open lock file"); + } + if (lockf(lfp,F_TLOCK,0) < 0) + { + throw CABRTException(EXCEP_FATAL, "CCrashWatcher::Lock(): cannot create lock on lockfile"); + } + /* only first instance continues */ + //sprintf(str,"%d\n",getpid()); + //write(lfp,str,strlen(str)); /* record pid to lockfile */ } void CCrashWatcher::StartWatch() @@ -573,7 +584,7 @@ void CCrashWatcher::GStartWatch() void CCrashWatcher::Run() { - Debug("Runnig..."); + Debug("Running..."); Lock(); CreatePidFile(); GStartWatch(); @@ -631,7 +642,7 @@ vector_crash_infos_t CCrashWatcher::GetCrashInfos(const std::string &pUID) //retval = m_pMW->GetCrashInfos(pUID); //Notify("Sent crash info"); - return retval; + return retval; } map_crash_report_t CCrashWatcher::CreateReport(const std::string &pUUID,const std::string &pUID) diff --git a/src/Daemon/Daemon.cpp b/src/Daemon/Daemon.cpp index 94f5e66..ea769c5 100644 --- a/src/Daemon/Daemon.cpp +++ b/src/Daemon/Daemon.cpp @@ -21,6 +21,9 @@ #include "ABRTException.h" #include #include +#include +#include +#include CCrashWatcher *g_pCrashWatcher = NULL; @@ -54,7 +57,15 @@ int main(int argc, char** argv) } if(daemonize) { - // forking to background + /* Open stdin to /dev/null. We do it before forking + * in order to emit useful exitcode to the parent + * if open fails */ + close(STDIN_FILENO); + if (open("/dev/null", O_RDWR)) + { + throw CABRTException(EXCEP_FATAL, "Can't open /dev/null"); + } + /* forking to background */ pid_t pid = fork(); if (pid < 0) { @@ -68,9 +79,12 @@ int main(int argc, char** argv) { throw CABRTException(EXCEP_FATAL, "CCrashWatcher::Daemonize(): setsid failed"); } - close(STDIN_FILENO); + /* We must not leave fds 0,1,2 closed. + * Otherwise fprintf(stderr) dumps messages into random fds, etc. */ close(STDOUT_FILENO); close(STDERR_FILENO); + dup(0); + dup(0); } g_pCrashWatcher = new CCrashWatcher(DEBUG_DUMPS_DIR); g_pCrashWatcher->Run(); @@ -83,5 +97,7 @@ int main(int argc, char** argv) { std::cerr << "Cannot create daemon: " << e.what() << std::endl; } + //do we need this? delete g_pCrashWatcher; + return 1; /* Any exit is a failure. Normally we don't exit at all */ } -- cgit From 264b636f2687ebc48743e895ccbf816898c136ed Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Fri, 17 Jul 2009 13:20:51 +0200 Subject: simplify CCrashWatcher::CreatePidFile Signed-off-by: Denys Vlasenko --- src/Daemon/CrashWatcher.cpp | 42 +++++++++++++++++++----------------------- 1 file changed, 19 insertions(+), 23 deletions(-) (limited to 'src/Daemon') diff --git a/src/Daemon/CrashWatcher.cpp b/src/Daemon/CrashWatcher.cpp index 4513309..9b67707 100644 --- a/src/Daemon/CrashWatcher.cpp +++ b/src/Daemon/CrashWatcher.cpp @@ -509,18 +509,14 @@ void CCrashWatcher::CreatePidFile() /* open the pidfile */ fd = open(VAR_RUN_PIDFILE, O_WRONLY|O_CREAT|O_EXCL, 0644); - if (fd >= 0) { - FILE *f; - - /* write our pid to it */ - f = fdopen(fd, "w"); - if (f != NULL) { - fprintf(f, "%d\n", getpid()); - fclose(f); - /* leave the fd open */ - return; - } - close(fd); + if (fd >= 0) + { + /* write our pid to it */ + char buf[sizeof(int)*3 + 2]; + int len = sprintf(buf, "%u\n", (unsigned)getpid()); + write(fd, buf, len); + close(fd); + return; } /* something went wrong */ @@ -530,17 +526,17 @@ void CCrashWatcher::CreatePidFile() void CCrashWatcher::Lock() { int lfp = open(VAR_RUN_LOCK_FILE, O_RDWR|O_CREAT,0640); - if (lfp < 0) - { - throw CABRTException(EXCEP_FATAL, "CCrashWatcher::Lock(): can not open lock file"); - } - if (lockf(lfp,F_TLOCK,0) < 0) - { - throw CABRTException(EXCEP_FATAL, "CCrashWatcher::Lock(): cannot create lock on lockfile"); - } - /* only first instance continues */ - //sprintf(str,"%d\n",getpid()); - //write(lfp,str,strlen(str)); /* record pid to lockfile */ + if (lfp < 0) + { + throw CABRTException(EXCEP_FATAL, "CCrashWatcher::Lock(): can not open lock file"); + } + if (lockf(lfp,F_TLOCK,0) < 0) + { + throw CABRTException(EXCEP_FATAL, "CCrashWatcher::Lock(): cannot create lock on lockfile"); + } + /* only first instance continues */ + //sprintf(str,"%d\n",getpid()); + //write(lfp,str,strlen(str)); /* record pid to lockfile */ } void CCrashWatcher::StartWatch() -- cgit From 27967b17597a24e76f06871332d7a44eeb790a80 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Fri, 17 Jul 2009 14:11:58 +0200 Subject: prevent leaking buf in some cases Signed-off-by: Denys Vlasenko --- src/Daemon/CrashWatcher.cpp | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) (limited to 'src/Daemon') diff --git a/src/Daemon/CrashWatcher.cpp b/src/Daemon/CrashWatcher.cpp index 9b67707..e21d57a 100644 --- a/src/Daemon/CrashWatcher.cpp +++ b/src/Daemon/CrashWatcher.cpp @@ -47,7 +47,8 @@ to_string( T x ) } */ -gboolean CCrashWatcher::handle_event_cb(GIOChannel *gio, GIOCondition condition, gpointer daemon){ +gboolean CCrashWatcher::handle_event_cb(GIOChannel *gio, GIOCondition condition, gpointer daemon) +{ GIOError err; char *buf = new char[INOTIFY_BUFF_SIZE]; gsize len; @@ -56,8 +57,9 @@ gboolean CCrashWatcher::handle_event_cb(GIOChannel *gio, GIOCondition condition, CCrashWatcher *cc = (CCrashWatcher*)daemon; if (err != G_IO_ERROR_NONE) { - cc->Warning("Error reading inotify fd."); - return FALSE; + cc->Warning("Error reading inotify fd."); + delete[] buf; + return FALSE; } /* reconstruct each event and send message to the dbus */ while (i < len) @@ -67,7 +69,7 @@ gboolean CCrashWatcher::handle_event_cb(GIOChannel *gio, GIOCondition condition, event = (struct inotify_event *) &buf[i]; if (event->len) - name = &buf[i] + sizeof (struct inotify_event); + name = &buf[i] + sizeof (struct inotify_event); i += sizeof (struct inotify_event) + event->len; cc->Debug(std::string("Created file: ") + name); @@ -114,9 +116,15 @@ gboolean CCrashWatcher::handle_event_cb(GIOChannel *gio, GIOCondition condition, cc->Warning(e.what()); if (e.type() == EXCEP_FATAL) { + delete[] buf; return -1; } } + catch (...) + { + delete[] buf; + throw; + } } else { -- cgit From 37ab187408799ba3f3f9107bdc5a72fea0b4b608 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Fri, 17 Jul 2009 15:52:27 +0200 Subject: rework unsafe handling of SIGINT/SIGTERM Signals are asynchronous. It is unsafe to perform such complex operations in a signal handler. I changed signal handler to just set a flag, and added an event source which returns an event when this variable is set. The action is to stop event loop. Execution then falls through to program exit. Signed-off-by: Denys Vlasenko --- src/Daemon/CrashWatcher.cpp | 60 +++++++++++++++++++++++++++++++++++++++++++-- src/Daemon/CrashWatcher.h | 5 ++-- src/Daemon/Daemon.cpp | 16 ++++++------ 3 files changed, 69 insertions(+), 12 deletions(-) (limited to 'src/Daemon') diff --git a/src/Daemon/CrashWatcher.cpp b/src/Daemon/CrashWatcher.cpp index e21d57a..73c90e0 100644 --- a/src/Daemon/CrashWatcher.cpp +++ b/src/Daemon/CrashWatcher.cpp @@ -578,12 +578,63 @@ void CCrashWatcher::StartWatch() delete[] buff; } +extern uint8_t sig_caught; +//prepare() +//If the source can determine that it is ready here (without waiting +//for the results of the poll() call) it should return TRUE. It can also +//return a timeout_ value which should be the maximum timeout (in milliseconds) +//which should be passed to the poll() call. +//check() +//Called after all the file descriptors are polled. The source should +//return TRUE if it is ready to be dispatched. +//dispatch() +//Called to dispatch the event source, after it has returned TRUE +//in either its prepare or its check function. The dispatch function +//is passed in a callback function and data. The callback function +//may be NULL if the source was never connected to a callback using +//g_source_set_callback(). The dispatch function should +//call the callback function with user_data and whatever additional +//parameters are needed for this type of event source. +typedef struct SignalSource +{ + GSource src; + CCrashWatcher* watcher; +} SignalSource; +static gboolean waitsignal_prepare(GSource *source, gint *timeout_) +{ + /* We depend on the fact that in Unix, poll() is interrupted + * by caught signals (returns EINTR). Thus we do not need to set + * a small timeout here: infinite timeout (-1) works too */ + *timeout_ = -1; + return sig_caught != 0; +} +static gboolean waitsignal_check(GSource *source) +{ + return sig_caught != 0; +} +static gboolean waitsignal_dispatch(GSource *source, GSourceFunc callback, gpointer user_data) +{ + SignalSource *ssrc = (SignalSource*) source; + ssrc->watcher->StopRun(); +} + /* daemon loop with glib */ void CCrashWatcher::GStartWatch() { - g_io_add_watch (m_pGio, G_IO_IN, handle_event_cb, this); + g_io_add_watch(m_pGio, G_IO_IN, handle_event_cb, this); + + GSourceFuncs waitsignal_funcs; + memset(&waitsignal_funcs, 0, sizeof(waitsignal_funcs)); + waitsignal_funcs.prepare = waitsignal_prepare; + waitsignal_funcs.check = waitsignal_check; + waitsignal_funcs.dispatch = waitsignal_dispatch; + //waitsignal_funcs.finalize = NULL; - already done + SignalSource *waitsignal_src = (SignalSource*) g_source_new(&waitsignal_funcs, sizeof(*waitsignal_src)); + waitsignal_src->watcher = this; + g_source_attach(&waitsignal_src->src, g_main_context_default()); + //enter the event loop - g_main_run (m_pMainloop); + g_main_run(m_pMainloop); } void CCrashWatcher::Run() @@ -594,6 +645,11 @@ void CCrashWatcher::Run() GStartWatch(); } +void CCrashWatcher::StopRun() +{ + g_main_quit(m_pMainloop); +} + vector_crash_infos_t CCrashWatcher::GetCrashInfos(const std::string &pUID) { vector_crash_infos_t retval; diff --git a/src/Daemon/CrashWatcher.h b/src/Daemon/CrashWatcher.h index e512478..d19da17 100644 --- a/src/Daemon/CrashWatcher.h +++ b/src/Daemon/CrashWatcher.h @@ -96,11 +96,12 @@ class CCrashWatcher /*FIXME not needed */ //DBus::Connection *m_pConn; CSettings *m_pSettings; - public: + public: //CCrashWatcher(const std::string& pPath,DBus::Connection &connection); CCrashWatcher(const std::string& pPath); - virtual ~CCrashWatcher(); + virtual ~CCrashWatcher(); void Run(); + void StopRun(); /* methods exported on dbus */ public: diff --git a/src/Daemon/Daemon.cpp b/src/Daemon/Daemon.cpp index ea769c5..c28331f 100644 --- a/src/Daemon/Daemon.cpp +++ b/src/Daemon/Daemon.cpp @@ -25,15 +25,15 @@ #include #include -CCrashWatcher *g_pCrashWatcher = NULL; +uint8_t sig_caught; -void terminate(int signal) +static void handle_fatal_signal(int signal) { - fprintf(stderr, "Got SIGINT/SIGTERM, cleaning up..\n"); - delete g_pCrashWatcher; - exit(0); + sig_caught = signal; } +CCrashWatcher *g_pCrashWatcher = NULL; + void print_help() { @@ -43,8 +43,8 @@ int main(int argc, char** argv) { int daemonize = 1; /*signal handlers */ - signal(SIGTERM, terminate); - signal(SIGINT, terminate); + signal(SIGTERM, handle_fatal_signal); + signal(SIGINT, handle_fatal_signal); try { @@ -97,7 +97,7 @@ int main(int argc, char** argv) { std::cerr << "Cannot create daemon: " << e.what() << std::endl; } - //do we need this? delete g_pCrashWatcher; + delete g_pCrashWatcher; return 1; /* Any exit is a failure. Normally we don't exit at all */ } -- cgit From db9ffc6dafce568e0b70d064411db81933d61eb4 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Fri, 17 Jul 2009 15:58:51 +0200 Subject: On exit, take care to emit consistent exit status. If we were Ctrl-C'ed, then we should exit by killing ourself, not exit(N). Signed-off-by: Denys Vlasenko --- src/Daemon/Daemon.cpp | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) (limited to 'src/Daemon') diff --git a/src/Daemon/Daemon.cpp b/src/Daemon/Daemon.cpp index c28331f..dff8de6 100644 --- a/src/Daemon/Daemon.cpp +++ b/src/Daemon/Daemon.cpp @@ -42,13 +42,14 @@ void print_help() int main(int argc, char** argv) { int daemonize = 1; - /*signal handlers */ + + /* signal handlers */ signal(SIGTERM, handle_fatal_signal); signal(SIGINT, handle_fatal_signal); + try { - - if (argc > 1) + if (argv[1]) { if (strcmp(argv[1], "-d") == 0) { @@ -97,7 +98,14 @@ int main(int argc, char** argv) { std::cerr << "Cannot create daemon: " << e.what() << std::endl; } + delete g_pCrashWatcher; - return 1; /* Any exit is a failure. Normally we don't exit at all */ -} + /* Take care to emit correct exit status */ + if (sig_caught) { + signal(sig_caught, SIG_DFL); + raise(sig_caught); + } + /* I think we never end up here */ + return 0; +} -- cgit