Dynamically allocated GMutex and GCond variables

The GLib documentation says statically allocated GMutex and GCond
variables do not need initialized.  This seems to imply magic on
first use.  That magic could be possible, but would be inefficient.
More likely is that they meant statical globals don't need initialization
because they are zero'd, where static locals are on the stack and could
be / are uninitialized?  But even that doesn't mesh with some runtime
behavior I've sometimes seen with my two statically allocated global
GMutex/GCond pairs:

+ Not running as daemon
+ Begin scanning /var/lib/corewatcher/...
GLib (gthread-posix.c): Unexpected error from C library during
'pthread_mutex_lock': Invalid argument.  Aborting.

Converting them to dynamically allocated variables, with the whole slough
of new/init/clear/free calls seems to fix this as expected.  Of course
the clear/free calls are never reached, since the threads only end/exit
via an (currently) unhandled signal and thus add to the list of things
about which valgrind complains at exit.

Signed-off-by: Tim Pepper <timothy.c.pepper@linux.intel.com>

diff --git a/src/coredump.c b/src/coredump.c
index 46c1a6a..6864df9 100644 (file)
--- a/src/coredump.c
+++ b/src/coredump.c
@@ -49,9 +49,9 @@
  * race where the condition is set before the thread is awaiting it and
  * thus is not woken.
  */
-static GMutex pq_mtx;
+GMutex *pq_mtx;
 static gboolean pq = FALSE;
-static GCond pq_work;
+GCond *pq_work;
 
 static char *get_release(void)
 {
@@ -578,9 +578,6 @@ int scan_core_folder(void __unused *unused)
        }
        fprintf(stderr, "+ Begin scanning %s...\n", core_folder);
        while(1) {
-               free(fullpath);
-               fullpath = NULL;
-
                entry = readdir(dir);
                if (!entry || !entry->d_name)
                        break;
@@ -603,15 +600,18 @@ int scan_core_folder(void __unused *unused)
                ret = move_core(fullpath, "to-process");
                if (ret == 0)
                        work++;
+
+               free(fullpath);
+               fullpath = NULL;
        }
        closedir(dir);
 
        if (work) {
                fprintf(stderr, "+ Found %d files, setting pq_work condition\n", work);
-               g_mutex_lock(&pq_mtx);
-               g_cond_signal(&pq_work);
+               g_mutex_lock(pq_mtx);
+               g_cond_signal(pq_work);
                pq = TRUE;
-               g_mutex_unlock(&pq_mtx);
+               g_mutex_unlock(pq_mtx);
        }
 
        fprintf(stderr, "+ End scanning %s...\n", core_folder);
@@ -630,13 +630,13 @@ void *scan_processed_folder(void __unused *unused)
        struct oops *oops = NULL;
 
        while(1) {
-               g_mutex_lock(&pq_mtx);
+               g_mutex_lock(pq_mtx);
                while (pq != TRUE) {
                        fprintf(stderr, "+ Awaiting work in %s...\n", processed_folder);
-                       g_cond_wait(&pq_work, &pq_mtx);
+                       g_cond_wait(pq_work, pq_mtx);
                }
                pq = FALSE;
-               g_mutex_unlock(&pq_mtx);
+               g_mutex_unlock(pq_mtx);
 
                fprintf(stderr, "+ Begin scanning %s...\n", processed_folder);
 
@@ -685,10 +685,10 @@ int scan_folders(void __unused *unused)
 {
        scan_core_folder(NULL);
 
-       g_mutex_lock(&pq_mtx);
-       g_cond_signal(&pq_work);
+       g_mutex_lock(pq_mtx);
+       g_cond_signal(pq_work);
        pq = TRUE;
-       g_mutex_unlock(&pq_mtx);
+       g_mutex_unlock(pq_mtx);
 
        return TRUE;
 }

diff --git a/src/corewatcher.c b/src/corewatcher.c
index eb373b3..c5b18e4 100644 (file)
--- a/src/corewatcher.c
+++ b/src/corewatcher.c
@@ -82,8 +82,6 @@ int main(int argc, char**argv)
        GThread *submit_thread = NULL;
        GThread *processing_thread = NULL;
 
-       g_thread_init (NULL);
-
 /*
  * Signal the kernel that we're not timing critical
  */
@@ -163,15 +161,23 @@ int main(int argc, char**argv)
        loop = g_main_loop_new(NULL, FALSE);
        loop = g_main_loop_ref(loop);
 
-       g_mutex_lock(&bt_mtx);
+       bt_mtx = g_new(GMutex, 1);
+       g_mutex_init(bt_mtx);
+       bt_work = g_new(GCond, 1);
+       g_cond_init(bt_work);
+       g_mutex_lock(bt_mtx);
        bt_hash = g_hash_table_new(g_str_hash, g_str_equal);
-       g_mutex_unlock(&bt_mtx);
+       g_mutex_unlock(bt_mtx);
        submit_thread = g_thread_new("corewatcher submit", submit_loop, NULL);
        if (submit_thread == NULL) {
                fprintf(stderr, "+ Unable to start submit thread...exiting\n");
                return EXIT_FAILURE;
        }
 
+       pq_mtx = g_new(GMutex, 1);
+       g_mutex_init(pq_mtx);
+       pq_work = g_new(GCond, 1);
+       g_cond_init(pq_work);
        processing_thread = g_thread_new("corewatcher processing", scan_processed_folder, NULL);
        if (processing_thread == NULL) {
                fprintf(stderr, "+ Unable to start processing thread...exiting\n");
@@ -212,6 +218,12 @@ int main(int argc, char**argv)
        g_main_loop_run(loop);
 out:
        g_main_loop_unref(loop);
+       g_cond_clear(bt_work);
+       g_cond_clear(pq_work);
+       g_mutex_clear(bt_mtx);
+       g_mutex_clear(pq_mtx);
+       g_free(bt_mtx);
+       g_free(pq_mtx);
 
        for (j = 0; j < url_count; j++)
                free(submit_url[j]);

diff --git a/src/corewatcher.h b/src/corewatcher.h
index 0730515..82412a1 100644 (file)
--- a/src/corewatcher.h
+++ b/src/corewatcher.h
@@ -56,13 +56,16 @@ struct oops {
 extern void *inotify_loop(void __unused *unused);
 
 /* submit.c */
-extern GMutex bt_mtx;
+extern GMutex *bt_mtx;
+extern GCond *bt_work;
 extern GHashTable *bt_hash;
 extern void queue_backtrace(struct oops *oops);
 extern char *replace_name(char *filename, char *replace, char *new);
 extern void *submit_loop(void __unused *unused);
 
 /* coredump.c */
+extern GMutex *pq_mtx;
+extern GCond *pq_work;
 extern int scan_folders(void __unused *unused);
 extern int scan_core_folder(void __unused *unused);
 extern void *scan_processed_folder(void __unused *unused);

diff --git a/src/submit.c b/src/submit.c
index 878b7a4..d5616c3 100644 (file)
--- a/src/submit.c
+++ b/src/submit.c
@@ -37,8 +37,8 @@
 
 #include "corewatcher.h"
 
-GMutex bt_mtx;
-static GCond bt_work;
+GMutex *bt_mtx;
+GCond *bt_work;
 GHashTable *bt_hash;
 static struct oops *bt_list = NULL;
 
@@ -51,12 +51,12 @@ void queue_backtrace(struct oops *oops)
        if (!oops || !oops->filename)
                return;
 
-       g_mutex_lock(&bt_mtx);
+       g_mutex_lock(bt_mtx);
 
        /* if this is already on bt_list / bt_hash, free and done */
        if (g_hash_table_lookup(bt_hash, oops->filename)) {
                FREE_OOPS(oops);
-               g_mutex_unlock(&bt_mtx);
+               g_mutex_unlock(bt_mtx);
                return;
        }
 
@@ -64,8 +64,8 @@ void queue_backtrace(struct oops *oops)
        oops->next = bt_list;
        bt_list = oops;
        g_hash_table_insert(bt_hash, oops->filename, oops->filename);
-       g_cond_signal(&bt_work);
-       g_mutex_unlock(&bt_mtx);
+       g_cond_signal(bt_work);
+       g_mutex_unlock(bt_mtx);
 }
 
 /*
@@ -79,7 +79,7 @@ static void print_queue(void)
        struct oops *oops = NULL, *next = NULL;
        int count = 0;
 
-       g_mutex_lock(&bt_mtx);
+       g_mutex_lock(bt_mtx);
        oops = bt_list;
        while (oops) {
                fprintf(stderr, "+ Submit text is:\n---[start of oops]---\n%s\n---[end of oops]---\n", oops->text);
@@ -89,7 +89,7 @@ static void print_queue(void)
                count++;
        }
        g_hash_table_remove_all(bt_hash);
-       g_mutex_unlock(&bt_mtx);
+       g_mutex_unlock(bt_mtx);
 }
 
 static size_t writefunction(void *ptr, size_t size, size_t nmemb, void __attribute((unused)) *stream)
@@ -170,9 +170,9 @@ void report_good_send(int *sentcount, struct oops *oops)
        rename(oops->filename, newfilename);
        free(newfilename);
 
-       g_mutex_lock(&bt_mtx);
+       g_mutex_lock(bt_mtx);
        g_hash_table_remove(bt_hash, oops->filename);
-       g_mutex_unlock(&bt_mtx);
+       g_mutex_unlock(bt_mtx);
        FREE_OOPS(oops);
 }
 
@@ -211,7 +211,7 @@ void *submit_loop(void __unused *unused)
        }
 
        while (1) {
-               g_mutex_lock(&bt_mtx);
+               g_mutex_lock(bt_mtx);
                while (!bt_list) {
                        if (requeue_list) {
                                bt_list = requeue_list;
@@ -220,13 +220,13 @@ void *submit_loop(void __unused *unused)
                        } else {
                                fprintf(stderr, "+ submit_loop() queue empty, awaiting new work\n");
                        }
-                       g_cond_wait(&bt_work, &bt_mtx);
+                       g_cond_wait(bt_work, bt_mtx);
                }
                fprintf(stderr, "+ submit_loop() checking for work\n");
                /* pull out current work and release the mutex */
                work_list = bt_list;
                bt_list = NULL;
-               g_mutex_unlock(&bt_mtx);
+               g_mutex_unlock(bt_mtx);
 
                /* net init */
                handle = curl_easy_init();