Re: more on pipe bug

Re: more on pipe bug

[whukriede]

Hi!

> I've made some progress.  The simplest form required to show the bug
> is something like:

> % if true; then echo | more; fi

The bug seems to have been in 2.5.03 already:

    zsh-2.5.03> if true ; then cat some-file | more ; fi
      ^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C
    ^Z^Z^Z
    
    
    [ ... nothing happens ...]

    [then after killing the (btw busy!) more-process:]
    
    zsh: 1060 interrupt   cat some-file |
    zsh: 1061 terminated  more
    zsh-2.5.03>

So long, Wolfgang.

more on pipe bug

[Peter Stephenson]

I've made some progress.  The simplest form required to show the bug
is something like:

% if true; then echo | more; fi

Any shell construct will do, with any builtin piped to anything that
uses the terminal.  (You only need a builtin on the left of the pipe
because it finishes faster.)

It seems to localise to what's going on in entersubsh() in exec.c, in
particular the code calling setpgrp() and attachtty().  With no outer
construct, i.e. `echo | more' on its own, the first chunk with the
list_pipe_job stuff (just before the first chunk in the patch below)
comes into play.  With the construct, it doesn't (I haven't checked
the logic why not), and since the `echo' job was forked first and
becomes the process group leader, the last alternative, a simple
setpgprp(), is chosen.  This falls over if the echo process already
finished.

So a simple fix (below) is just to check whether the setpgrp() failed
and in that case make the process a new job leader.  This fixes the
bug as described and is certainly an improvement on the current code,
since if setpgrp() does fail, this is the right thing to do about it.

*However* I don't understand the list_pipe stuff and it's quite
possible there is a bug there, in other words the first chunk should
be coming into effect here as with the plain `echo | more'.  It's
there to keep track of pipelines in the current shell, which this
certainly is.

Anyway, here's the simple fix.

*** Src/exec.c.pgrp	Thu Aug 15 12:38:56 1996
--- Src/exec.c	Thu Sep 19 11:45:24 1996
***************
*** 1872,1878 ****
  		    attachtty(jobtab[thisjob].gleader);
  	    }
  	}
! 	else if (!jobtab[thisjob].gleader) {
  	    jobtab[thisjob].gleader = getpid();
  	    if (list_pipe_job != thisjob &&
  		!jobtab[list_pipe_job].gleader)
--- 1872,1879 ----
  		    attachtty(jobtab[thisjob].gleader);
  	    }
  	}
! 	else if (!jobtab[thisjob].gleader ||
! 		 (setpgrp(0L, jobtab[thisjob].gleader) == -1)) {
  	    jobtab[thisjob].gleader = getpid();
  	    if (list_pipe_job != thisjob &&
  		!jobtab[list_pipe_job].gleader)
***************
*** 1880,1887 ****
  	    setpgrp(0L, jobtab[thisjob].gleader);
  	    if (how & Z_SYNC)
  		attachtty(jobtab[thisjob].gleader);
! 	} else
! 	    setpgrp(0L, jobtab[thisjob].gleader);
      }
      if (!fake)
  	subsh = 1;
--- 1881,1887 ----
  	    setpgrp(0L, jobtab[thisjob].gleader);
  	    if (how & Z_SYNC)
  		attachtty(jobtab[thisjob].gleader);
! 	}
      }
      if (!fake)
  	subsh = 1;

-- 
Peter Stephenson <pws@ifh.de>       Tel: +49 33762 77366
WWW:  http://www.ifh.de/~pws/       Fax: +49 33762 77330
Deutches Electronen-Synchrotron --- Institut fuer Hochenergiephysik Zeuthen
DESY-IfH, 15735 Zeuthen, Germany.