* Re: memory leaks report
2004-03-10 7:46 memory leaks report Felix Rosencrantz
@ 2004-03-18 11:34 ` Peter Stephenson
2004-03-18 11:54 ` Peter Stephenson
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Peter Stephenson @ 2004-03-18 11:34 UTC (permalink / raw)
To: zw
Felix Rosencrantz wrote:
> Test B02typeset:
> 28 bytes in 4 blocks are definitely lost in loss record 1 of 11
> at malloc (vg_replace_malloc.c:153)
> by zalloc (mem.c:490)
> by ztrdup (string.c:52)
> by bin_typeset (builtin.c:2264)
Think I've finally found this one.
Index: Src/params.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/params.c,v
retrieving revision 1.77
diff -u -r1.77 params.c
--- Src/params.c 8 Mar 2004 11:01:19 -0000 1.77
+++ Src/params.c 18 Mar 2004 11:23:37 -0000
@@ -2673,6 +2673,7 @@
zfree(pm->u.data, sizeof(struct tieddata));
/* paranoia -- shouldn't need these, but in case we reuse the struct... */
pm->u.data = NULL;
+ zsfree(pm->ename);
pm->flags &= ~PM_TIED;
}
--
Peter Stephenson <pws@csr.com> Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK Tel: +44 (0)1223 692070
**********************************************************************
This email and any files transmitted with it are confidential and
intended solely for the use of the individual or entity to whom they
are addressed. If you have received this email in error please notify
the system manager.
This footnote also confirms that this email message has been swept by
MIMEsweeper for the presence of computer viruses.
www.mimesweeper.com
**********************************************************************
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: memory leaks report
2004-03-10 7:46 memory leaks report Felix Rosencrantz
2004-03-18 11:34 ` Peter Stephenson
@ 2004-03-18 11:54 ` Peter Stephenson
2004-03-18 12:21 ` Peter Stephenson
2004-03-18 12:28 ` Peter Stephenson
3 siblings, 0 replies; 5+ messages in thread
From: Peter Stephenson @ 2004-03-18 11:54 UTC (permalink / raw)
To: zw
Felix Rosencrantz wrote:
> Here are the latest memory leaks found by valgrind from around March 6th.
> Most were all found before, but I didn't provide much context.... So here's
> a little more. If there is a a simple way to force the tests to execute a
> separate shell per testcase, it would make it easier to link up the leaks to
> the test case.
You can set ZTST_verbose=2 in the environment to give more context
for the tests, so you can see when reports appear. A separate shell
for each test is harder (and slower).
> Test E01options:
> Valgrind reports an error summary after every zsh subprocess exits. Running
> the
> E01options test, there are 42 error summaries. This leak is first reported=
> in
> the 32nd error summary. It looks like there are 48 tests.
> 493 bytes in 1 blocks are definitely lost in loss record 16 of 21
> at malloc (vg_replace_malloc.c:153)
> by zalloc (mem.c:490)
> by mkenvstr (params.c:3423)
> by addenv (params.c:3386)
> by restore_params (exec.c:2590)
Well, well, well, who would have thought it, a bug in the parameter
code. I think it's the following, which looks like it should logically
be there in save_params to match the chunk in restore_params which is
causing the leak.
Index: Src/exec.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
retrieving revision 1.57
diff -u -r1.57 exec.c
--- Src/exec.c 13 Nov 2003 14:34:38 -0000 1.57
+++ Src/exec.c 18 Mar 2004 11:43:38 -0000
@@ -2519,6 +2519,10 @@
while (wc_code(ac = *pc) == WC_ASSIGN) {
s = ecrawstr(state->prog, pc + 1, NULL);
if ((pm = (Param) paramtab->getnode(paramtab, s))) {
+ if (pm->env) {
+ delenv(pm->env);
+ pm->env = NULL;
+ }
if (!(pm->flags & PM_SPECIAL)) {
paramtab->removenode(paramtab, s);
} else if (!(pm->flags & PM_READONLY) &&
--
Peter Stephenson <pws@csr.com> Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK Tel: +44 (0)1223 692070
**********************************************************************
This email and any files transmitted with it are confidential and
intended solely for the use of the individual or entity to whom they
are addressed. If you have received this email in error please notify
the system manager.
This footnote also confirms that this email message has been swept by
MIMEsweeper for the presence of computer viruses.
www.mimesweeper.com
**********************************************************************
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: memory leaks report
2004-03-10 7:46 memory leaks report Felix Rosencrantz
2004-03-18 11:34 ` Peter Stephenson
2004-03-18 11:54 ` Peter Stephenson
@ 2004-03-18 12:21 ` Peter Stephenson
2004-03-18 12:28 ` Peter Stephenson
3 siblings, 0 replies; 5+ messages in thread
From: Peter Stephenson @ 2004-03-18 12:21 UTC (permalink / raw)
To: zw
Felix Rosencrantz wrote:
> V01zmodload.ztst:
> This is a new leak:
> 4 bytes in 1 blocks are definitely lost in loss record 1 of 19
> at malloc (vg_replace_malloc.c:153)
> by zalloc (mem.c:490)
> by ztrdup (string.c:52)
> by add_automathfunc (module.c:2118)
> by bin_zmodload_math (module.c:1397)
> by bin_zmodload (module.c:1005)
This was easy to track down, but in the process I discovered that
`zmodload -af' is fatally flawed: you can't autoload more than one
function from the same library, since it complains about replacing
autoloadable math functions other than the one that triggered module
loading. Since autoloading one math function is hardly worth it, and
since there is only one math library, it rendered the feature useless.
This seems to fix it. We need more tests for autoloading.
Index: Src/module.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/module.c,v
retrieving revision 1.13
diff -u -r1.13 module.c
--- Src/module.c 11 Mar 2004 14:27:10 -0000 1.13
+++ Src/module.c 18 Mar 2004 12:08:23 -0000
@@ -2039,6 +2039,19 @@
MathFunc mathfuncs;
/**/
+static void removemathfunc(MathFunc previous, MathFunc current)
+{
+ if (previous)
+ previous->next = current->next;
+ else
+ mathfuncs = current->next;
+
+ zsfree(current->name);
+ zsfree(current->module);
+ zfree(current, sizeof(*current));
+}
+
+/**/
MathFunc
getmathfunc(char *name, int autol)
{
@@ -2049,13 +2062,7 @@
if (autol && p->module) {
char *n = dupstring(p->module);
- if (q)
- q->next = p->next;
- else
- mathfuncs = p->next;
-
- zsfree(p->module);
- zfree(p, sizeof(*p));
+ removemathfunc(q, p);
load_module(n);
@@ -2071,14 +2078,22 @@
mod_export int
addmathfunc(MathFunc f)
{
- MathFunc p;
+ MathFunc p, q = NULL;
if (f->flags & MFF_ADDED)
return 1;
- for (p = mathfuncs; p; p = p->next)
- if (!strcmp(f->name, p->name))
+ for (p = mathfuncs; p; q = p, p = p->next)
+ if (!strcmp(f->name, p->name)) {
+ if (p->module) {
+ /*
+ * Autoloadable, replace.
+ */
+ removemathfunc(q, p);
+ break;
+ }
return 1;
+ }
f->flags |= MFF_ADDED;
f->next = mathfuncs;
--
Peter Stephenson <pws@csr.com> Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK Tel: +44 (0)1223 692070
**********************************************************************
This email and any files transmitted with it are confidential and
intended solely for the use of the individual or entity to whom they
are addressed. If you have received this email in error please notify
the system manager.
This footnote also confirms that this email message has been swept by
MIMEsweeper for the presence of computer viruses.
www.mimesweeper.com
**********************************************************************
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: memory leaks report
2004-03-10 7:46 memory leaks report Felix Rosencrantz
` (2 preceding siblings ...)
2004-03-18 12:21 ` Peter Stephenson
@ 2004-03-18 12:28 ` Peter Stephenson
3 siblings, 0 replies; 5+ messages in thread
From: Peter Stephenson @ 2004-03-18 12:28 UTC (permalink / raw)
To: zw
Felix Rosencrantz wrote:
> This is an old leak
> 68 bytes in 1 blocks are definitely lost in loss record 8 of 19
> at malloc (vg_replace_malloc.c:153)
> by zshcalloc (mem.c:508)
> by newhashtable (hashtable.c:99)
> by ???
> by ???
> by dyn_boot_module (module.c:625)
> by boot_module (module.c:677)
> by load_module (module.c:784)
> by require_module (module.c:889)
> by bin_zmodload_load (module.c:1615)
I had a go at this last one before, but there's not enough information,
so I had to guess. Some module initialisation function is initialising
a hash table; the obvious one is zsh/parameter, but there was no obvious
sign of a leak there.
A test wehre every module is loaded sparately by hand with `zmodload
zsh/...' should be able to pick up which it is.
--
Peter Stephenson <pws@csr.com> Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK Tel: +44 (0)1223 692070
**********************************************************************
This email and any files transmitted with it are confidential and
intended solely for the use of the individual or entity to whom they
are addressed. If you have received this email in error please notify
the system manager.
This footnote also confirms that this email message has been swept by
MIMEsweeper for the presence of computer viruses.
www.mimesweeper.com
**********************************************************************
^ permalink raw reply [flat|nested] 5+ messages in thread