Merge branch 'master' of ssh://git.fedorahosted.org/git/abrt

5 files changed, 159 insertions, 63 deletions

diff --git a/lib/MiddleWare/PluginManager.cpp b/lib/MiddleWare/PluginManager.cpp
index b47d70ff..8ce32f45 100644
--- a/lib/MiddleWare/PluginManager.cpp
+++ b/lib/MiddleWare/PluginManager.cpp
@@ -133,7 +133,7 @@ void CPluginManager::RegisterPlugin(const std::string& pName)
                 return;
             }
             m_mapPlugins[pName] = plugin;
-            comm_layer_inner_debug("Registred plugin " + pName + "("
+            comm_layer_inner_debug("Registered plugin " + pName + "("
                                   + std::string(plugin_type_str_t[m_mapABRTPlugins[pName]->GetType()])
                                   + ")");
         }
diff --git a/src/Daemon/CrashWatcher.cpp b/src/Daemon/CrashWatcher.cpp
index 1a87f8d5..dfb86794 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
             {
@@ -438,27 +446,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()
@@ -556,18 +575,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 */
@@ -577,17 +592,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()
@@ -621,22 +636,78 @@ 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()
 {
-    Debug("Runnig...");
+    Debug("Running...");
     Lock();
     CreatePidFile();
     GStartWatch();
 }
 
+void CCrashWatcher::StopRun()
+{
+    g_main_quit(m_pMainloop);
+}
+
 vector_crash_infos_t CCrashWatcher::GetCrashInfos(const std::string &pUID)
 {
     vector_crash_infos_t retval;
@@ -689,7 +760,7 @@ vector_crash_infos_t CCrashWatcher::GetCrashInfos(const std::string &pUID)
 
     //retval = m_pMW->GetCrashInfos(pUID);
     //Notify("Sent crash info");
-	return retval;
+        return retval;
 }
 
 uint64_t CCrashWatcher::CreateReport_t(const std::string &pUUID,const std::string &pUID)
diff --git a/src/Daemon/CrashWatcher.h b/src/Daemon/CrashWatcher.h
index 23322d8b..0ed207bf 100644
--- a/src/Daemon/CrashWatcher.h
+++ b/src/Daemon/CrashWatcher.h
@@ -107,11 +107,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 94f5e666..dff8de6c 100644
--- a/src/Daemon/Daemon.cpp
+++ b/src/Daemon/Daemon.cpp
@@ -21,16 +21,19 @@
 #include "ABRTException.h"
 #include <iostream>
 #include <cstdio>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
 
-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()
 {
 
@@ -39,13 +42,14 @@ void print_help()
 int main(int argc, char** argv)
 {
     int daemonize = 1;
-    /*signal handlers */
-    signal(SIGTERM, terminate);
-    signal(SIGINT, terminate);
+
+    /* 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)
             {
@@ -54,7 +58,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 +80,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 +98,14 @@ int main(int argc, char** argv)
     {
         std::cerr << "Cannot create daemon: " << e.what() << std::endl;
     }
-}
 
+    delete g_pCrashWatcher;
+
+    /* 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;
+}
diff --git a/src/Daemon/abrt.conf b/src/Daemon/abrt.conf
index 4255e023..fa6a9b37 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]