implement abrtd -t TIMEOUT_IN_SEC

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>

4 files changed, 105 insertions, 88 deletions

diff --git a/src/Daemon/CrashWatcher.cpp b/src/Daemon/CrashWatcher.cpp
index 94fb9d79..ad3f37ad 100644
--- a/src/Daemon/CrashWatcher.cpp
+++ b/src/Daemon/CrashWatcher.cpp
@@ -163,18 +163,9 @@ static void* create_report(void* arg)
     }
     catch (CABRTException& e)
     {
-        if (e.type() == EXCEP_FATAL)
-        {
-            set_client_name(NULL);
-            /* free strduped strings */
-            free(thread_data->UUID);
-            free(thread_data->UID);
-            free(thread_data->peer);
-            free(thread_data);
-            throw e;
-        }
         warn_client(e.what());
     }
+    catch (...) {}
     set_client_name(NULL);
 
     /* free strduped strings */
@@ -193,10 +184,12 @@ int CreateReportThread(const char* pUUID, const char* pUID, int force, const cha
     thread_data->UID = xstrdup(pUID);
     thread_data->force = force;
     thread_data->peer = xstrdup(pSender);
+
 //TODO: do we need this?
 //pthread_attr_t attr;
 //pthread_attr_init(&attr);
 //pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED);
+
     int r = pthread_create(&thread_data->thread_id, NULL, create_report, thread_data);
     if (r != 0)
     {
@@ -208,11 +201,9 @@ int CreateReportThread(const char* pUUID, const char* pUID, int force, const cha
          * or ulimit is exceeded (someone floods us with CreateReport() dbus calls?)
          */
         error_msg("Can't create thread");
+        return r;
     }
-    else
-    {
-        VERB3 log("Thread %llx created", (unsigned long long)thread_data->thread_id);
-    }
+    VERB3 log("Thread %llx created", (unsigned long long)thread_data->thread_id);
 //pthread_attr_destroy(&attr);
     return r;
 }
diff --git a/src/Daemon/Daemon.cpp b/src/Daemon/Daemon.cpp
index b509b9fc..4a4f6918 100644
--- a/src/Daemon/Daemon.cpp
+++ b/src/Daemon/Daemon.cpp
@@ -107,12 +107,12 @@ typedef struct cron_callback_data_t
 
 
 static uint8_t s_sig_caught;
-static GMainLoop* g_pMainloop;
+static int s_signal_pipe[2];
+static unsigned s_timeout;
+static bool s_exiting;
 
 CCommLayerServer* g_pCommLayer;
 
-pthread_mutex_t g_pJobsMutex;
-
 
 /* Is it "." or ".."? */
 /* abrtlib candidate */
@@ -323,13 +323,13 @@ static int SetUpCron()
             vector_pair_string_string_t::iterator it_ar = it_c->second.begin();
             for (; it_ar != it_c->second.end(); it_ar++)
             {
-                cron_callback_data_t* cronOneCallbackData = new cron_callback_data_t((*it_ar).first, (*it_ar).second, timeout);
+                cron_callback_data_t* cronOneCallbackData = new cron_callback_data_t(it_ar->first, it_ar->second, timeout);
                 g_timeout_add_seconds_full(G_PRIORITY_DEFAULT,
                                            timeout,
                                            cron_activation_one_cb,
                                            static_cast<gpointer>(cronOneCallbackData),
                                            cron_delete_callback_data_cb);
-                cron_callback_data_t* cronResheduleCallbackData = new cron_callback_data_t((*it_ar).first, (*it_ar).second, 24 * 60 * 60);
+                cron_callback_data_t* cronResheduleCallbackData = new cron_callback_data_t(it_ar->first, it_ar->second, 24 * 60 * 60);
                 g_timeout_add_seconds_full(G_PRIORITY_DEFAULT,
                                            timeout,
                                            cron_activation_reshedule_cb,
@@ -457,60 +457,38 @@ static int Lock()
 static void handle_fatal_signal(int signal)
 {
     s_sig_caught = signal;
+    write(s_signal_pipe[1], &s_sig_caught, 1);
 }
 
-/* One of our event sources is s_sig_caught when it becomes != 0.
- * glib machinery we need to hook it up to the main loop:
- * 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.
- */
-static gboolean waitsignal_prepare(GSource *source, gint *timeout_)
-{
-    /* We depend on the fact that in Unix, poll() is interrupted
-     * by caught signals (in returns EINTR). Thus we do not need to set
-     * a small timeout here: infinite timeout (-1) works too */
-    *timeout_ = -1;
-    return s_sig_caught != 0;
-}
-static gboolean waitsignal_check(GSource *source)
-{
-    return s_sig_caught != 0;
-}
-static gboolean waitsignal_dispatch(GSource *source, GSourceFunc callback, gpointer user_data)
+/* Signal pipe handler */
+static gboolean handle_signal_cb(GIOChannel *gio, GIOCondition condition, gpointer ptr_unused)
 {
-    g_main_quit(g_pMainloop);
-    return 1;
+    char signo;
+    gsize len = 0;
+    g_io_channel_read(gio, &signo, 1, &len);
+    if (len == 1)
+    {
+        /* we did receive a signal */
+        s_exiting = 1;
+        return TRUE;
+    }
+    return FALSE;
 }
 
 /* Inotify handler */
-static gboolean handle_event_cb(GIOChannel *gio, GIOCondition condition, gpointer ptr_unused)
+static gboolean handle_inotify_cb(GIOChannel *gio, GIOCondition condition, gpointer ptr_unused)
 {
     /* 128 simultaneous actions */
 #define INOTIFY_BUFF_SIZE ((sizeof(struct inotify_event) + FILENAME_MAX)*128)
-    GIOError err;
-    char *buf = new char[INOTIFY_BUFF_SIZE];
+    char *buf = (char*)xmalloc(INOTIFY_BUFF_SIZE);
     gsize len;
     gsize i = 0;
     errno = 0;
-    err = g_io_channel_read(gio, buf, INOTIFY_BUFF_SIZE, &len);
+    GIOError err = g_io_channel_read(gio, buf, INOTIFY_BUFF_SIZE, &len);
     if (err != G_IO_ERROR_NONE)
     {
         perror_msg("Error reading inotify fd");
-        delete[] buf;
+        free(buf);
         return FALSE;
     }
     /* reconstruct each event and send message to the dbus */
@@ -602,21 +580,57 @@ static gboolean handle_event_cb(GIOChannel *gio, GIOCondition condition, gpointe
             warn_client(e.what());
             if (e.type() == EXCEP_FATAL)
             {
-                delete[] buf;
+                free(buf);
                 return -1;
             }
         }
         catch (...)
         {
-            delete[] buf;
+            free(buf);
             throw;
         }
     } /* while */
 
-    delete[] buf;
+    free(buf);
     return TRUE;
 }
 
+/* Run main loop with idle timeout.
+ * Basically, almost like glib's g_main_run(loop)
+ */
+static void run_main_loop(GMainLoop* loop)
+{
+    GMainContext *context = g_main_loop_get_context(loop);
+
+    while (!s_exiting)
+    {
+        /* we have just a handful of sources, 32 should be ample */
+        const unsigned NUM_POLLFDS = 32;
+        GPollFD fds[NUM_POLLFDS];
+        gboolean some_ready;
+        gint max_priority;
+        gint timeout;
+
+        some_ready = g_main_context_prepare(context, &max_priority);
+        if (some_ready)
+            g_main_context_dispatch(context);
+
+        gint nfds = g_main_context_query(context, max_priority, &timeout, fds, NUM_POLLFDS);
+        if (nfds > NUM_POLLFDS)
+            error_msg_and_die("Internal error");
+
+        if (s_timeout)
+            alarm(s_timeout);
+	g_poll(fds, nfds, timeout);
+        if (s_timeout)
+            alarm(0);
+
+        some_ready = g_main_context_check(context, max_priority, fds, nfds);
+        if (some_ready)
+            g_main_context_dispatch(context);
+    }
+}
+
 static void start_syslog_logging()
 {
     /* Open stdin to /dev/null */
@@ -670,8 +684,10 @@ int main(int argc, char** argv)
     textdomain(PACKAGE);
 #endif
 
-    while ((opt = getopt(argc, argv, "dsv")) != -1)
+    while ((opt = getopt(argc, argv, "dsvt:")) != -1)
     {
+        unsigned long ul;
+
         switch (opt)
         {
         case 'd':
@@ -683,20 +699,32 @@ int main(int argc, char** argv)
         case 'v':
             g_verbose++;
             break;
+        case 't':
+            char *end;
+            errno = 0;
+            s_timeout = ul = strtoul(optarg, &end, 0);
+            if (errno == 0 && *end == '\0' && ul <= INT_MAX)
+                break;
+            /* fall through to error */
         default:
             error_msg_and_die(
                 "Usage: abrtd [-dv]\n"
         	"\nOptions:"
                 "\n\t-d\tDo not daemonize"
                 "\n\t-s\tLog to syslog even with -d"
+                "\n\t-t SEC\tExit after SEC seconds of inactivity"
                 "\n\t-v\tVerbose"
             );
         }
     }
 
     msg_prefix = "abrtd: "; /* for log(), error_msg() and such */
+
+    xpipe(s_signal_pipe);
     signal(SIGTERM, handle_fatal_signal);
-    signal(SIGINT, handle_fatal_signal);
+    signal(SIGINT,  handle_fatal_signal);
+    if (s_timeout)
+        signal(SIGALRM, handle_fatal_signal);
 
     /* Daemonize unless -d */
     if (daemonize)
@@ -732,7 +760,9 @@ int main(int argc, char** argv)
             start_syslog_logging();
     }
 
-    GIOChannel* pGio = NULL;
+    GMainLoop* pMainloop = NULL;
+    GIOChannel* pGiochannel_inotify = NULL;
+    GIOChannel* pGiochannel_signal = NULL;
     bool lockfile_created = false;
     bool pidfile_created = false;
     CCrashWatcher watcher;
@@ -740,10 +770,9 @@ int main(int argc, char** argv)
     /* Initialization */
     try
     {
-        pthread_mutex_init(&g_pJobsMutex, NULL); /* never fails */
         init_daemon_logging(&watcher);
         VERB1 log("Creating glib main loop");
-        g_pMainloop = g_main_loop_new(NULL, FALSE);
+        pMainloop = g_main_loop_new(NULL, FALSE);
         /* Watching DEBUG_DUMPS_DIR for new files... */
         VERB1 log("Initializing inotify");
 // Enabled again since we have new abrt-pyhook-helper, remove comment when verified to work
@@ -775,18 +804,12 @@ int main(int argc, char** argv)
         if (g_pCommLayer->m_init_error)
             throw 1;
         VERB1 log("Adding inotify watch to glib main loop");
-        pGio = g_io_channel_unix_new(inotify_fd);
-        g_io_add_watch(pGio, G_IO_IN, handle_event_cb, NULL);
+        pGiochannel_inotify = g_io_channel_unix_new(inotify_fd);
+        g_io_add_watch(pGiochannel_inotify, G_IO_IN, handle_inotify_cb, NULL);
         /* Add an event source which waits for INT/TERM signal */
-        VERB1 log("Adding signal watch to glib main loop");
-        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 */
-        GSource *waitsignal_src = (GSource*) g_source_new(&waitsignal_funcs, sizeof(*waitsignal_src));
-        g_source_attach(waitsignal_src, g_main_context_default());
+        VERB1 log("Adding signal pipe watch to glib main loop");
+        pGiochannel_signal = g_io_channel_unix_new(s_signal_pipe[0]);
+        g_io_add_watch(pGiochannel_signal, G_IO_IN, handle_signal_cb, NULL);
         /* Mark the territory */
         VERB1 log("Creating lock file");
         if (Lock() != 0)
@@ -822,7 +845,7 @@ int main(int argc, char** argv)
         /* This may take a while, therefore we don't do it in init section */
         FindNewDumps(DEBUG_DUMPS_DIR);
         log("Running...");
-        g_main_run(g_pMainloop);
+        run_main_loop(pMainloop);
     }
     catch (CABRTException& e)
     {
@@ -841,8 +864,12 @@ int main(int argc, char** argv)
         unlink(VAR_RUN_PIDFILE);
     if (lockfile_created)
         unlink(VAR_RUN_LOCK_FILE);
-    if (pGio)
-        g_io_channel_unref(pGio);
+
+    if (pGiochannel_signal)
+        g_io_channel_unref(pGiochannel_signal);
+    if (pGiochannel_inotify)
+        g_io_channel_unref(pGiochannel_inotify);
+
     delete g_pCommLayer;
     if (g_pPluginManager)
     {
@@ -850,13 +877,11 @@ int main(int argc, char** argv)
         g_pPluginManager->UnLoadPlugins();
         delete g_pPluginManager;
     }
-    if (g_pMainloop)
-        g_main_loop_unref(g_pMainloop);
-    if (pthread_mutex_destroy(&g_pJobsMutex) != 0)
-        error_msg("Threading error: job mutex locked");
+    if (pMainloop)
+        g_main_loop_unref(pMainloop);
 
     /* Exiting */
-    if (s_sig_caught)
+    if (s_sig_caught && s_sig_caught != SIGALRM)
     {
         error_msg_and_die("Got signal %d, exiting", s_sig_caught);
         signal(s_sig_caught, SIG_DFL);
diff --git a/src/Daemon/Daemon.h b/src/Daemon/Daemon.h
index c9a653ac..ac998b93 100644
--- a/src/Daemon/Daemon.h
+++ b/src/Daemon/Daemon.h
@@ -39,6 +39,4 @@ extern CPluginManager* g_pPluginManager;
  */
 extern set_string_t g_setBlackList;
 
-extern pthread_mutex_t g_pJobsMutex;
-
 #endif
diff --git a/src/Daemon/com.redhat.abrt.service b/src/Daemon/com.redhat.abrt.service
index 163f276c..e12c7ee3 100644
--- a/src/Daemon/com.redhat.abrt.service
+++ b/src/Daemon/com.redhat.abrt.service
@@ -1,4 +1,7 @@
 [D-BUS Service]
 Name=com.redhat.abrt
-Exec=/usr/sbin/abrtd -ds
+# We use small timeout of 33 seconds in order to make
+# "abrtd exited while clients existed but were idle" situations
+# to be triggered more often and thus better tested
+Exec=/usr/sbin/abrtd -ds -t33
 User=root