Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Aug 16, 2021
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8731085
fix: potential fix for malloc deadlock in some corner cases
thefab Aug 6, 2021
bfdee3f
build: fix every_second signature
thefab Aug 6, 2021
6a93596
build: better shutdown process
thefab Aug 6, 2021
208cb39
Update log_proxy.c
thefab Aug 6, 2021
9b97aea
Update log_proxy.c
thefab Aug 6, 2021
e3fa3e3
Update log_proxy.c
thefab Aug 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
101 changes: 65 additions & 36 deletions src/log_proxy.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand All @@ -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;
Expand All @@ -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) {
Expand All @@ -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;
Expand All @@ -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();
Expand All @@ -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));
Expand All @@ -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;
}
1 change: 0 additions & 1 deletion src/out.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down