[ABRT PATCH] abrtd: remove "post-create" machinery. Related to #657

Denys Vlasenko dvlasenk at redhat.com
Thu Aug 1 10:46:05 UTC 2013


It is now handled to abrt-server.

Signed-off-by: Denys Vlasenko <dvlasenk at redhat.com>
---
 src/daemon/abrt-handle-event.c |   7 +
 src/daemon/abrtd.c             | 388 +----------------------------------------
 2 files changed, 16 insertions(+), 379 deletions(-)

diff --git a/src/daemon/abrt-handle-event.c b/src/daemon/abrt-handle-event.c
index e5f3600..73e53f7 100644
--- a/src/daemon/abrt-handle-event.c
+++ b/src/daemon/abrt-handle-event.c
@@ -402,6 +402,13 @@ int main(int argc, char **argv)
         if (post_create)
         {
             run_state->post_run_callback = is_crash_a_dup;
+            /*
+             * The post-create event cannot be run concurrently for more problem
+             * directories. The problem is in searching for duplicates process
+             * in case when two concurrently processed directories are duplicates
+             * of each other. Both of the directories are marked as duplicates
+             * of each other and are deleted.
+             */
             create_lockfile();
         }
 
diff --git a/src/daemon/abrtd.c b/src/daemon/abrtd.c
index 62511d3..5586953 100644
--- a/src/daemon/abrtd.c
+++ b/src/daemon/abrtd.c
@@ -60,76 +60,6 @@ static GIOChannel *channel_socket = NULL;
 static guint channel_id_socket = 0;
 static int child_count = 0;
 
-/* We spawn three kinds of children:
- * - handlers for accepted socket connections (abrt-server)
- * - handlers for detected uploads (abrt-upload)
- * - handlers for "post-create" + "notify[-dup]" events
- * First two kinds of children don't need spcial handling:
- * we only need to collect their exit status to avoid creating zombies.
- * Unfortunately, the third kind is not this simple: wee need to collect
- * their exit status _after_ we ate their output.
- * Since we can either (1) detect EOF first and do waitpid after it,
- * OR (2) we can get SIGCHLD and do waitpit first, we need to maintain a list
- * of known children of third kind, or else in the case (2) we won't be able
- * to save exit status. s_pid_list exists solely for this purpose.
- */
-static GList *s_pid_list;
-
-/*
- * The post-create event cannot be run concurrently for more problem
- * directories. The problem is in searching for duplicates process in case when
- * two concurrently processed directories are duplicates of each other. Both of
- * the directories are marked as duplicates of each other and are deleted.
- *
- * This queue is used for serialization of processing of created problem
- * directories.
- *
- * The GIO notify handler pushes new directories to this queue and if the queue was
- * empty before push then starts processing of the pushed directory.
- *
- * Directory processing code pops the processed directory from the queue at the
- * end of processing. If the queue is not empty starts processing of the next
- * directory which is on the top of the queue.
- */
-static GList *s_dir_queue;
-
-enum {
-    EVTYPE_POST_CREATE,
-    EVTYPE_NOTIFY,
-};
-struct event_processing_state
-{
-    int   event_type;
-    pid_t child_pid;
-    int   child_exitstatus;
-    int   child_stdout_fd;
-    struct strbuf *cmd_output;
-    char *dirname;
-    char *dup_of_dir;
-};
-static struct event_processing_state *new_event_processing_state(void)
-{
-    struct event_processing_state *p = xzalloc(sizeof(*p));
-    p->child_pid = -1;
-    p->child_stdout_fd = -1;
-    p->cmd_output = strbuf_new();
-    return p;
-}
-static void free_event_processing_state(struct event_processing_state *p)
-{
-    if (!p)
-        return;
-    /*
-    if (p->child_stdout_fd >= 0)
-        close(p->child_stdout_fd);
-    */
-    strbuf_free(p->cmd_output);
-    free(p->dirname);
-    free(p->dup_of_dir);
-    free(p);
-}
-
-
 /* Helpers */
 
 static guint add_watch_or_die(GIOChannel *channel, unsigned condition, GIOFunc func)
@@ -185,53 +115,6 @@ static void decrement_child_count(void)
     }
 }
 
-static pid_t spawn_event_handler_child(const char *dump_dir_name, const char *event_name, int *fdp)
-{
-    char *args[6];
-    args[0] = (char *) LIBEXEC_DIR"/abrt-handle-event";
-    args[1] = (char *) "-e";
-    args[2] = (char *) event_name;
-    args[3] = (char *) "--";
-    args[4] = (char *) dump_dir_name;
-    args[5] = NULL;
-
-    int pipeout[2];
-    int flags = EXECFLG_INPUT_NUL | EXECFLG_OUTPUT | EXECFLG_QUIET | EXECFLG_ERR2OUT;
-    VERB1 flags &= ~EXECFLG_QUIET;
-
-    pid_t child = fork_execv_on_steroids(flags, args, pipeout,
-                                         /*env_vec:*/ NULL, /*dir:*/ NULL,
-                                         /*uid(unused):*/ 0);
-    if (fdp)
-        *fdp = pipeout[0];
-    increment_child_count();
-    return child;
-}
-
-static int wait_for_child(struct event_processing_state *state)
-{
-    int status = 0;
-
-    if (state->child_pid < 0)
-        status = state->child_exitstatus;
-    else
-    {
-        if (safe_waitpid(state->child_pid, &status, 0) > 0)
-        {
-            state->child_pid = -1;
-            state->child_exitstatus = status;
-            decrement_child_count();
-        }
-        else /* should not happen */
-            perror_msg("waitpid(%d)", (int)state->child_pid);
-    }
-
-    s_pid_list = g_list_remove(s_pid_list, state);
-
-    return status;
-}
-
-
 /* Callback called by glib main loop when a client connects to ABRT's socket. */
 static gboolean server_socket_cb(GIOChannel *source, GIOCondition condition, gpointer ptr_unused)
 {
@@ -272,14 +155,7 @@ static gboolean server_socket_cb(GIOChannel *source, GIOCondition condition, gpo
     return TRUE;
 }
 
-
 /* Signal pipe handler */
-static gint compare_pids(gconstpointer a, gconstpointer b)
-{
-    const struct event_processing_state *state = a;
-    pid_t pid = (pid_t) (uintptr_t) b;
-    return state->child_pid != pid;
-}
 static gboolean handle_signal_cb(GIOChannel *gio, GIOCondition condition, gpointer ptr_unused)
 {
     uint8_t signo;
@@ -293,239 +169,15 @@ static gboolean handle_signal_cb(GIOChannel *gio, GIOCondition condition, gpoint
             s_exiting = 1;
         else
         {
-            pid_t pid;
-            int status;
-            while ((pid = safe_waitpid(-1, &status, WNOHANG)) > 0)
+            while (safe_waitpid(-1, NULL, WNOHANG) > 0)
             {
                 decrement_child_count();
-                GList *l = g_list_find_custom(s_pid_list, (gconstpointer) (uintptr_t) pid, compare_pids);
-                if (l)
-                {
-                    struct event_processing_state *state = l->data;
-                    state->child_pid = -1;
-                    state->child_exitstatus = status;
-                }
             }
         }
     }
     return TRUE; /* "please don't remove this event" */
 }
 
-static gboolean handle_event_output_cb(GIOChannel *gio, GIOCondition condition, gpointer ptr);
-
-/* Runs post-create event asynchronously. Uses GIOChanel for communication with event process. */
-static void run_post_create_on_dir_async(const char *name)
-{
-    struct event_processing_state *state = new_event_processing_state();
-    state->event_type = EVTYPE_POST_CREATE;
-    state->dirname = concat_path_file(g_settings_dump_location, name);
-
-    state->child_pid = spawn_event_handler_child(state->dirname, "post-create", &state->child_stdout_fd);
-    s_pid_list = g_list_prepend(s_pid_list, state);
-    ndelay_on(state->child_stdout_fd);
-
-    GIOChannel *channel_event_output = my_io_channel_unix_new(state->child_stdout_fd);
-    /*uint channel_id_event_output =*/ g_io_add_watch(channel_event_output,
-            G_IO_IN | G_IO_PRI | G_IO_HUP,
-            handle_event_output_cb,
-            state);
-}
-
-/* Event-processing child output handler (such as "post-create" event) */
-static gboolean handle_event_output_cb(GIOChannel *gio, GIOCondition condition, gpointer ptr)
-{
-    struct event_processing_state *state = ptr;
-
-    //log("Reading from event fd %d", state->child_stdout_fd);
-
-    /* Read streamed data and split lines */
-    for (;;)
-    {
-        char buf[250]; /* usually we get one line, no need to have big buf */
-        errno = 0;
-        gsize r = 0;
-        g_io_channel_read_chars(gio, buf, sizeof(buf) - 1, &r, NULL);
-        if (r <= 0)
-            break;
-        buf[r] = '\0';
-
-        /* split lines in the current buffer */
-        char *raw = buf;
-        char *newline;
-        while ((newline = strchr(raw, '\n')) != NULL)
-        {
-            *newline = '\0';
-            strbuf_append_str(state->cmd_output, raw);
-            char *msg = state->cmd_output->buf;
-
-            /* Hmm, DUP_OF_DIR: ends up in syslog. move log() into 'else'? */
-            log("%s", msg);
-            if (state->event_type == EVTYPE_POST_CREATE
-             && strncmp("DUP_OF_DIR: ", msg, strlen("DUP_OF_DIR: ")) == 0
-            ) {
-                free(state->dup_of_dir);
-                state->dup_of_dir = xstrdup(msg + strlen("DUP_OF_DIR: "));
-            }
-
-            strbuf_clear(state->cmd_output);
-            /* jump to next line */
-            raw = newline + 1;
-        }
-
-        /* beginning of next line. the line continues by next read */
-        strbuf_append_str(state->cmd_output, raw);
-    }
-
-    if (errno == EAGAIN)
-    {
-        /* We got all buffered data, but fd is still open. Done for now */
-        //log("EAGAIN on fd %d", state->child_stdout_fd);
-        return TRUE; /* "glib, please don't remove this event (yet)" */
-    }
-
-    /* EOF/error */
-
-    /* Wait for child to actually exit, collect status */
-    int status = wait_for_child(state);
-
-    /* If it was a "notify[-dup]" event, then we're done */
-    if (state->event_type == EVTYPE_NOTIFY)
-        goto ret;
-
-    /* Else: it was "post-create" event */
-
-    /* exit 0 means "this is a good, non-dup dir" */
-    /* exit with 1 + "DUP_OF_DIR: dir" string => dup */
-    if (status != 0)
-    {
-        if (WIFSIGNALED(status))
-        {
-            log("'post-create' on '%s' killed by signal %d",
-                            state->dirname, WTERMSIG(status));
-            goto delete_bad_dir;
-        }
-        /* else: it is WIFEXITED(status) */
-        else if (!state->dup_of_dir)
-        {
-            log("'post-create' on '%s' exited with %d",
-                            state->dirname, WEXITSTATUS(status));
-            goto delete_bad_dir;
-        }
-    }
-
-    char *work_dir = state->dup_of_dir ? state->dup_of_dir : state->dirname;
-
-    /* Load problem_data (from the *first dir* if this one is a dup) */
-    struct dump_dir *dd = dd_opendir(work_dir, /*flags:*/ 0);
-    if (!dd)
-        /* dd_opendir already emitted error msg */
-        goto delete_bad_dir;
-
-    /* Update count */
-    char *count_str = dd_load_text_ext(dd, FILENAME_COUNT, DD_FAIL_QUIETLY_ENOENT);
-    unsigned long count = strtoul(count_str, NULL, 10);
-
-    /* Don't increase crash count if we are working with newly uploaded
-     * directory (remote crash) which already has its crash count set.
-     */
-    if ((status != 0 && state->dup_of_dir) || count == 0)
-    {
-        count++;
-        char new_count_str[sizeof(long)*3 + 2];
-        sprintf(new_count_str, "%lu", count);
-        dd_save_text(dd, FILENAME_COUNT, new_count_str);
-
-        /* This condition can be simplified to either
-         * (status * != 0 && * state->dup_of_dir) or (count == 1). But the
-         * chosen form is much more reliable and safe. We must not call
-         * dd_opendir() to locked dd otherwise we go into a deadlock.
-         */
-        if (strcmp(dd->dd_dirname, state->dirname) != 0)
-        {
-            /* Update the last occurrence file by the time file of the new problem */
-            struct dump_dir *new_dd = dd_opendir(state->dirname, DD_OPEN_READONLY);
-            char *last_ocr = NULL;
-            if (new_dd)
-            {
-                /* TIME must exists in a valid dump directory but we don't want to die
-                 * due to broken duplicated dump directory */
-                last_ocr = dd_load_text_ext(new_dd, FILENAME_TIME,
-                            DD_LOAD_TEXT_RETURN_NULL_ON_FAILURE | DD_FAIL_QUIETLY_ENOENT);
-                dd_close(new_dd);
-            }
-            else
-            {   /* dd_opendir() already produced a message with good information about failure */
-                error_msg("Can't read the last occurrence file from the new dump directory.");
-            }
-
-            if (!last_ocr)
-            {   /* the new dump directory may lie in the dump location for some time */
-                log("Using current time for the last occurrence file which may be incorrect.");
-                time_t t = time(NULL);
-                last_ocr = xasprintf("%lu", (long)t);
-            }
-
-            dd_save_text(dd, FILENAME_LAST_OCCURRENCE, last_ocr);
-
-            free(last_ocr);
-        }
-    }
-
-    /* Reset mode/uig/gid to correct values for all files created by event run */
-    dd_sanitize_mode_and_owner(dd);
-
-    dd_close(dd);
-
-    if (!state->dup_of_dir)
-        log("New problem directory %s, processing", work_dir);
-    else
-    {
-        log("Deleting problem directory %s (dup of %s)",
-                strrchr(state->dirname, '/') + 1,
-                strrchr(state->dup_of_dir, '/') + 1);
-        delete_dump_dir(state->dirname);
-    }
-
-    /* Run "notify[-dup]" event */
-    int fd;
-    state->child_pid = spawn_event_handler_child(
-                work_dir,
-                (state->dup_of_dir ? "notify-dup" : "notify"),
-                &fd
-    );
-    s_pid_list = g_list_prepend(s_pid_list, state);
-    ndelay_on(fd);
-    //log("Started notify, fd %d -> %d", fd, state->child_stdout_fd);
-    xmove_fd(fd, state->child_stdout_fd);
-    state->event_type = EVTYPE_NOTIFY;
-    return TRUE; /* "glib, please don't remove this event (yet)" */
-
-
- delete_bad_dir:
-    log("Deleting problem directory '%s'", state->dirname);
-    delete_dump_dir(state->dirname);
-
- ret:
-    //log("Closing event fd %d", state->child_stdout_fd);
-    free_event_processing_state(state);
-
-    /* We stop using this channel */
-    g_io_channel_unref(gio);
-
-    /* pop the currently processed directory from the incoming queue */
-    char *name = s_dir_queue->data;
-    s_dir_queue = g_list_remove(s_dir_queue, name);
-    free(name);
-
-    /* if the queue is not empty process directory from */
-    /* the top of the incoming queue */
-    if (s_dir_queue)
-        run_post_create_on_dir_async(s_dir_queue->data);
-
-    return FALSE; /* "glib, please remove this events source!" */
-    /* Removing will also drop the last ref to this gio, closing/freeing it */
-}
-
 static void ensure_writable_dir(const char *dir, mode_t mode, const char *user)
 {
     struct stat sb;
@@ -666,16 +318,19 @@ static gboolean handle_inotify_cb(GIOChannel *gio, GIOCondition condition, gpoin
              * but this handler is used for 'g_settings_sWatchCrashdumpArchiveDir' too
              */
             log("Recreating deleted dump location '%s'", g_settings_dump_location);
-            inotify_rm_watch(g_io_channel_unix_get_fd(gio), event->wd);
+
+            int inotify_fd = g_io_channel_unix_get_fd(gio);
+            inotify_rm_watch(inotify_fd, event->wd);
             sanitize_dump_dir_rights();
-            if (inotify_add_watch(g_io_channel_unix_get_fd(gio), g_settings_dump_location, IN_DUMP_LOCATION_FLAGS) < 0)
+            if (inotify_add_watch(inotify_fd, g_settings_dump_location, IN_DUMP_LOCATION_FLAGS) < 0)
             {
                 perror_msg_and_die("inotify_add_watch failed on recreated '%s'", g_settings_dump_location);
             }
 
             continue;
         }
-
+/* We no longer watch for subdirectory creations */
+#if 0
         if (!(event->mask & IN_ISDIR) || !name)
         {
             /* ignore lock files and such */
@@ -690,33 +345,8 @@ static gboolean handle_inotify_cb(GIOChannel *gio, GIOCondition condition, gpoin
             continue;
         }
         log("Directory '%s' creation detected", name);
-
-        if (g_settings_nMaxCrashReportsSize > 0)
-        {
-            char *worst_dir = NULL;
-            while (g_settings_nMaxCrashReportsSize > 0
-             && get_dirsize_find_largest_dir(g_settings_dump_location, &worst_dir, name) / (1024*1024) >= g_settings_nMaxCrashReportsSize
-             && worst_dir
-            ) {
-                log("Size of '%s' >= %u MB, deleting '%s'",
-                    g_settings_dump_location, g_settings_nMaxCrashReportsSize, worst_dir);
-                char *d = concat_path_file(g_settings_dump_location, worst_dir);
-                free(worst_dir);
-                worst_dir = NULL;
-                delete_dump_dir(d);
-                free(d);
-            }
-            free(worst_dir);
-        }
-
-        const bool empty_queue = s_dir_queue == NULL;
-        /* push the new directory to the end of the incoming queue */
-        s_dir_queue = g_list_append(s_dir_queue, xstrdup(name));
-
-        /* if the queue was empty process the current directory */
-        if (empty_queue)
-            run_post_create_on_dir_async(name);
-
+        ...
+#endif
     } /* while */
 
     free(buf);
-- 
1.8.1.4



More information about the Crash-catcher mailing list