Bug#579427: tinyproxy patch for reload memleak

[Mark Kamichoff]

Hi John - 

> Valgrind reports that the regcomp functions used in the conf.c file
> leak.  This is because regfree is never called.
> 
> I fixed this by adding a cleaning function, and also running the
> regcomp loop just once. It was done every time the config was loaded,
> and since it did not free, it would leak about 2.5mb per fork
> everytime a reload was issued.

Ah, that's why it was growing each day.  It was logrotate that was
sending the SIGHUP (reload) after rotating logfiles.

Thanks for figuring this out and writing the patch!  I've updated the
bug report at Banu, too:

https://www.banu.com/bugzilla/show_bug.cgi?id=89#c4

- Mark

-- 
Mark Kamichoff
p...@prolixium.com
http://www.prolixium.com/



-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Bug#579427: tinyproxy patch for reload memleak

[John van der Kamp]

Hi,

Valgrind reports that the regcomp functions used in the conf.c file leak. 
This is because regfree is never called.


I fixed this by adding a cleaning function, and also running the regcomp 
loop just once. It was done every time the config was loaded, and since it 
did not free, it would leak about 2.5mb per fork everytime a reload was 
issued.


Valgrind still reports the leak on exit. Not sure how that happends, since 
I added an atexit() to cleanup the regcomp data, but maybe that gets reset 
somewhere. I havn't put that much effort in it anymore, since it now 
doesn't leak anymore in normal production mode.


Patch is attached.

John--- tinyproxy-orig/src/conf.c	2010-05-22 22:20:41.0 +0200
+++ tinyproxy-1.8.1/src/conf.c	2010-05-22 22:31:03.732746545 +0200
@@ -317,7 +317,7 @@
  *
  * Returns 0 on success; negative upon failure.
  */
-static int config_compile (void)
+int initialize_directives (void)
 {
 unsigned int i, r;
 
@@ -338,6 +338,18 @@
 return 0;
 }
 
+void deinitialize_directives (void)
+{
+unsigned int i;
+
+for (i = 0; i != ndirectives; ++i) {
+assert (directives[i].cre);
+
+regfree (directives[i].cre);
+safefree (directives[i].cre);
+}
+}
+
 /*
  * Attempt to match the supplied line with any of the configuration
  * regexes defined above.  If a match is found, call the handler
@@ -397,7 +409,7 @@
 goto done;
 }
 
-if (config_compile () || config_parse (conf, config_file)) {
+if (config_parse (conf, config_file)) {
 fprintf (stderr, "Unable to parse config file. "
  "Not starting.\n");
 goto done;
diff -rub tinyproxy-orig/src/conf.h tinyproxy-1.8.1/src/conf.h
--- tinyproxy-orig/src/conf.h	2010-05-22 22:20:41.0 +0200
+++ tinyproxy-1.8.1/src/conf.h	2010-05-22 22:30:50.524954887 +0200
@@ -115,4 +115,7 @@
 extern int reload_config_file (const char *config_fname, struct config_s *conf,
struct config_s *defaults);
 
+int initialize_directives (void);
+void deinitialize_directives (void);
+
 #endif
diff -rub tinyproxy-orig/src/main.c tinyproxy-1.8.1/src/main.c
--- tinyproxy-orig/src/main.c	2010-03-09 12:41:45.0 +0100
+++ tinyproxy-1.8.1/src/main.c	2010-05-22 22:30:53.383355647 +0200
@@ -364,6 +364,9 @@
 
 log_message (LOG_INFO, "Initializing " PACKAGE " ...");
 
+		initialize_directives();
+		atexit(deinitialize_directives);
+
 initialize_config_defaults (&config_defaults);
 process_cmdline (argc, argv, &config_defaults);