[syslog-ng] Memory leak when handling SIGHUP

anthony lineham anthony.lineham at alliedtelesis.co.nz
Thu Jul 12 23:51:58 CEST 2007


Hi Bazsi,

I've finally found the memory leak I noticed when syslog-ng handles a
SIGHUP. It turned out that
there were 3 leaks. They are not very big so perhaps not significant
for most users but in my system
there is limited memory and frequent use of SIGHUP.

I've attached a patch that I believe resolves the leaks. Valgrind seems
to like it. 
However please let me know if there are any problems with it. This is
the first time I've really gotten into
the code so I'm sure I may have overlooked something.

Please note that my original fix was made and tested on 2.0.0+20061212,
but the attached patch is for 2.0.4.
I didn't notice any differences that would seem to impact the fix, but
I may have missed something.

Here's a description of the changes:
cfg-grammar.y
cfg-grammar.c
- The template reference counter gets incremented in the template
lookup
function, so remove the second increment here. Otherwise the counter
never gets back to 0 and the template won't be able to be deleted.

affile.c
affile_dd_deinit:
+ Unreference the writer after de-initing it. The unreference will
cause
the writer to be deleted, which causes it to unreference this
destination
driver (a circular reference). When the call to affile_dd_free is made
via cfg_free() the ref counter will be 1, allowing a final decrement
and hence the removal of the destination diver. Previously the counter
was
2 due to the writer reference, which prevented the destination driver
from
being removed.

templates.c
+ When freeing a template also deallocate the memory used by a
compiled
template.

regards
Anthony
 
>>> "anthony lineham" <anthony.lineham at alliedtelesis.co.nz> 07/09/07
8:56 AM >>> 

 
>>> Balazs Scheidler <bazsi at balabit.hu> 07/06/07 11:54 PM >>> 
On Fri, 2007-  07-  06 at 17:18 +1200, anthony lineham wrote:
>> I've noticed a small memory leak  when syslog-  ng handles a
SIGHUP.
>> 
>> A bit of valgrind-  ing gave me the following result for syslog- 
ng
>> without any SIGHUPs
>> 1,004 (652 direct, 352 indirect) bytes in 7 blocks are definitely
lost
>> in loss record 9 of 16
>> ==11308==    at 0x402C9C0: calloc (in
>> /opt/4/lib/valgrind/ppc32-  linux/vgpreload_memcheck.so)
>> ==11308==    by 0x40B9CF4: g_malloc0 (in
>> /usr/lib/libglib-  2.0.so.0.1101.0)
>> ==11308==    by 0x1000C73C: affile_dd_new (affile.c:607)
>> ==11308==    by 0x10007A80: yyparse (cfg-  grammar.y:478)
>> ==11308==    by 0x1000528C: cfg_new (cfg.c:284)
>> ==11308==    by 0x100042D0: main (main.c:424)
>> ==11308==
>> 
>> By inspection I see that the call tree from cfg_new (cfg.c:284)
upwards
>> applies when a SIGHUP is handled so I'm guessing this may be the
cause.
>> I don't know how to send a SIGHUP to an app that is running under
>> valgrind, so I haven't confirmed this.
>> 
>> The interesting thing is that the size of the leak I'm seeing is a
lot
>> smaller (4-  8bytes) than the amount I would expect if the chunk
allocated
>> in affile_dd_new is getting lost.
>> 
>> I'm running a snapshot from 2.0.0 -   2.0.0+20061212.
>> 
>> I check the release note for 2.0.4 but didn't see any mention of
memory
>> leak fixes.
>> 

>You only seek leaks when SIGHUP is sent, or also during normal
>operation?

I'm only seeing the leak when SIGHUP is sent. I haven't noticed
anything associated with normal operation.

_______________________________________________
syslog- ng maillist  -   syslog- ng at lists.balabit.hu
https://lists.balabit.hu/mailman/listinfo/syslog- ng
Frequently asked questions at http://www.campin.net/syslog-
ng/faq.html


-------------- next part --------------
diff -Nruw -x '*~' -x '\.*' syslog-ng-2.0.4/src/affile.c syslog-ng-2.0.4-mod/src/affile.c
--- syslog-ng-2.0.4/src/affile.c	2007-04-20 07:37:16.000000000 +1200
+++ syslog-ng-2.0.4-mod/src/affile.c	2007-07-13 09:18:38.000000000 +1200
@@ -597,7 +597,11 @@
   AFFileDestDriver *self = (AFFileDestDriver *) s;
 
   if (self->writer)
+  {
     log_pipe_deinit(self->writer, cfg, NULL);
+    log_pipe_unref(self->writer);
+    self->writer = NULL;
+  }
   if (self->writer_hash)
     g_hash_table_foreach_remove(self->writer_hash, affile_dd_remove_writers, self);
   if (self->reap_timer)
diff -Nruw -x '*~' -x '\.*' syslog-ng-2.0.4/src/cfg-grammar.c syslog-ng-2.0.4-mod/src/cfg-grammar.c
--- syslog-ng-2.0.4/src/cfg-grammar.c	2007-05-16 02:41:15.000000000 +1200
+++ syslog-ng-2.0.4-mod/src/cfg-grammar.c	2007-07-13 09:23:08.000000000 +1200
@@ -3035,8 +3035,6 @@
 	                                              last_writer_options->template = log_template_new(NULL, (yyvsp[(3) - (4)].cptr)); 
 	                                              last_writer_options->template->def_inline = TRUE;
 	                                            }
-	                                          else
-	                                            log_template_ref(last_writer_options->template);
 	                                          free((yyvsp[(3) - (4)].cptr));
 	                                        }
     break;
diff -Nruw -x '*~' -x '\.*' syslog-ng-2.0.4/src/cfg-grammar.y syslog-ng-2.0.4-mod/src/cfg-grammar.y
--- syslog-ng-2.0.4/src/cfg-grammar.y	2007-04-21 08:24:08.000000000 +1200
+++ syslog-ng-2.0.4-mod/src/cfg-grammar.y	2007-07-13 09:23:24.000000000 +1200
@@ -677,8 +677,6 @@
 	                                              last_writer_options->template = log_template_new(NULL, $3); 
 	                                              last_writer_options->template->def_inline = TRUE;
 	                                            }
-	                                          else
-	                                            log_template_ref(last_writer_options->template);
 	                                          free($3);
 	                                        }
 	| KW_TEMPLATE_ESCAPE '(' yesno ')'	{ log_writer_options_set_template_escape(last_writer_options, $3); }
diff -Nruw -x '*~' -x '\.*' syslog-ng-2.0.4/src/templates.c syslog-ng-2.0.4-mod/src/templates.c
--- syslog-ng-2.0.4/src/templates.c	2007-04-20 07:37:16.000000000 +1200
+++ syslog-ng-2.0.4-mod/src/templates.c	2007-07-13 09:22:02.000000000 +1200
@@ -123,6 +123,25 @@
 }
 
 void
+log_compiled_template_free(LogTemplate *self)
+{
+  GList* p;
+  LogTemplateElem *e;
+  
+  if (self->compiled_template)
+  {
+    for (p = self->compiled_template; p; p = g_list_first(p))
+    {
+      e = p->data;
+      p = g_list_remove(p, e);
+      g_string_free(e->text, TRUE);
+      g_free(e);
+    }
+    self->compiled_template = NULL;
+  }
+}
+
+void
 log_template_format(LogTemplate *self, LogMessage *lm, guint macro_flags, gint ts_format, glong zone_offset, gint frac_digits, GString *result)
 {
   GList *p;
@@ -167,6 +186,7 @@
 static void 
 log_template_free(LogTemplate *self)
 {
+  log_compiled_template_free(self);
   g_string_free(self->name, TRUE);
   g_string_free(self->template, TRUE);
   g_free(self);


More information about the syslog-ng mailing list