moved log cleanups to own function; fixed memleaks in config parser and plugin_core

src/config_parser.h

@@ -10,7 +10,7 @@ typedef struct config_parser_context_t config_parser_context_t;
 
 /* returns a new config parser stack with the first context in it */
 GList *config_parser_init(server *srv);
-void config_parser_finish(server *srv, GList *ctx_stack);
+void config_parser_finish(server *srv, GList *ctx_stack, gboolean free_all);
 
 /* loads a file into memory and parses it */
 gboolean config_parser_file(server *srv, GList *ctx_stack, const gchar *path);

src/config_parser.rl

@@ -2,7 +2,7 @@
 #include "condition.h"
 #include "config_parser.h"
 
-#if 0
+#if 1
 	#define _printf(fmt, ...) g_print(fmt, __VA_ARGS__)
 #else
 	#define _printf(fmt, ...) /* */
@@ -80,7 +80,7 @@
 
 		g_string_free(str, TRUE);
 
-		_printf("got int with suffix: %d\n", o->data.number);
+		_printf("got int with suffix: %" G_GINT64_FORMAT "\n", o->data.number);
 
 		/* make sure there was no overflow that led to negative numbers */
 		if (o->data.number < 0) {
@@ -306,7 +306,7 @@
 
 			if (r->type == VALUE_STRING && ctx->value_op == '+') {
 				/* str + str */
-				o->data.string = g_string_append_len(o->data.string, r->data.string->str, r->data.string->len);
+				o->data.string = g_string_append_len(o->data.string, GSTR_LEN(r->data.string));
 			}
 			else if (r->type == VALUE_NUMBER && ctx->value_op == '+') {
 				/* str + int */
@@ -343,21 +343,11 @@
 			}
 			else if (ctx->value_op == '*') {
 				/* merge l and r */
-				//GArray *a;
-				//free_l = free_r = FALSE;
-				//o = l;
-
-
-				//a = g_array_sized_new(FALSE, FALSE, sizeof(value*), r->list->len);
-				//a = g_array_append_vals(a, r->list->data, r->list->len); /* copy old list from r to a */
-				//o->list = g_array_append_vals(o->list, a->data, a->len); /* append data from a to o */
-				//g_array_free(a, FALSE); /* free a but not the data because it is now in o */
-
 				if (r->type == VALUE_LIST) {
 					/* merge lists */
 					free_l = FALSE;
 					g_array_append_vals(l->data.list, r->data.list->data, r->data.list->len);
-					r->type = VALUE_NONE;
+					g_array_set_size(r->data.list, 0);
 					o = l;
 				}
 			}
@@ -415,6 +405,8 @@
 
 		o = g_queue_pop_head(ctx->option_stack);
 
+		_printf("got actionref: %s in line %zd\n", o->data.string->str, ctx->line);
+
 		/* action refs starting with "var." are user defined variables */
 		if (g_str_has_prefix(o->data.string->str, "var.")) {
 			/* look up var in hashtable, copy and push value onto stack */
@@ -486,15 +478,27 @@
 
 		assert(name->type == VALUE_STRING);
 
-		_printf("got assignment: %s = %s; in line %zd\n", name->string->str, value_type_string(val->type), ctx->line);
+		_printf("got assignment: %s = %s; in line %zd\n", name->data.string->str, value_type_string(val->type), ctx->line);
 
 		if (ctx->in_setup_block) {
 			/* in setup { } block => set default values for options */
-			/* todo name */
+			/* TODO */
 		}
 		else if (g_str_has_prefix(name->data.string->str, "var.")) {
 			/* assignment vor user defined variable, insert into hashtable */
-			g_hash_table_insert(ctx->uservars, name->data.string, val);
+			gpointer old_key;
+			gpointer old_val;
+			GString *str = value_extract(name).string;
+
+			/* free old key and value if we are overwriting it */
+			if (g_hash_table_lookup_extended(ctx->uservars, str, &old_key, &old_val)) {
+				g_hash_table_remove(ctx->uservars, str);
+				g_string_free(old_key, TRUE);
+				value_free(old_val);
+			}
+
+			g_hash_table_insert(ctx->uservars, str, val);
+			val = NULL; /* prevent free */
 		}
 		else {
 			/* normal assignment */
@@ -505,8 +509,10 @@
 
 			al = g_queue_peek_head(ctx->action_list_stack);
 			g_array_append_val(al->data.list, a);
-			value_free(name);
 		}
+
+		value_free(name);
+		value_free(val);
 	}
 
 	action function_noparam {
@@ -517,7 +523,7 @@
 
 		assert(name->type == VALUE_STRING);
 
-		_printf("got function: %s; in line %zd\n", name->string->str, ctx->line);
+		_printf("got function: %s; in line %zd\n", name->data.string->str, ctx->line);
 
 		if (g_str_equal(name->data.string->str, "break")) {
 		}
@@ -539,6 +545,8 @@
 
 				g_array_append_val(al->data.list, a);
 			}
+
+			value_free(name);
 		}
 	}
 
@@ -553,7 +561,7 @@
 
 		assert(name->type == VALUE_STRING);
 
-		_printf("got function: %s %s; in line %zd\n", name->string->str, value_type_string(val->type), ctx->line);
+		_printf("got function: %s %s; in line %zd\n", name->data.string->str, value_type_string(val->type), ctx->line);
 
 		if (g_str_equal(name->data.string->str, "include")) {
 			if (val->type != VALUE_STRING) {
@@ -583,6 +591,7 @@
 				GString *tmpstr = value_to_string(val);
 				g_printerr("%s:%zd type: %s, value: %s\n", ctx->filename, ctx->line, value_type_string(val->type), tmpstr->str);
 				g_string_free(tmpstr, TRUE);
+				value_free(val);
 			}
 		}
 		/* normal function action */
@@ -624,7 +633,7 @@
 
 		assert(n->type == VALUE_STRING);
 
-		_printf("got condition: %s:%s %s %s in line %zd\n", n->string->str, ctx->condition_with_key ? k->string->str : "", comp_op_to_string(ctx->op), value_type_string(v->type), ctx->line);
+		_printf("got condition: %s:%s %s %s in line %zd\n", n->data.string->str, ctx->condition_with_key ? k->data.string->str : "", comp_op_to_string(ctx->op), value_type_string(v->type), ctx->line);
 
 		/* create condition lvalue */
 		str = n->data.string->str;
@@ -655,7 +664,7 @@
 					log_warning(srv, NULL, "%s", "header conditional needs a key");
 					return FALSE;
 				}
-				lvalue = condition_lvalue_new(COMP_REQUEST_HEADER, k->data.string);
+				lvalue = condition_lvalue_new(COMP_REQUEST_HEADER, value_extract(k).string);
 			}
 			else {
 				log_warning(srv, NULL, "unkown lvalue for condition: %s", n->data.string->str);
@@ -690,10 +699,10 @@
 		}
 
 		if (v->type == VALUE_STRING) {
-			cond = condition_new_string(srv, ctx->op, lvalue, v->data.string);
+			cond = condition_new_string(srv, ctx->op, lvalue, value_extract(v).string);
 		}
 		else if (v->type == VALUE_NUMBER)
-			cond = condition_new_int(srv, ctx->op, lvalue, (gint64) v->data.number);
+			cond = condition_new_int(srv, ctx->op, lvalue, value_extract_number(v));
 		else {
 			cond = NULL;
 		}
@@ -709,6 +718,9 @@
 		g_queue_push_head(ctx->action_list_stack, action_new_list());
 
 		/* TODO: free stuff */
+		value_free(n);
+		value_free(k);
+		value_free(v);
 		ctx->condition_with_key = FALSE;
 	}
 
@@ -806,7 +818,7 @@
 		else {
 			GString *str;
 
-			_printf("action block %s in line %zd\n", o->string->str, ctx->line);
+			_printf("action block %s in line %zd\n", o->data.string->str, ctx->line);
 
 			/* create new action list and put it on the stack */
 			al = action_new_list();
@@ -904,7 +916,7 @@ GList *config_parser_init(server* srv) {
 	return g_list_append(NULL, ctx);
 }
 
-void config_parser_finish(server *srv, GList *ctx_stack) {
+void config_parser_finish(server *srv, GList *ctx_stack, gboolean free_all) {
 	config_parser_context_t *ctx;
 	GHashTableIter iter;
 	gpointer key, val;
@@ -912,37 +924,41 @@ void config_parser_finish(server *srv, GList *ctx_stack) {
 	_printf("ctx_stack size: %u\n", g_list_length(ctx_stack));
 
 	/* clear all contexts from the stack */
-	while ((ctx = g_list_nth_data(ctx_stack, 1)) != NULL) {
+	GList *l = g_list_nth(ctx_stack, 1);
+	while (l) {
+		ctx = l->data;
 		config_parser_context_free(srv, ctx, FALSE);
+		l = l->next;
 	}
 
-	ctx = (config_parser_context_t*) ctx_stack->data;
-
-	g_hash_table_iter_init(&iter, ctx->action_blocks);
+	if (free_all) {
+		ctx = (config_parser_context_t*) ctx_stack->data;
 
-	while (g_hash_table_iter_next(&iter, &key, &val)) {
-		g_hash_table_iter_steal(&iter);
-		g_string_free(key, TRUE);
-	}
-
-	g_hash_table_destroy(ctx->action_blocks);
+		g_hash_table_iter_init(&iter, ctx->action_blocks);
 
+		while (g_hash_table_iter_next(&iter, &key, &val)) {
+			action_release(srv, val);
+			g_string_free(key, TRUE);
+		}
 
+		g_hash_table_destroy(ctx->action_blocks);
 
-	g_hash_table_iter_init(&iter, ctx->uservars);
+		g_hash_table_iter_init(&iter, ctx->uservars);
 
-	while (g_hash_table_iter_next(&iter, &key, &val)) {
-		g_hash_table_iter_steal(&iter);
-		g_string_free(key, TRUE);
-	}
+		while (g_hash_table_iter_next(&iter, &key, &val)) {
+			value_free(val);
+			g_string_free(key, TRUE);
+		}
 
-	g_hash_table_destroy(ctx->uservars);
+		g_hash_table_destroy(ctx->uservars);
 
 
 
-	config_parser_context_free(srv, ctx, TRUE);
+		config_parser_context_free(srv, ctx, TRUE);
 
-	g_list_free(ctx_stack);
+		g_list_free(ctx_stack);
+		srv->mainaction = NULL;
+	}
 }
 
 config_parser_context_t *config_parser_context_new(server *srv, GList *ctx_stack) {
@@ -1015,8 +1031,15 @@ void config_parser_context_free(server *srv, config_parser_context_t *ctx, gbool
 				value_free(o);
 		}
 
+		if (g_queue_get_length(ctx->condition_stack) > 0) {
+			condition *c;
+			while ((c = g_queue_pop_head(ctx->condition_stack)))
+				condition_release(srv, c);
+		}
+
 		g_queue_free(ctx->action_list_stack);
 		g_queue_free(ctx->option_stack);
+		g_queue_free(ctx->condition_stack);
 	}
 
 	g_slice_free(config_parser_context_t, ctx);

src/lighttpd.c

@@ -9,7 +9,7 @@
 void plugin_core_init(server *srv, plugin *p);
 
 int main(int argc, char *argv[]) {
-	GError *error;
+	GError *error = NULL;
 	GOptionContext *context;
 	server *srv;
 	gboolean res;
@@ -19,6 +19,8 @@ int main(int argc, char *argv[]) {
 	gboolean test_config = FALSE;
 	gboolean show_version = FALSE;
 
+	GList *ctx_stack = NULL;
+
 	GOptionEntry entries[] = {
 		{ "config", 'c', 0, G_OPTION_ARG_FILENAME, &config_path, "filename/path of the config", "PATH" },
 		{ "lua", 'l', 0, G_OPTION_ARG_NONE, &luaconfig, "use the lua config frontend", NULL },
@@ -38,6 +40,7 @@ int main(int argc, char *argv[]) {
 
 	if (!res) {
 		g_printerr("failed to parse command line arguments: %s\n", error->message);
+		g_error_free(error);
 		return 1;
 	}
 
@@ -68,10 +71,10 @@ int main(int argc, char *argv[]) {
 		guint64 d;
 
 		/* standard config frontend */
-		GList *ctx_stack = config_parser_init(srv);
+		ctx_stack = config_parser_init(srv);
 		config_parser_context_t *ctx = (config_parser_context_t*) ctx_stack->data;
 		if (!config_parser_file(srv, ctx_stack, config_path)) {
-			config_parser_finish(srv, ctx_stack);
+			config_parser_finish(srv, ctx_stack, TRUE);
 			log_thread_start(srv);
 			g_atomic_int_set(&srv->exiting, TRUE);
 			log_thread_wakeup(srv);
@@ -90,6 +93,7 @@ int main(int argc, char *argv[]) {
 		log_debug(srv, NULL, "option_stack: %u action_list_stack: %u (should be 0:1)", g_queue_get_length(ctx->option_stack), g_queue_get_length(ctx->action_list_stack));
 
 		/* TODO config_parser_finish(srv, ctx_stack); */
+		config_parser_finish(srv, ctx_stack, FALSE);
 	}
 	else {
 #ifdef HAVE_LUA_H
@@ -110,6 +114,7 @@ int main(int argc, char *argv[]) {
 	server_loop_init(srv);
 	server_start(srv);
 
+	config_parser_finish(srv, ctx_stack, TRUE);
 	server_free(srv);
 
 	return 0;

src/log.c

@@ -51,11 +51,11 @@ gboolean log_write_(server *srv, connection *con, log_level_t log_level, guint f
 			return TRUE;
 		ts = CORE_OPTION(CORE_OPTION_LOG_TS_FORMAT).ptr;
 		if (!ts)
-			ts = &g_array_index(srv->logs.timestamps, log_timestamp_t, 0);
+			ts = g_array_index(srv->logs.timestamps, log_timestamp_t*, 0);
 	}
 	else {
 		log = srv->logs.stderr;
-		ts = &g_array_index(srv->logs.timestamps, log_timestamp_t, 0);
+		ts = g_array_index(srv->logs.timestamps, log_timestamp_t*, 0);
 	}
 
 	log_ref(srv, log);
@@ -133,9 +133,8 @@ gboolean log_write_(server *srv, connection *con, log_level_t log_level, guint f
 
 	/* on critical error, exit */
 	if (log_level == LOG_LEVEL_ABORT) {
+		log_thread_stop(srv);
 		g_atomic_int_set(&srv->exiting, TRUE);
-		g_atomic_int_set(&srv->logs.stop_thread, TRUE);
-		log_thread_wakeup(srv); /* just in case the logging thread is sleeping at this point */
 	}
 
 	return TRUE;
@@ -163,19 +162,17 @@ gpointer log_thread(server *srv) {
 		}
 		*/
 
+		if (g_atomic_int_get(&srv->logs.thread_stop) == TRUE)
+			break;
+
+		if (g_atomic_int_get(&srv->logs.thread_finish) == TRUE && g_async_queue_length(srv->logs.queue) == 0)
+			break;
+
 		log_entry = g_async_queue_pop(srv->logs.queue);
 
 		/* if log_entry->log is NULL, it means that the logger thread has been woken up probably because it should exit */
 		if (log_entry->log == NULL) {
 			g_slice_free(log_entry_t, log_entry);
-
-			/* lighty is exiting, end logging thread */
-			if (g_atomic_int_get(&srv->state) == SERVER_STOPPING && g_async_queue_length(srv->logs.queue) == 0)
-				break;
-
-			if (g_atomic_int_get(&srv->logs.stop_thread) == TRUE)
-				break;
-
 			continue;
 		}
 
@@ -378,7 +375,8 @@ void log_init(server *srv) {
 	srv->logs.targets = g_hash_table_new_full(g_str_hash, g_str_equal, NULL, NULL);
 	srv->logs.queue = g_async_queue_new();
 	srv->logs.mutex = g_mutex_new();
-	srv->logs.timestamps = g_array_new(FALSE, FALSE, sizeof(log_timestamp_t));
+	srv->logs.timestamps = g_array_new(FALSE, FALSE, sizeof(log_timestamp_t*));
+	srv->logs.thread_alive = FALSE;
 
 	/* first entry in srv->logs.timestamps is the default timestamp */
 	log_timestamp_new(srv, g_string_new_len(CONST_STR_LEN("%d/%b/%Y %T %Z")));
@@ -388,10 +386,45 @@ void log_init(server *srv) {
 	srv->logs.stderr = log_new(srv, LOG_TYPE_STDERR, str);
 }
 
+void log_cleanup(server *srv) {
+	guint i;
+	/* wait for logging thread to exit */
+	if (g_atomic_int_get(&srv->logs.thread_alive) == TRUE)
+	{
+		log_thread_finish(srv);
+		g_thread_join(srv->logs.thread);
+	}
+
+
+	log_free(srv, srv->logs.stderr);
+
+	g_hash_table_destroy(srv->logs.targets);
+	g_mutex_free(srv->logs.mutex);
+	g_async_queue_unref(srv->logs.queue);
+
+	log_timestamp_t *ts;
+	for (i = 0; i < srv->logs.timestamps->len; i++) {
+		ts = g_array_index(srv->logs.timestamps, log_timestamp_t*, i);
+		g_print("ts #%d refcount: %d\n", i, ts->refcount);
+		/*if (g_atomic_int_dec_and_test(&ts->refcount)) {
+			g_string_free(ts->cached, TRUE);
+			g_string_free(ts->format, TRUE);
+			g_slice_free(log_timestamp_t, ts);
+			g_array_remove_index_fast(srv->logs.timestamps, i);
+			i--;
+		}*/
+	}
+
+	log_timestamp_free(srv, g_array_index(srv->logs.timestamps, log_timestamp_t*, 0));
+
+	g_array_free(srv->logs.timestamps, TRUE);
+}
+
 void log_thread_start(server *srv) {
 	GError *err = NULL;
 
 	srv->logs.thread = g_thread_create((GThreadFunc)log_thread, srv, TRUE, &err);
+	g_atomic_int_set(&srv->logs.thread_alive, TRUE);
 
 	if (srv->logs.thread == NULL) {
 		g_printerr("could not create loggin thread: %s\n", err->message);
@@ -400,6 +433,20 @@ void log_thread_start(server *srv) {
 	}
 }
 
+void log_thread_stop(server *srv) {
+	if (g_atomic_int_get(&srv->logs.thread_alive) == TRUE) {
+		g_atomic_int_set(&srv->logs.thread_stop, TRUE);
+		log_thread_wakeup(srv);
+	}
+}
+
+void log_thread_finish(server *srv) {
+	if (g_atomic_int_get(&srv->logs.thread_alive) == TRUE) {
+		g_atomic_int_set(&srv->logs.thread_finish, TRUE);
+		log_thread_wakeup(srv);
+	}
+}
+
 void log_thread_wakeup(server *srv) {
 	log_entry_t *e;
 
@@ -422,9 +469,10 @@ log_timestamp_t *log_timestamp_new(server *srv, GString *format) {
 
 	/* check if there already exists a timestamp entry with the same format */
 	for (guint i = 0; i < srv->logs.timestamps->len; i++) {
-		ts = &g_array_index(srv->logs.timestamps, log_timestamp_t, i);
+		ts = g_array_index(srv->logs.timestamps, log_timestamp_t*, i);
 		if (g_string_equal(ts->format, format)) {
 			g_atomic_int_inc(&(ts->refcount));
+			g_string_free(format, TRUE);
 			return ts;
 		}
 	}
@@ -436,19 +484,24 @@ log_timestamp_t *log_timestamp_new(server *srv, GString *format) {
 	ts->refcount = 1;
 	ts->format = format;
 
-	g_array_append_val(srv->logs.timestamps, *ts);
+	g_array_append_val(srv->logs.timestamps, ts);
 
 	return ts;
 }
 
-void log_timestamp_free(server *srv, log_timestamp_t *ts) {
+gboolean log_timestamp_free(server *srv, log_timestamp_t *ts) {
 	if (g_atomic_int_dec_and_test(&(ts->refcount))) {
 		for (guint i = 0; i < srv->logs.timestamps->len; i++) {
-			if (g_string_equal(g_array_index(srv->logs.timestamps, log_timestamp_t, i).format, ts->format)) {
+			if (g_string_equal(g_array_index(srv->logs.timestamps, log_timestamp_t*, i)->format, ts->format)) {
 				g_array_remove_index_fast(srv->logs.timestamps, i);
 				break;
 			}
 		}
+		g_string_free(ts->cached, TRUE);
+		g_string_free(ts->format, TRUE);
 		g_slice_free(log_timestamp_t, ts);
+		return TRUE;
 	}
+
+	return FALSE;
 }

src/log.h

@@ -170,13 +170,16 @@ void log_rotate_logs(server *srv);
 
 gpointer log_thread(server *srv);
 void log_thread_start(server *srv);
+void log_thread_stop(server *srv);
+void log_thread_finish(server *srv);
 void log_thread_wakeup(server *srv);
 
 void log_init(server *srv);
+void log_cleanup(server *srv);
 
 LI_API gboolean log_write_(server *srv, connection *con, log_level_t log_level, guint flags, const gchar *fmt, ...) __ATTRIBUTE_PRINTF_FORMAT(5, 6);
 
 log_timestamp_t *log_timestamp_new(server *srv, GString *format);
-void log_timestamp_free(server *srv, log_timestamp_t *ts);
+gboolean log_timestamp_free(server *srv, log_timestamp_t *ts);
 
 #endif

src/plugin_core.c

@@ -453,6 +453,8 @@ static gboolean core_option_log_parse(server *srv, plugin *p, size_t ndx, value
 		}
 	}
 
+	value_free(val);
+
 	return TRUE;
 }
 
@@ -475,7 +477,7 @@ static gboolean core_option_log_timestamp_parse(server *srv, plugin *p, size_t n
 	UNUSED(ndx);
 
 	if (!val) return TRUE;
-	oval->ptr = log_timestamp_new(srv, val->data.string);
+	oval->ptr = log_timestamp_new(srv, value_extract(val).string);
 
 	return TRUE;
 }

src/server.c

@@ -127,24 +127,11 @@ void server_free(server* srv) {
 		g_slice_free1(srv->option_count * sizeof(*srv->option_def_values), srv->option_def_values);
 	}
 
+	log_cleanup(srv);
+
 	server_plugins_free(srv);
 	g_array_free(srv->plugins_handle_close, TRUE); /* TODO: */
 
-	/* free logs */
-	g_thread_join(srv->logs.thread);
-	{
-		GHashTableIter iter;
-		gpointer k, v;
-		g_hash_table_iter_init(&iter, srv->logs.targets);
-		while (g_hash_table_iter_next(&iter, &k, &v)) {
-			log_free(srv, v);
-		}
-		g_hash_table_destroy(srv->logs.targets);
-	}
-
-	g_mutex_free(srv->logs.mutex);
-	g_async_queue_unref(srv->logs.queue);
-
 	g_slice_free(server, srv);
 }
 

src/server.h

@@ -58,11 +58,13 @@ struct server {
 
 	struct {
 		GMutex *mutex;
-		GHashTable *targets;  /** const gchar* path => (log_t*) */
+		GHashTable *targets;    /** const gchar* path => (log_t*) */
 		GAsyncQueue *queue;
 		GThread *thread;
-		gboolean stop_thread; /** access with atomic functions */
-		GArray *timestamps;   /** array of log_timestamp_t */
+		gboolean thread_finish; /** finish writing logs in the queue, then exit thread; access with atomic functions */
+		gboolean thread_stop;   /** stop thread immediately; access with atomic functions */
+		gboolean thread_alive;  /** access with atomic functions */
+		GArray *timestamps;     /** array of log_timestamp_t */
 		struct log_t *stderr;
 	} logs;