fix: potential fix for malloc deadlock in some corner cases (#30)

Author: thefab

src/Makefile

@@ -32,11 +32,11 @@ else
 	DEBUG_CFLAGS=-O2 -Os
 endif
 ifeq ($(shell expr $(GCC_VERSION) \< "8.0.0" ), 1)
-    _CFLAGS=$(CFLAGS) -I. $(shell pkg-config --cflags $(_STATIC_EXTRA_OPT) glib-2.0) -fPIC -Wall -std=c99 -Wextra -pedantic -Wshadow -Wstrict-overflow -Wno-deprecated-declarations -fno-strict-aliasing -DG_LOG_DOMAIN=\"log_proxy\" $(DEBUG_CFLAGS) $(COVERAGE_CFLAGS)
+    _CFLAGS=$(CFLAGS) -I. $(shell pkg-config --cflags $(_STATIC_EXTRA_OPT) glib-2.0 gthread-2.0) -fPIC -Wall -std=c99 -Wextra -pedantic -Wshadow -Wstrict-overflow -Wno-deprecated-declarations -fno-strict-aliasing -DG_LOG_DOMAIN=\"log_proxy\" $(DEBUG_CFLAGS) $(COVERAGE_CFLAGS)
 else
-    _CFLAGS=$(CFLAGS) -I. $(shell pkg-config --cflags $(_STATIC_EXTRA_OPT) glib-2.0) -fPIC -Wall -std=c99 -Wextra -pedantic -Wshadow -Wstrict-overflow -Wno-deprecated-declarations -Wno-cast-function-type -fno-strict-aliasing -DG_LOG_DOMAIN=\"log_proxy\" $(DEBUG_CFLAGS) $(COVERAGE_CFLAGS)
+    _CFLAGS=$(CFLAGS) -I. $(shell pkg-config --cflags $(_STATIC_EXTRA_OPT) glib-2.0 gthread-2.0) -fPIC -Wall -std=c99 -Wextra -pedantic -Wshadow -Wstrict-overflow -Wno-deprecated-declarations -Wno-cast-function-type -fno-strict-aliasing -DG_LOG_DOMAIN=\"log_proxy\" $(DEBUG_CFLAGS) $(COVERAGE_CFLAGS)
 endif
-_LDFLAGS=$(LDFLAGS) -L. $(shell pkg-config --libs $(_STATIC_EXTRA_OPT) glib-2.0) $(shell echo '$(FORCE_RPATH_STR)' |sed 's/@/$$/g' |sed s/~/"'"/g) -lrt
+_LDFLAGS=$(LDFLAGS) -L. $(shell pkg-config --libs $(_STATIC_EXTRA_OPT) glib-2.0 gthread-2.0) $(shell echo '$(FORCE_RPATH_STR)' |sed 's/@/$$/g' |sed s/~/"'"/g) -lrt
 
 OBJECTS=util.o control.o out.o
 BINARIES=log_proxy log_proxy_wrapper

src/log_proxy.c

@@ -17,13 +17,19 @@
 
 struct sigaction sigact;
 static gboolean first_iteration = TRUE;
+static GMutex *mutex = NULL;
+static GAsyncQueue *queue = NULL;
+static volatile gboolean stop_signal = FALSE;
+static GIOChannel *in = NULL;
 
 gint _list_compare(gconstpointer a, gconstpointer b) {
     gchar *ca = (gchar *) a;
     gchar *cb = (gchar *) b;
     return g_strcmp0(cb, ca);
 }
 
+#define UNUSED(x) (void)(x)
+
 void clean_too_old_files() {
     gchar *dirpath = g_path_get_dirname(log_file);
     GDir *dir = g_dir_open(dirpath, 0, NULL);
@@ -92,12 +98,11 @@ gboolean rotate() {
 
 void signal_handler(int signum) {
     if ((signum == SIGTERM) || (signum == SIGTERM)) {
-        // nice exit to execute exit_handler
-        exit(0);
+        stop_signal = TRUE;
     }
 }
 
-static void every_second(int sig) {
+static void every_second() {
     int fd = lock_control_file(log_file);
     if (fd >= 0) {
         if (first_iteration) {
@@ -111,19 +116,11 @@ static void every_second(int sig) {
             destroy_output_channel();
             init_output_channel(log_file, use_locks, TRUE, chmod_str, chown_str, chgrp_str);
             unlock_control_file(fd);
-            if (sig > 0) {
-                // if sig<0, this is the final call before program end
-                alarm(1);
-            }
             return;
         }
         glong size = get_file_size(log_file);
         if (size < 0) {
             unlock_control_file(fd);
-            if (sig > 0) {
-                // if sig<0, this is the final call before program end
-                alarm(1);
-            }
             return;
         }
         gboolean must_rotate = FALSE;
@@ -149,18 +146,53 @@ static void every_second(int sig) {
         }
         unlock_control_file(fd);
     }
-    if (sig > 0) {
-        // if sig<0, this is the final call before program end
-        alarm(1);
+}
+
+gpointer stop_thread(gpointer data) {
+    // we do this here and not in signal_handler to avoid malloc in signal
+    // handlers
+    UNUSED(data);
+    while (stop_signal == FALSE) {
+        sleep(1);
     }
+    g_async_queue_push(queue, GINT_TO_POINTER(2));
+    return NULL;
 }
 
-void init_every_second_signal() {
-    sigact.sa_handler = every_second;
-    sigemptyset(&sigact.sa_mask);
-    sigact.sa_flags = 0;
-    sigaction(SIGALRM, &sigact, (struct sigaction *)NULL);
-    alarm(1);
+gpointer management_thread(gpointer data) {
+    gpointer qdata;
+    GTimeVal tval;
+    gint stop_flag = 0;
+    UNUSED(data);
+    while (TRUE) {
+        g_get_current_time(&tval);
+        g_time_val_add(&tval, 1000000);
+        qdata = g_async_queue_timed_pop(queue, &tval);
+        if (qdata != NULL) {
+            stop_flag = GPOINTER_TO_INT(qdata);
+        }
+        g_mutex_lock(mutex);
+        every_second();
+        g_mutex_unlock(mutex);
+        if (qdata != NULL) {
+            break;
+        }
+    }
+    if (stop_flag == 2) {
+        // in this case (sigterm), we prefer to keep the mutex
+        g_mutex_lock(mutex);
+    }
+    destroy_output_channel();
+    if (rm_fifo_at_exit == TRUE) {
+        if (fifo != NULL) {
+            g_unlink(fifo);
+        }
+    }
+    if (stop_flag == 2) {
+        // we exit here for sigterm as main thread is blocked in reading
+        exit(0);
+    }
+    return NULL;
 }
 
 void init_or_reinit_output_channel(const gchar *lg_file, gboolean us_locks) {
@@ -174,14 +206,6 @@ void init_or_reinit_output_channel(const gchar *lg_file, gboolean us_locks) {
     unlock_control_file(lock_fd);
 }
 
-void exit_handler() {
-    if (rm_fifo_at_exit == TRUE) {
-        if (fifo != NULL) {
-            g_unlink(fifo);
-        }
-    }
-}
-
 int main(int argc, char *argv[])
 {
     GOptionContext *context;
@@ -198,7 +222,9 @@ int main(int argc, char *argv[])
         g_print("%s", g_option_context_get_help(context, TRUE, NULL));
         exit(1);
     }
-    atexit(exit_handler);
+    g_thread_init(NULL);
+    mutex = g_mutex_new();
+    queue = g_async_queue_new();
     signal(SIGTERM, signal_handler);
     signal(SIGINT, signal_handler);
     set_default_values_from_env();
@@ -212,7 +238,6 @@ int main(int argc, char *argv[])
         }
     }
     g_free(log_dir);
-    GIOChannel *in = NULL;
     if (fifo == NULL) {
         // We read from stdin
         in = g_io_channel_unix_new(fileno(stdin));
@@ -228,30 +253,34 @@ int main(int argc, char *argv[])
     GIOStatus in_status = G_IO_STATUS_NORMAL;
     GString *in_buffer = g_string_new(NULL);
     init_or_reinit_output_channel(log_file, use_locks);
-    init_every_second_signal();
+    g_thread_create(stop_thread, NULL, FALSE, NULL);
+    GThread* management = g_thread_create(management_thread, NULL, TRUE, NULL);
     while ((in_status != G_IO_STATUS_EOF) && (in_status != G_IO_STATUS_ERROR)) {
         in_status = g_io_channel_read_line_string(in, in_buffer, NULL, NULL);
         if (in_status == G_IO_STATUS_NORMAL) {
+            g_mutex_lock(mutex);
             while (TRUE) {
                 gboolean write_status = write_output_channel(in_buffer);
                 if (write_status == FALSE) {
                     g_warning("error during write on: %s", log_file);
-                    alarm(0);  // to avoid a potential deadlock with SIGALARM every_second() calls
                     init_or_reinit_output_channel(log_file, use_locks);
-                    alarm(1);
                     continue;
                 }
                 break;
             }
+            g_mutex_unlock(mutex);
         }
     }
-    alarm(0);  // to avoid a potential deadlock with SIGALARM every_second() calls
-    every_second(-1);
-    destroy_output_channel();
+    // if we are here, the "in" input channel is closed
+    signal(SIGINT, SIG_DFL);
+    signal(SIGTERM, SIG_DFL);
+    g_async_queue_push(queue, GINT_TO_POINTER(1));
+    g_thread_join(management);
     g_io_channel_shutdown(in, FALSE, NULL);
     g_io_channel_unref(in);
     g_string_free(in_buffer, TRUE);
     g_option_context_free(context);
     g_free(log_file);
+    g_mutex_free(mutex);
     return 0;
 }

src/out.c

@@ -6,7 +6,6 @@
 #include <stdlib.h>
 #include <unistd.h>
 #include <string.h>
-#include <signal.h>
 #include <errno.h>
 #include "util.h"
 #include "control.h"