BUG: permanent allocation in mathevall

BUG: permanent allocation in mathevall

[Bart Schaefer]

I'm getting the "BUG: permanent allocation in mathevall" message every time
my TRAPALRM function fires.

TRAPALRM () {
        TMOUT=60 
        title
}

It's happening below getiparam("TMOUT"):

#0  mathevall (s=0x819cea0 "6", prek=17, ep=0xbffff0b8)
    at ../../zsh-3.1.6/Src/math.c:852
#1  0x8075097 in matheval (s=0x819cea0 "6") at ../../zsh-3.1.6/Src/math.c:913
#2  0x80750ef in mathevali (s=0x819cea0 "6") at ../../zsh-3.1.6/Src/math.c:924
#3  0x807a710 in getintvalue (v=0x80f6340) at ../../zsh-3.1.6/Src/params.c:1368
#4  0x807b2c5 in getiparam (s=0x80cee62 "")
    at ../../zsh-3.1.6/Src/params.c:1603
#5  0x80875f5 in handler (sig=14) at ../../zsh-3.1.6/Src/signals.c:526
#6  <signal handler called>
#7  0x4009c6f4 in __read ()
#8  0x80a6607 in getkeybuf (w=0)
    at ../../../zsh-3.1.6/Src/Zle/zle_keymap.c:1191

Unfortunately, I'm not sure what to do about it.  I believe signal handlers
deliberately avoid using the heap because of issues about when it should be
popped, but I don't recall for certain.

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Re: BUG: permanent allocation in mathevall

[Peter Stephenson]

"Bart Schaefer" wrote:
> I'm getting the "BUG: permanent allocation in mathevall" message every time
> my TRAPALRM function fires.
> 
> TRAPALRM () {
>         TMOUT=60 
>         title
> }
> 
> It's happening below getiparam("TMOUT"):
> 
> Unfortunately, I'm not sure what to do about it.  I believe signal handlers
> deliberately avoid using the heap because of issues about when it should be
> popped, but I don't recall for certain.

It doesn't seem to be happening with TRAPUSR1, though: compare

TRAPUSR1() { integer i; i=4; print i is $i; }
kill -USR1 $$

with the same function as TRAPALRM with a non-zero timeout.  Slightly odd.

The easiest thing to do is to make user mathparse() gets called from
mathevall() with heap allocation.  It would be easy to write a heap-only
version of dupstring(), in fact I would feel a lot happier if there was
such a version, since dupstring() is almost always called with no intention
of every freeing the space.  However, the heap-alloced linked list for the
function call rather sets the cat among the pigeons.  We could zalloc() and
free that, of course.  With both these changes math evaluation would be
independent of allocation strategy, apart from possible problems with calls
into the parameter code, so maybe that's the right thing to do and this is
just too lazy.

There's an ancient issue with traps which has been in the back of mind for
years:  it would be much safer not to run them asynchronously at arbitrary
points, but to set a flag and call the trap at one of several strategic
points, e.g. after a foreground command has run or when an editor read is
interrupted.  This would mean traps not being run during a foreground
programme, I don't know how many people that would inconvenience.  We had
some of this discussion a long time ago.

--- Src/math.c.bak	Thu Sep 23 15:42:20 1999
+++ Src/math.c	Sun Sep 26 15:54:54 1999
@@ -852,7 +852,6 @@
     struct mathvalue *xstack = 0, nstack[STACKSZ];
     mnumber ret;
 
-    MUSTUSEHEAP("mathevall");
     if (mlevel++) {
 	xlastbase = lastbase;
 	xnoeval = noeval;
@@ -875,7 +874,9 @@
     ptr = s;
     sp = -1;
     unary = 1;
-    mathparse(prek);
+    HEAPALLOC {
+	mathparse(prek);
+    } LASTALLOC;
     *ep = ptr;
     DPUTS(!errflag && sp,
 	  "BUG: math: wallabies roaming too freely in outback");

-- 
Peter Stephenson <pws@ibmth.df.unipi.it>       Tel: +39 050 844536
WWW:  http://www.ifh.de/~pws/
Dipartimento di Fisica, Via Buonarroti 2, 56127 Pisa, Italy

Re: BUG: permanent allocation in mathevall

[Bart Schaefer]

On Sep 26,  4:02pm, Peter Stephenson wrote:
} Subject: Re: BUG: permanent allocation in mathevall
}
} "Bart Schaefer" wrote:
} > I'm getting the "BUG: permanent allocation in mathevall" message every time
} > my TRAPALRM function fires.
} > 
} > It's happening below getiparam("TMOUT"):
} 
} It doesn't seem to be happening with TRAPUSR1, though: compare
} 
} TRAPUSR1() { integer i; i=4; print i is $i; }
} kill -USR1 $$
} 
} with the same function as TRAPALRM with a non-zero timeout.  Slightly odd.

I should have looked more closely at this before reporting it ...

If you look again at the stack trace I sent, you'll see that TMOUT is a
special case; the code looks like this:

	    int tmout;
            dotrap(SIGALRM);
            if ((tmout = getiparam("TMOUT")))
                alarm(tmout);           /* reset the alarm */

Outside of dotrap(), global allocation is in effect.  Inside the trap
function itself, heap allocation is.

It looks like that's the only place where getiparam() is called in a
global allocation context, so maybe it'd be sufficient to put HEAPALLOC()
around that one call, rather than inside mathevall().

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Re: BUG: permanent allocation in mathevall

[Peter Stephenson]

"Bart Schaefer" wrote:
> If you look again at the stack trace I sent, you'll see that TMOUT is a
> special case; the code looks like this:
> 
> 	    int tmout;
>             dotrap(SIGALRM);
>             if ((tmout = getiparam("TMOUT")))
>                 alarm(tmout);           /* reset the alarm */
> 
> Outside of dotrap(), global allocation is in effect.  Inside the trap
> function itself, heap allocation is.

OK, then use this instead of 8049.  The allocation at this point is
undetermined, since it's in the signal handler, so this certainly seems
safeer.  Presumably global allocation is in effect only because it is in
zleread().

--- Src/signals.c.ha	Thu Sep  2 10:09:24 1999
+++ Src/signals.c	Sun Sep 26 18:52:31 1999
@@ -523,8 +523,10 @@
         if (sigtrapped[SIGALRM]) {
 	    int tmout;
             dotrap(SIGALRM);
-            if ((tmout = getiparam("TMOUT")))
-                alarm(tmout);           /* reset the alarm */
+	    HEAPALLOC {
+		if ((tmout = getiparam("TMOUT")))
+		    alarm(tmout);           /* reset the alarm */
+	    } LASTALLOC;
         } else {
 	    int idle = ttyidlegetfn(NULL);
 	    int tmout = getiparam("TMOUT");

-- 
Peter Stephenson <pws@ibmth.df.unipi.it>       Tel: +39 050 844536
WWW:  http://www.ifh.de/~pws/
Dipartimento di Fisica, Via Buonarroti 2, 56127 Pisa, Italy