zsh-workers
 help / color / mirror / code / Atom feed
* memory leaks report
@ 2004-03-10  7:46 Felix Rosencrantz
  2004-03-18 11:34 ` Peter Stephenson
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Felix Rosencrantz @ 2004-03-10  7:46 UTC (permalink / raw)
  To: zw

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=us-ascii, Size: 1844 bytes --]

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.


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)


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)

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 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)


-FR.

__________________________________
Do you Yahoo!?
Yahoo! Search - Find what you’re looking for faster
http://search.yahoo.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
                   ` (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

end of thread, other threads:[~2004-03-18 12:29 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).