zsh-workers
 help / color / mirror / code / Atom feed
* zsh 5.0.6 hanged in freejob from TRAPCHLD
@ 2014-09-30 17:21 Vincent Lefevre
  2014-09-30 23:18 ` Bart Schaefer
  0 siblings, 1 reply; 8+ messages in thread
From: Vincent Lefevre @ 2014-09-30 17:21 UTC (permalink / raw)
  To: zsh-workers

I typed:

  ps -aef|gr pulseaudio

and zsh hanged.

gr: aliased to grep -P --color=always

grep: aliased to pager-wrapper grep

pager-wrapper () {
        if [[ -t 1 ]]
        then
                $@ | less -+c -FRX
        else
                $@
        fi
}

TRAPCHLD () {
        [[ -o interactive && -n $TTY ]] && updprompt
}

updprompt () {
        [[ -n $zsh410 && -n $TTY && -n $terminfo ]] && {
                echoti rmacs
                echoti sgr0
                echoti cnorm
        } > $TTY 2> /dev/null
        psvar[2]="" 
        if [[ $domain = local.ay && "$(pmu_battery)" = "Battery" ]]
        then
                psvar[2]="[$(pmu_percent)%]" 
        elif [[ $domain = local.xvii ]]
        then
                local battery="$(acpi -b)"
                [[ $battery = *Discharging* ]] && psvar[2]="[${${battery#*Discharging, }%\%,*}%]" 
        fi
        local njobs jobstr
        njobs=$#jobstates 
        [[ $njobs -gt 1 ]] && jobstr="s" 
        [[ $njobs -ge 1 ]] && jobstr=" $njobs job$jobstr |" 
        [[ -n $TTY && $TERM = (xterm*|dtterm|mlterm|rxvt*|screen*) ]] && {
                [[ $TERM = screen* ]] || print -nP "\e]1;%m:%.\x07"
                print -nP "\e]2;${jobstr}${WINTITLE:+ $WINTITLE |} %n@%m - %~ | %y\x07"
        } > $TTY
}

This zsh instance has no children. The backtrace:

__lll_lock_wait_private ()
    at ../nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:95
95      ../nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S: No such file or directory.
(gdb) bt
#0  __lll_lock_wait_private ()
    at ../nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:95
#1  0x00007fe153d2efb9 in _L_lock_3719 () from /lib/x86_64-linux-gnu/libc.so.6
#2  0x00007fe153d2a2bb in _int_free (av=0x7fe154053620 <main_arena>, 
    p=0xbf0680, have_lock=0) at malloc.c:3943
#3  0x0000000000441d0d in freejob (jn=jn@entry=0x9afe30, 
    deleting=deleting@entry=1) at ../../Src/jobs.c:1240
#4  0x00000000004408b8 in deletejob (jn=jn@entry=0x9afe30, 
    disowning=disowning@entry=0) at ../../Src/jobs.c:1295
#5  0x0000000000440b0f in printjob (jn=jn@entry=0x9afe30, lng=0, 
    synch=synch@entry=0) at ../../Src/jobs.c:1012
#6  0x0000000000441c10 in update_job (jn=0x9afe30) at ../../Src/jobs.c:565
#7  0x0000000000470bb1 in wait_for_processes () at ../../Src/signals.c:515
#8  0x00000000004712c5 in zhandler (sig=17) at ../../Src/signals.c:594
#9  <signal handler called>
#10 0x00007fe153d2a022 in _int_free (av=0x7fe154053620 <main_arena>, 
    p=<optimized out>, have_lock=0) at malloc.c:3990
#11 0x0000000000445eae in lexrestore () at ../../Src/lex.c:344
#12 0x0000000000447dbf in parse_subscript (s=0x7fe154b9d42f "]", 
    sub=sub@entry=1, endchar=endchar@entry=93) at ../../Src/lex.c:1692
#13 0x0000000000457dc3 in isident (s=s@entry=0x7fe154b9d428 "psvar[2]")
    at ../../Src/params.c:1036
#14 0x000000000045e66c in assignsparam (s=s@entry=0x7fe154b9d428 "psvar[2]", 
    val=val@entry=0xbf19a0 "", flags=flags@entry=0) at ../../Src/params.c:2631
#15 0x0000000000422f8a in addvars (state=0x7fffa1a3b030, pc=<optimized out>, 
    addflags=addflags@entry=0) at ../../Src/exec.c:2279
#16 0x0000000000423271 in execsimple (state=state@entry=0x7fffa1a3b030)
    at ../../Src/exec.c:1113
#17 0x000000000042a4d0 in execlist (state=state@entry=0x7fffa1a3b030, 
    dont_change_job=dont_change_job@entry=1, exiting=exiting@entry=0)
    at ../../Src/exec.c:1259
#18 0x000000000042aa60 in execode (p=p@entry=0x9be400, 
    dont_change_job=dont_change_job@entry=1, exiting=exiting@entry=0, 
    context=context@entry=0x48acce "shfunc") at ../../Src/exec.c:1070
#19 0x0000000000424ab1 in runshfunc (prog=0x9be400, wrap=0x0, 
    name=0x7fe154ba82c8 "updprompt") at ../../Src/exec.c:4895
#20 0x00000000004250f6 in doshfunc (shfunc=shfunc@entry=0x9d1950, 
    doshargs=doshargs@entry=0x7fe154ba8268, noreturnval=noreturnval@entry=0)
    at ../../Src/exec.c:4775
#21 0x00000000004254a0 in execshfunc (shf=0x9d1950, args=0x7fe154ba8268)
    at ../../Src/exec.c:4432
#22 0x0000000000428cf9 in execshfunc (args=<optimized out>, 
    shf=<optimized out>) at ../../Src/exec.c:4400
#23 execcmd (state=0x7fffa1a3b7c0, input=input@entry=0, output=131696, 
    output@entry=0, how=0, how@entry=18, last1=0) at ../../Src/exec.c:3269
#24 0x000000000042921e in execpline2 (state=state@entry=0x7fffa1a3b7c0, 
    pcode=pcode@entry=67, how=how@entry=18, input=0, output=0, 
    last1=last1@entry=0) at ../../Src/exec.c:1691
#25 0x00000000004295e6 in execpline (state=state@entry=0x7fffa1a3b7c0, 
    slcode=<optimized out>, how=how@entry=18, last1=0) at ../../Src/exec.c:1478
#26 0x000000000042a791 in execlist (state=state@entry=0x7fffa1a3b7c0, 
    dont_change_job=dont_change_job@entry=1, exiting=exiting@entry=0)
    at ../../Src/exec.c:1261
#27 0x000000000042aa60 in execode (p=p@entry=0x9bc9a0, 
    dont_change_job=dont_change_job@entry=1, exiting=exiting@entry=0, 
    context=context@entry=0x48acce "shfunc") at ../../Src/exec.c:1070
#28 0x0000000000424ab1 in runshfunc (prog=0x9bc9a0, wrap=0x0, 
    name=0x7fe154ba8218 "TRAPCHLD") at ../../Src/exec.c:4895
#29 0x00000000004250f6 in doshfunc (shfunc=shfunc@entry=0x9d1860, 
    doshargs=doshargs@entry=0xbeaee0, noreturnval=noreturnval@entry=1)
    at ../../Src/exec.c:4775
#30 0x000000000046febd in dotrapargs (sig=17, 
    sigtr=sigtr@entry=0x6c2504 <sigtrapped+68>, sigfn=0x9d1860)
    at ../../Src/signals.c:1229
#31 0x0000000000470d78 in dotrap (sig=sig@entry=17) at ../../Src/signals.c:1322
#32 0x0000000000441a6e in update_job (jn=0x9afdb0) at ../../Src/jobs.c:570
#33 0x0000000000470bb1 in wait_for_processes () at ../../Src/signals.c:515
#34 0x00000000004712c5 in zhandler (sig=17) at ../../Src/signals.c:594
#35 <signal handler called>
#36 0x00007fe153ce63b6 in do_sigsuspend (set=0x7fffa1a3c370)
    at ../sysdeps/unix/sysv/linux/sigsuspend.c:31
#37 __GI___sigsuspend (set=set@entry=0x7fffa1a3c370)
    at ../sysdeps/unix/sysv/linux/sigsuspend.c:41
#38 0x0000000000470766 in signal_suspend (sig=sig@entry=17, 
    wait_cmd=wait_cmd@entry=0) at ../../Src/signals.c:375
#39 0x0000000000442186 in zwaitjob (job=<optimized out>, wait_cmd=0)
    at ../../Src/jobs.c:1454
#40 0x0000000000442867 in waitjobs () at ../../Src/jobs.c:1499
#41 0x0000000000429c0b in execpline (state=state@entry=0x7fffa1a3cc40, 
    slcode=<optimized out>, how=<optimized out>, how@entry=18, last1=0)
    at ../../Src/exec.c:1554
#42 0x000000000042a791 in execlist (state=state@entry=0x7fffa1a3cc40, 
    dont_change_job=dont_change_job@entry=1, exiting=exiting@entry=0)
    at ../../Src/exec.c:1261
#43 0x000000000044a7b1 in execif (state=0x7fffa1a3cc40, do_exec=0)
    at ../../Src/loop.c:525
#44 0x0000000000428126 in execcmd (state=0x7fffa1a3cc40, input=8, 
    input@entry=0, output=output@entry=0, how=-1, how@entry=18, last1=0)
    at ../../Src/exec.c:3232
#45 0x000000000042921e in execpline2 (state=state@entry=0x7fffa1a3cc40, 
    pcode=pcode@entry=195, how=how@entry=18, input=0, output=0, 
    last1=last1@entry=0) at ../../Src/exec.c:1691
#46 0x00000000004295e6 in execpline (state=state@entry=0x7fffa1a3cc40, 
    slcode=<optimized out>, how=how@entry=18, last1=0) at ../../Src/exec.c:1478
#47 0x000000000042a791 in execlist (state=state@entry=0x7fffa1a3cc40, 
    dont_change_job=dont_change_job@entry=1, exiting=exiting@entry=0)
    at ../../Src/exec.c:1261
#48 0x000000000042aa60 in execode (p=p@entry=0x9da200, 
    dont_change_job=dont_change_job@entry=1, exiting=exiting@entry=0, 
    context=context@entry=0x48acce "shfunc") at ../../Src/exec.c:1070
#49 0x0000000000424ab1 in runshfunc (prog=0x9da200, wrap=0x0, 
    name=0x7fe154ba8090 "pager-wrapper") at ../../Src/exec.c:4895
#50 0x00000000004250f6 in doshfunc (shfunc=shfunc@entry=0x9da2c0, 
    doshargs=doshargs@entry=0x7fe154ba7f68, noreturnval=noreturnval@entry=0)
    at ../../Src/exec.c:4775
#51 0x00000000004254a0 in execshfunc (shf=0x9da2c0, args=0x7fe154ba7f68)
    at ../../Src/exec.c:4432
#52 0x0000000000428cf9 in execshfunc (args=<optimized out>, 
    shf=<optimized out>) at ../../Src/exec.c:4400
#53 execcmd (state=0x7fffa1a3d470, input=8, input@entry=11, 
    output=output@entry=0, how=-1, how@entry=18, last1=0)
    at ../../Src/exec.c:3269
#54 0x000000000042921e in execpline2 (state=state@entry=0x7fffa1a3d470, 
    pcode=<optimized out>, how=how@entry=18, input=11, output=output@entry=0, 
    last1=last1@entry=0) at ../../Src/exec.c:1691
#55 0x00000000004291dd in execpline2 (state=state@entry=0x7fffa1a3d470, 
    pcode=pcode@entry=163, how=how@entry=18, input=0, output=0, 
    last1=last1@entry=0) at ../../Src/exec.c:1752
#56 0x00000000004295e6 in execpline (state=state@entry=0x7fffa1a3d470, 
    slcode=<optimized out>, how=how@entry=18, last1=0) at ../../Src/exec.c:1478
#57 0x000000000042a791 in execlist (state=state@entry=0x7fffa1a3d470, 
    dont_change_job=dont_change_job@entry=0, exiting=exiting@entry=0)
    at ../../Src/exec.c:1261
#58 0x000000000042aa60 in execode (p=p@entry=0x7fe154ba7ce8, 
    dont_change_job=dont_change_job@entry=0, exiting=exiting@entry=0, 
    context=context@entry=0x48c2f1 "toplevel") at ../../Src/exec.c:1070
#59 0x000000000043b5cc in loop (toplevel=toplevel@entry=1, 
    justonce=justonce@entry=0) at ../../Src/init.c:185
#60 0x000000000043e6f6 in zsh_main (argc=<optimized out>, argv=<optimized out>)
    at ../../Src/init.c:1625
#61 0x00007fe153cd2b45 in __libc_start_main (main=0x40f1a0 <main>, argc=1, 
    argv=0x7fffa1a3d6a8, init=<optimized out>, fini=<optimized out>, 
    rtld_fini=<optimized out>, stack_end=0x7fffa1a3d698) at libc-start.c:287
#62 0x000000000040f1ce in _start ()

-- 
Vincent Lefèvre <vincent@vinc17.net> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: zsh 5.0.6 hanged in freejob from TRAPCHLD
  2014-09-30 17:21 zsh 5.0.6 hanged in freejob from TRAPCHLD Vincent Lefevre
@ 2014-09-30 23:18 ` Bart Schaefer
  2014-10-01  0:53   ` PATCH signal-safe lexrestore() [Re: zsh 5.0.6 hanged in freejob from TRAPCHLD] Bart Schaefer
  2014-10-01  9:00   ` zsh 5.0.6 hanged in freejob from TRAPCHLD Peter Stephenson
  0 siblings, 2 replies; 8+ messages in thread
From: Bart Schaefer @ 2014-09-30 23:18 UTC (permalink / raw)
  To: Zsh hackers list

[-- Attachment #1: Type: text/plain, Size: 683 bytes --]

On Sep 30, 2014 10:30 AM, "Vincent Lefevre" <vincent@vinc17.net> wrote:
>
> I typed:
>
>   ps -aef|gr pulseaudio
>
> and zsh hanged.

The stack trace seems to indicate that the problem likely originates in
lexrestore() which calls free() directly (without the signal-safe zfree()
wrapper).  There's a corresponding problem in lexsave() with a direct call
to malloc().  The CHLD signal interrupts the free, leading to internal
deadlock when another free is called (correctly via zfree this time, though
it is optimized out of the backtrace).

This resembles the problems in execsave()/execrestore() that I fixed last
October.  I'll send a patch later if no one else gets there first.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* PATCH signal-safe lexrestore() [Re: zsh 5.0.6 hanged in freejob from TRAPCHLD]
  2014-09-30 23:18 ` Bart Schaefer
@ 2014-10-01  0:53   ` Bart Schaefer
  2014-10-01  9:00   ` zsh 5.0.6 hanged in freejob from TRAPCHLD Peter Stephenson
  1 sibling, 0 replies; 8+ messages in thread
From: Bart Schaefer @ 2014-10-01  0:53 UTC (permalink / raw)
  To: Zsh hackers list

On Sep 30,  4:18pm, Bart Schaefer wrote:
}
} The stack trace seems to indicate that the problem likely originates in
} lexrestore() which calls free() directly (without the signal-safe zfree()
} wrapper).
} 
} This resembles the problems in execsave()/execrestore() that I fixed last
} October.  I'll send a patch later if no one else gets there first.

Fix is almost exactly the same, except for field and variable names.


diff --git a/Src/lex.c b/Src/lex.c
index 8e9a49f..1a854f5 100644
--- a/Src/lex.c
+++ b/Src/lex.c
@@ -325,66 +325,70 @@ lexsave(void)
 mod_export void
 lexrestore(void)
 {
-    struct lexstack *ln;
+    struct lexstack *ln = lstack;
 
     DPUTS(!lstack, "BUG: lexrestore() without lexsave()");
-    incmdpos = lstack->incmdpos;
-    incond = lstack->incond;
-    incasepat = lstack->incasepat;
-    dbparens = lstack->dbparens;
-    isfirstln = lstack->isfirstln;
-    isfirstch = lstack->isfirstch;
-    histactive = lstack->histactive;
-    histdone = lstack->histdone;
-    lexflags = lstack->lexflags;
-    stophist = lstack->stophist;
-    chline = lstack->hline;
-    hptr = lstack->hptr;
+
+    queue_signals();
+    lstack = lstack->next;
+
+    if (!lstack) {
+	/* Back to top level: don't need special ZLE value */
+	DPUTS(ln->hline != zle_chline, "BUG: Ouch, wrong chline for ZLE");
+	zle_chline = NULL;
+    }
+
+    incmdpos = ln->incmdpos;
+    incond = ln->incond;
+    incasepat = ln->incasepat;
+    dbparens = ln->dbparens;
+    isfirstln = ln->isfirstln;
+    isfirstch = ln->isfirstch;
+    histactive = ln->histactive;
+    histdone = ln->histdone;
+    lexflags = ln->lexflags;
+    stophist = ln->stophist;
+    chline = ln->hline;
+    hptr = ln->hptr;
     if (cmdstack)
-	free(cmdstack);
-    cmdstack = lstack->cstack;
-    cmdsp = lstack->csp;
-    tok = lstack->tok;
-    isnewlin = lstack->isnewlin;
-    tokstr = lstack->tokstr;
-    zshlextext = lstack->zshlextext;
-    bptr = lstack->bptr;
-    bsiz = lstack->bsiz;
-    len = lstack->len;
-    chwords = lstack->chwords;
-    chwordlen = lstack->chwordlen;
-    chwordpos = lstack->chwordpos;
-    hwgetword = lstack->hwgetword;
-    lexstop = lstack->lexstop;
-    hdocs = lstack->hdocs;
-    hgetc = lstack->hgetc;
-    hungetc = lstack->hungetc;
-    hwaddc = lstack->hwaddc;
-    hwbegin = lstack->hwbegin;
-    hwend = lstack->hwend;
-    addtoline = lstack->addtoline;
+	zfree(cmdstack, CMDSTACKSZ);
+    cmdstack = ln->cstack;
+    cmdsp = ln->csp;
+    tok = ln->tok;
+    isnewlin = ln->isnewlin;
+    tokstr = ln->tokstr;
+    zshlextext = ln->zshlextext;
+    bptr = ln->bptr;
+    bsiz = ln->bsiz;
+    len = ln->len;
+    chwords = ln->chwords;
+    chwordlen = ln->chwordlen;
+    chwordpos = ln->chwordpos;
+    hwgetword = ln->hwgetword;
+    lexstop = ln->lexstop;
+    hdocs = ln->hdocs;
+    hgetc = ln->hgetc;
+    hungetc = ln->hungetc;
+    hwaddc = ln->hwaddc;
+    hwbegin = ln->hwbegin;
+    hwend = ln->hwend;
+    addtoline = ln->addtoline;
     if (ecbuf)
 	zfree(ecbuf, eclen);
-    eclen = lstack->eclen;
-    ecused = lstack->ecused;
-    ecnpats = lstack->ecnpats;
-    ecbuf = lstack->ecbuf;
-    ecstrs = lstack->ecstrs;
-    ecsoffs = lstack->ecsoffs;
-    ecssub = lstack->ecssub;
-    ecnfunc = lstack->ecnfunc;
-    hlinesz = lstack->hlinesz;
-    toklineno = lstack->toklineno;
+    eclen = ln->eclen;
+    ecused = ln->ecused;
+    ecnpats = ln->ecnpats;
+    ecbuf = ln->ecbuf;
+    ecstrs = ln->ecstrs;
+    ecsoffs = ln->ecsoffs;
+    ecssub = ln->ecssub;
+    ecnfunc = ln->ecnfunc;
+    hlinesz = ln->hlinesz;
+    toklineno = ln->toklineno;
     errflag = 0;
+    free(ln);
 
-    ln = lstack->next;
-    if (!ln) {
-	/* Back to top level: don't need special ZLE value */
-	DPUTS(chline != zle_chline, "BUG: Ouch, wrong chline for ZLE");
-	zle_chline = NULL;
-    }
-    free(lstack);
-    lstack = ln;
+    unqueue_signals();
 }
 
 /**/


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: zsh 5.0.6 hanged in freejob from TRAPCHLD
  2014-09-30 23:18 ` Bart Schaefer
  2014-10-01  0:53   ` PATCH signal-safe lexrestore() [Re: zsh 5.0.6 hanged in freejob from TRAPCHLD] Bart Schaefer
@ 2014-10-01  9:00   ` Peter Stephenson
  2014-10-01 14:53     ` Bart Schaefer
  1 sibling, 1 reply; 8+ messages in thread
From: Peter Stephenson @ 2014-10-01  9:00 UTC (permalink / raw)
  To: Zsh hackers list

On Tue, 30 Sep 2014 16:18:43 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Sep 30, 2014 10:30 AM, "Vincent Lefevre" <vincent@vinc17.net> wrote:
> >
> > I typed:
> >
> >   ps -aef|gr pulseaudio
> >
> > and zsh hanged.
> 
> The stack trace seems to indicate that the problem likely originates in
> lexrestore() which calls free() directly (without the signal-safe zfree()
> wrapper).  There's a corresponding problem in lexsave() with a direct call
> to malloc().  The CHLD signal interrupts the free, leading to internal
> deadlock when another free is called (correctly via zfree this time, though
> it is optimized out of the backtrace).

The queue_signals() is presumably important.  I don't see that zfree()
makes a practical difference here, though --- the backtrace suggests
malloc() isn't from zsh, in which case zfree() is surely just a simple
front end to free() that checks the pointer isn't NULL?

pws


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: zsh 5.0.6 hanged in freejob from TRAPCHLD
  2014-10-01  9:00   ` zsh 5.0.6 hanged in freejob from TRAPCHLD Peter Stephenson
@ 2014-10-01 14:53     ` Bart Schaefer
  2014-10-01 15:25       ` Oliver Kiddle
  0 siblings, 1 reply; 8+ messages in thread
From: Bart Schaefer @ 2014-10-01 14:53 UTC (permalink / raw)
  To: Zsh hackers list

On Oct 1, 10:00am, Peter Stephenson wrote:
} Subject: Re: zsh 5.0.6 hanged in freejob from TRAPCHLD
}
} > The stack trace seems to indicate that the problem likely originates in
} > lexrestore() which calls free() directly (without the signal-safe zfree()
} 
} The queue_signals() is presumably important.  I don't see that zfree()
} makes a practical difference here, though

Right, I realized after hitting "send" that zfree() is only signal-safe
when using zsh's mem.c malloc() replacement -- in which case free() is
also a wrapper for zfree() instead of the other way around.

This plus the possibility of interrupting the copy from the saved state
back to the globals is the need for queue_signals() inside lexrestore().

There may be other save/restore routines that still need this treatment,
I haven't yet done an audit.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: zsh 5.0.6 hanged in freejob from TRAPCHLD
  2014-10-01 14:53     ` Bart Schaefer
@ 2014-10-01 15:25       ` Oliver Kiddle
  2014-10-01 15:40         ` Peter Stephenson
  0 siblings, 1 reply; 8+ messages in thread
From: Oliver Kiddle @ 2014-10-01 15:25 UTC (permalink / raw)
  To: Zsh hackers list

Bart wrote:
> This plus the possibility of interrupting the copy from the saved state
> back to the globals is the need for queue_signals() inside lexrestore().

Eww. Wouldn't it perhaps be easier and better in the long run to replace
this whole save/restore/globals concept with non-global structs and make
all the lexer functions take a pointer to the relevant instance of the
struct.

Oliver


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: zsh 5.0.6 hanged in freejob from TRAPCHLD
  2014-10-01 15:25       ` Oliver Kiddle
@ 2014-10-01 15:40         ` Peter Stephenson
  2014-10-01 16:04           ` Bart Schaefer
  0 siblings, 1 reply; 8+ messages in thread
From: Peter Stephenson @ 2014-10-01 15:40 UTC (permalink / raw)
  To: Zsh hackers list

On Wed, 01 Oct 2014 17:25:33 +0200
Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
> Bart wrote:
> > This plus the possibility of interrupting the copy from the saved state
> > back to the globals is the need for queue_signals() inside lexrestore().
> 
> Eww. Wouldn't it perhaps be easier and better in the long run to replace
> this whole save/restore/globals concept with non-global structs and make
> all the lexer functions take a pointer to the relevant instance of the
> struct.

Better --- highly likely.

Easier --- only after you've persuaded someone else to do the work.
You'll soon find the paths you need to pass the structures through
proliferate somewhat hair-raisingly.  That may lead to useful
simplifications, though, so this isn't an argument against doing it.

pws


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: zsh 5.0.6 hanged in freejob from TRAPCHLD
  2014-10-01 15:40         ` Peter Stephenson
@ 2014-10-01 16:04           ` Bart Schaefer
  0 siblings, 0 replies; 8+ messages in thread
From: Bart Schaefer @ 2014-10-01 16:04 UTC (permalink / raw)
  To: Zsh hackers list

On Oct 1,  4:40pm, Peter Stephenson wrote:
} Subject: Re: zsh 5.0.6 hanged in freejob from TRAPCHLD
}
} On Wed, 01 Oct 2014 17:25:33 +0200
} Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
} > 
} > Eww. Wouldn't it perhaps be easier and better in the long run to replace
} > this whole save/restore/globals concept with non-global structs and make
} > all the lexer functions take a pointer to the relevant instance of the
} > struct.
} 
} You'll soon find the paths you need to pass the structures through
} proliferate somewhat hair-raisingly.  That may lead to useful
} simplifications, though, so this isn't an argument against doing it.

It's not even quite that simple, some of the fields require their own
memory management.  Moving it onto the call stack and passing it around
that way would only change the lexsave/lexrestore calls into something
that allocates/cleans up a local lexstack object.  (Hey, why don't we
rewrite the whole shell in C++ instead?)

It'd be a large step in the right direction to have ONE global -- a
single (struct lexstack *) -- and have lexsave/lexrestore swap that
pointer rather than copy each of the fields.  However, doesn't remove
the need for the signal queuing (see "fields require their own
memory management").

Futher, that still means finding everything that references those
globals -- some of them are used outside the lexer to examine the
current lexer state -- and replacing all those mentions with deref
through the new global pointer.  Yeah, the compiler will sort that out
if we just remove the declarations of the globals.  Busywork.

Then you get to do that for all the other things that use batches of
globals in this way (see execsave/execrestrore, trap scopes, etc.) 


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-10-01 16:04 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-30 17:21 zsh 5.0.6 hanged in freejob from TRAPCHLD Vincent Lefevre
2014-09-30 23:18 ` Bart Schaefer
2014-10-01  0:53   ` PATCH signal-safe lexrestore() [Re: zsh 5.0.6 hanged in freejob from TRAPCHLD] Bart Schaefer
2014-10-01  9:00   ` zsh 5.0.6 hanged in freejob from TRAPCHLD Peter Stephenson
2014-10-01 14:53     ` Bart Schaefer
2014-10-01 15:25       ` Oliver Kiddle
2014-10-01 15:40         ` Peter Stephenson
2014-10-01 16:04           ` Bart Schaefer

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