Re: memory leak in unload(`gnu')

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Eric Blake on 10/3/2006 6:56 AM:
> 2006-10-03  Eric Blake  <address@hidden>
> 
>       Partially plug memory leak when unloading gnu module.
>       * tests/modules.at (unload gnu module): New test.
>       * modules/gnu.c (gnu_LTX_m4_finish_module): New function.
>       (m4_regexp_compile): Move static storage to module visibility.
>       * tests/options.at (--debug): Adjust to new output.

It helps if I actually attach it :)

- --
Life is short - so eat dessert first!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.1 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFFImkE84KuGfSFAYARAmHjAKC7TD8D6G2/MXLsDcEOjyvM98Wa0ACglVyV
heEwcTMVMfK1QQMggmRtzos=
=o8K2
-----END PGP SIGNATURE-----

Index: tests/modules.at
===================================================================
RCS file: /sources/m4/m4/tests/modules.at,v
retrieving revision 1.22
retrieving revision 1.23
diff -u -p -r1.22 -r1.23
--- tests/modules.at    14 Sep 2006 03:37:48 -0000      1.22
+++ tests/modules.at    3 Oct 2006 12:57:18 -0000       1.23
@@ -436,3 +436,28 @@ AT_CHECK_M4([-M "$abs_builddir" -m load 
            [0], [expout], [experr])
 
 AT_CLEANUP
+
+
+## ----------------- ##
+## unload gnu module ##
+## ----------------- ##
+
+AT_SETUP([unload gnu module])
+AT_CHECK_DYNAMIC_MODULE
+
+dnl Ensure that the gnu module does not leak memory.  I don't know how
+dnl to portably artificially limit the heap to cause an out-of-memory
+dnl condition in the case of a leak, but examining the run of this test
+dnl in a debugger can show whether it is leaking.
+AT_DATA([input.m4], [[divert(-1)
+define(`forloop',
+  `pushdef(`$1', `$2')_forloop(`$1', `$2', `$3', `$4')popdef(`$1')')
+define(`_forloop',
+  `$4`'ifelse($1, `$3', `',
+    `define(`$1', incr($1))_forloop(`$1', `$2', `$3', `$4')')')
+forloop(`i', `1', `5000', `unload(`gnu')load(`gnu')regexp(`123', `\(4\)?2')')
+]])
+
+AT_CHECK_M4([-m load input.m4], [0])
+
+AT_CLEANUP
Index: tests/options.at
===================================================================
RCS file: /sources/m4/m4/tests/options.at,v
retrieving revision 1.15
retrieving revision 1.16
diff -u -p -r1.15 -r1.16
--- tests/options.at    29 Sep 2006 18:20:13 -0000      1.15
+++ tests/options.at    3 Oct 2006 12:57:18 -0000       1.16
@@ -218,6 +218,7 @@ m4trace:in:2: -1- id 2: len(`abc') -> ??
 m4trace:in:2: -1- id 2: len(...) -> `3'
 m4debug:in:3: input exhausted
 m4debug: module gnu: symbols unloaded
+m4debug: module gnu: finish hook called
 m4debug: module gnu: closed
 m4debug: module m4: symbols unloaded
 m4debug: module m4: resident module not closed
Index: modules/gnu.c
===================================================================
RCS file: /sources/m4/m4/modules/gnu.c,v
retrieving revision 1.59
retrieving revision 1.60
diff -u -p -r1.59 -r1.60
--- modules/gnu.c       29 Sep 2006 18:20:12 -0000      1.59
+++ modules/gnu.c       3 Oct 2006 12:57:18 -0000       1.60
@@ -109,6 +109,7 @@ typedef struct {
   struct re_registers regs;    /* match registers */
 } m4_pattern_buffer;
 
+static m4_pattern_buffer buf;  /* compiled regular expression */
 
 /* Compile a REGEXP using the RESYNTAX bits, and return the buffer.
    Report errors on behalf of CALLER.  If NO_SUB, optimize the
@@ -126,10 +127,8 @@ m4_regexp_compile (m4 *context, const ch
      for its syntax (but at least the compiled regex remembers its
      syntax even if the global variable changes later), and since we
      use a static variable.  To be reentrant, we would need a mutex in
-     this method, and we should have a way to free the memory used by
-     buf when this module is unloaded.  */
+     this method, and move the storage for buf into context.  */
 
-  static m4_pattern_buffer buf;        /* compiled regular expression */
   const char *msg;             /* error message from re_compile_pattern */
 
   re_set_syntax (resyntax);
@@ -280,6 +279,17 @@ m4_regexp_substitute (m4 *context, m4_ob
 }
 
 
+/* Reclaim memory used by this module.  */
+M4FINISH_HANDLER(gnu)
+{
+  regfree (&buf.pat);
+  free (buf.regs.start);
+  free (buf.regs.end);
+  /* If this module was preloaded, then we need to explicitly reset
+     the memory in case it gets reloaded.  */
+  memset (&buf, 0, sizeof buf);
+}
+
 
 
 /**