[syslog-ng] Re: syslog-ng 2.0rc1 memory usage
Balazs Scheidler
bazsi at balabit.hu
Tue Aug 8 22:02:03 CEST 2006
On Mon, 2006-08-07 at 19:11 -0500, Martin, David M wrote:
> I ran valgrind with these options for 10 minutes:
>
> valgrind --tool=memcheck -v --leak-check=yes --show-reachable=yes
> --log-file=/tmp/debug.txt ./syslog-ng -dv
Attached my latest set of leak fixes, please check if it fixes your
problem. I've also committed it, so it should be available in tomorrow's
snapshot.
Thanks again for helping me to debug this problem.
--
Bazsi
-------------- next part --------------
* added files
{arch}/syslog-ng/syslog-ng--mainline/syslog-ng--mainline--2.0/devel at balabit.hu--other-1/patch-log/patch-73
* modified files
--- orig/ChangeLog
+++ mod/ChangeLog
@@ -2,6 +2,52 @@
# arch-tag: automatic-ChangeLog--devel at balabit.hu--other-1/syslog-ng--mainline--2.0
#
+2006-08-08 19:58:08 GMT Balazs Scheidler <bazsi at balabit.hu> patch-73
+
+ Summary:
+ various memory leak fixes
+ Revision:
+ syslog-ng--mainline--2.0--patch-73
+
+ * src/affile.c (affile_dw_deinit): check if self->writer is NULL,
+ (affile_dd_free): call log_writer_options_destroy as
+ LogWriterOptions might contain dynamically allocated memory,
+
+ * src/afprog.c (afprogram_dd_exit): removed log_pipe_unref, it is
+ performed by the children handling framework,
+ (afprogram_dd_init): use the new GDestroyNotify argument of children
+ manager,
+ (afprogram_dd_free): added log_writer_options_destroy call
+
+ * src/afsocket.c (afsocket_dd_free): added
+ log_writer_options_destroy call
+
+ * src/children.c (ChildEntry): added callback_data_destroy member,
+ (child_manager_child_entry_free): new function, frees a ChildEntry,
+ (child_manager_sigchild): removed g_free, it is done by the
+ GHashTable automatically,
+ (child_manager_init): use g_hash_table_new_full and specify destroy
+ notify callback for the data stored in the hashtable
+
+ * src/logreader.c (log_reader_iterate_buf): assume that saddr is a
+ borrowed reference and handle it as such,
+ (log_reader_fetch_log): iterate_buf borrows the reference so it is
+ our duty to free sockaddr, do so in all branches,
+
+ * src/logwriter.c (log_writer_deinit): drop reference to
+ self->control to break a circular reference,
+ (log_writer_free): removed log_pipe_unref self->control,
+
+ * src/messages.c (msg_send_internal_message): check if
+ internal_msg_queue is null,
+ (msg_deinit): free internal_msg_queue
+
+ modified files:
+ ChangeLog src/affile.c src/afprog.c src/afsocket.c
+ src/children.c src/children.h src/logreader.c src/logwriter.c
+ src/logwriter.h src/messages.c
+
+
2006-08-02 07:22:17 GMT Balazs Scheidler <bazsi at balabit.hu> patch-72
Summary:
--- orig/src/affile.c
+++ mod/src/affile.c
@@ -240,8 +240,12 @@ static gboolean
affile_dw_deinit(LogPipe *s, GlobalConfig *cfg, PersistentConfig *persist)
{
AFFileDestWriter *self = (AFFileDestWriter *) s;
- log_pipe_deinit(self->writer, NULL, NULL);
- log_pipe_unref(self->writer);
+
+ if (self->writer)
+ {
+ log_pipe_deinit(self->writer, NULL, NULL);
+ log_pipe_unref(self->writer);
+ }
self->writer = NULL;
return TRUE;
}
@@ -573,7 +577,7 @@ affile_dd_free(LogPipe *s)
log_pipe_unref(self->writer);
if (self->writer_hash)
g_hash_table_destroy(self->writer_hash);
-
+ log_writer_options_destroy(&self->writer_options);
log_drv_free_instance(&self->super);
g_free(self);
}
--- orig/src/afprog.c
+++ mod/src/afprog.c
@@ -61,8 +61,6 @@ afprogram_dd_exit(pid_t pid, int status,
log_pipe_deinit(&self->super.super, NULL, NULL);
log_pipe_init(&self->super.super, NULL, NULL);
}
- /* drop reference, init rereferences us as it registers the callback */
- log_pipe_unref(&self->super.super);
}
static gboolean
@@ -115,8 +113,7 @@ afprogram_dd_init(LogPipe *s, GlobalConf
{
/* parent */
- child_manager_register(self->pid, afprogram_dd_exit, self);
- log_pipe_ref(s); /* child manager holds a reference */
+ child_manager_register(self->pid, afprogram_dd_exit, log_pipe_ref(&self->super.super), (GDestroyNotify) log_pipe_unref);
g_fd_set_cloexec(msg_pipe[1], TRUE);
close(msg_pipe[0]);
@@ -154,6 +151,7 @@ afprogram_dd_free(LogPipe *s)
log_pipe_unref(self->writer);
g_string_free(self->cmdline, TRUE);
+ log_writer_options_destroy(&self->writer_options);
log_drv_free_instance(&self->super);
g_free(self);
}
--- orig/src/afsocket.c
+++ mod/src/afsocket.c
@@ -856,6 +856,7 @@ afsocket_dd_free(LogPipe *s)
g_sockaddr_unref(self->dest_addr);
log_drv_free_instance(&self->super);
log_pipe_unref(self->writer);
+ log_writer_options_destroy(&self->writer_options);
g_free(s);
}
--- orig/src/children.c
+++ mod/src/children.c
@@ -27,19 +27,28 @@ typedef struct _ChildEntry
{
pid_t pid;
gpointer callback_data;
+ GDestroyNotify callback_data_destroy;
void (*exit_callback)(pid_t pid, int status, gpointer user_data);
} ChildEntry;
GHashTable *child_hash;
+static void
+child_manager_child_entry_free(ChildEntry *ce)
+{
+ ce->callback_data_destroy(ce->callback_data);
+ g_free(ce);
+}
+
void
-child_manager_register(pid_t pid, void (*callback)(pid_t, int, gpointer), gpointer user_data)
+child_manager_register(pid_t pid, void (*callback)(pid_t, int, gpointer), gpointer user_data, GDestroyNotify callback_data_destroy)
{
ChildEntry *ce = g_new0(ChildEntry, 1);
ce->pid = pid;
ce->exit_callback = callback;
ce->callback_data = user_data;
+ ce->callback_data_destroy = callback_data_destroy;
g_hash_table_insert(child_hash, &ce->pid, ce);
}
@@ -54,14 +63,13 @@ child_manager_sigchild(pid_t pid, int st
{
ce->exit_callback(pid, status, ce->callback_data);
g_hash_table_remove(child_hash, &pid);
- g_free(ce);
}
}
void
child_manager_init(void)
{
- child_hash = g_hash_table_new(g_int_hash, g_int_equal);
+ child_hash = g_hash_table_new_full(g_int_hash, g_int_equal, NULL, (GDestroyNotify) child_manager_child_entry_free);
}
void
--- orig/src/children.h
+++ mod/src/children.h
@@ -27,7 +27,7 @@
#include "syslog-ng.h"
#include <sys/types.h>
-void child_manager_register(pid_t pid, void (*callback)(pid_t, int, gpointer), gpointer user_data);
+void child_manager_register(pid_t pid, void (*callback)(pid_t, int, gpointer), gpointer user_data, GDestroyNotify user_data_destroy);
void child_manager_sigchild(pid_t pid, int status);
void child_manager_init(void);
--- orig/src/logreader.c
+++ mod/src/logreader.c
@@ -287,9 +287,7 @@ log_reader_iterate_buf(LogReader *self,
self->ofs = &self->buffer[self->ofs] - start;
memmove(self->buffer, start, self->ofs);
if (self->ofs != 0)
- self->prev_addr = saddr;
- else
- g_sockaddr_unref(saddr);
+ self->prev_addr = g_sockaddr_ref(saddr);
}
return TRUE;
@@ -337,10 +335,12 @@ log_reader_fetch_log(LogReader *self, FD
while (msg_count < self->options->fetch_limit)
{
avail = self->options->msg_size - self->ofs;
+ sa = NULL;
rc = fd_read(fd, self->buffer + self->ofs, avail, &sa);
if (rc == -1)
{
+ g_sockaddr_unref(sa);
if (errno == EAGAIN)
{
/* ok we don't have any more data to read, return to main poll loop */
@@ -355,7 +355,6 @@ log_reader_fetch_log(LogReader *self, FD
NULL);
log_reader_iterate_buf(self, NULL, TRUE, &msg_count);
log_pipe_notify(self->control, &self->super.super, NC_READ_ERROR, self);
- g_sockaddr_unref(sa);
return FALSE;
}
}
@@ -386,7 +385,8 @@ log_reader_fetch_log(LogReader *self, FD
self->ofs += rc;
log_reader_iterate_buf(self, sa, FALSE, &msg_count);
}
-
+
+ g_sockaddr_unref(sa);
if (self->flags & LR_NOMREAD)
break;
}
--- orig/src/logwriter.c
+++ mod/src/logwriter.c
@@ -389,7 +389,9 @@ log_writer_deinit(LogPipe *s, GlobalConf
g_source_unref(self->source);
self->source = NULL;
}
-
+ log_pipe_unref(self->control);
+ self->control = NULL;
+
return TRUE;
}
@@ -402,7 +404,6 @@ log_writer_free(LogPipe *s)
stats_unregister_counter(SC_TYPE_DROPPED, self->options->stats_name, &self->dropped_messages);
g_queue_free(self->queue);
- log_pipe_unref(self->control);
g_free(self);
}
--- orig/src/logwriter.h
+++ mod/src/logwriter.h
@@ -86,5 +86,6 @@ gboolean log_writer_reopen(LogPipe *s, F
void log_writer_options_set_template_escape(LogWriterOptions *options, gboolean enable);
void log_writer_options_defaults(LogWriterOptions *options);
void log_writer_options_init(LogWriterOptions *options, GlobalConfig *cfg, guint32 flags, const gchar *stats_name);
+void log_writer_options_destroy(LogWriterOptions *options);
#endif
--- orig/src/messages.c
+++ mod/src/messages.c
@@ -55,7 +55,8 @@ msg_send_internal_message(int prio, cons
g_snprintf(buf, sizeof(buf), "<%d> syslog-ng[%d]: %s\n", prio, getpid(), msg);
m = log_msg_new(buf, strlen(buf), NULL, LP_INTERNAL | LP_LOCAL, NULL);
- g_queue_push_tail(internal_msg_queue, m);
+ if (G_LIKELY(internal_msg_queue))
+ g_queue_push_tail(internal_msg_queue, m);
}
}
@@ -132,4 +133,6 @@ void
msg_deinit()
{
evt_ctx_free(evt_context);
+ g_queue_free(internal_msg_queue);
+ internal_msg_queue = NULL;
}
More information about the syslog-ng
mailing list