Cant fg a suspended su (4.1.0-dev-7)

Cant fg a suspended su (4.1.0-dev-7)

[Peter Whaite]

I've noticed that I cannot resume a suspened su session lately (It
happened in 4.1.0-dev-6 as well). 

       aragorn% /bin/su peta
       Password: 
       aragorn% echo $ZSH_VERSION
       4.1.0-dev-7
       aragorn% suspend
       zsh: suspended (signal)  /bin/su peta
       aragorn% fg %1
       [1]  + continued  /bin/su peta
       zsh: suspended (signal)  /bin/su peta

There is approximately a 1 second pause between the contined and the
suspended messages.  

Any idea why the su command re-suspends after the continue?

	  aragorn% uname -a
	  Linux aragorn 2.4.14 #1 Sat Nov 10 17:16:26 EST 2001 i686  unknown

  	  aragorn% cat /etc/issue

	  Red Hat Linux release 7.1 (Seawolf)
	  Kernel 2.4.14 on an i686

	  aragorn% setopt
	  autocd
	  noautolist
	  autonamedirs
	  autopushd
	  cdablevars
	  completeinword
	  extendedglob
	  extendedhistory
	  histignoredups
	  histignorespace
	  histnofunctions
	  histreduceblanks
	  histsavenodups
	  interactive
	  login
	  menucomplete
	  monitor
	  numericglobsort
	  nopromptcr
	  promptsubst
	  pushdignoredups
	  pushdminus
	  pushdsilent
	  shinstdin
	  zle
--
peta

Re: Cant fg a suspended su (4.1.0-dev-7)

[Bart Schaefer]

On Feb 21,  1:07pm, Peter Whaite wrote:
}
} I've noticed that I cannot resume a suspened su session lately (It
} happened in 4.1.0-dev-6 as well). 
} 
} There is approximately a 1 second pause between the contined and the
} suspended messages.  
} 
} Any idea why the su command re-suspends after the continue?

It appears to be this loop in bin_suspend():

1669	    if (jobbing) {
1670		/* stay suspended */
1671		while (gettygrp() != mypgrp) {
1672		    sleep(1);
1673		    if (gettygrp() != mypgrp)
1674			kill(0, SIGTTIN);
1675		}
1676		/* restore signal handling */
1677		signal_ignore(SIGTTOU);
1678		signal_ignore(SIGTSTP);
1679		signal_ignore(SIGTTIN);
1680	    }

However, that code has been there forever [*] so the actual problem must
lie in some change that's been made to tty process group handling.  I
would tend to suspect that zsh-workers/17859 is the culprit.

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

Zsh: http://www.zsh.org | PHPerl Project: http://phperl.sourceforge.net   

Re: Cant fg a suspended su (4.1.0-dev-7)

[Bart Schaefer]

On Feb 22, 11:04pm, Bart Schaefer wrote:
}
} However, that code has been there forever [*]

Oops, forgot my footnote:

[*] Defined as "before July 1997" which is when my CVS repository of the
zsh-3.0.x sources begins.  I'm pretty sure it's actually been around
even longer than that.

Re: Cant fg a suspended su (4.1.0-dev-7)

[Philippe Troin]

"Bart Schaefer" <schaefer@brasslantern.com> writes:

> On Feb 21,  1:07pm, Peter Whaite wrote:
> }
> } I've noticed that I cannot resume a suspened su session lately (It
> } happened in 4.1.0-dev-6 as well). 
> } 
> } There is approximately a 1 second pause between the contined and the
> } suspended messages.  
> } 
> } Any idea why the su command re-suspends after the continue?
> 
> It appears to be this loop in bin_suspend():
> 
> 1669	    if (jobbing) {
> 1670		/* stay suspended */
> 1671		while (gettygrp() != mypgrp) {
> 1672		    sleep(1);
> 1673		    if (gettygrp() != mypgrp)
> 1674			kill(0, SIGTTIN);
> 1675		}
> 1676		/* restore signal handling */
> 1677		signal_ignore(SIGTTOU);
> 1678		signal_ignore(SIGTSTP);
> 1679		signal_ignore(SIGTTIN);
> 1680	    }
> 
> However, that code has been there forever [*] so the actual problem must
> lie in some change that's been made to tty process group handling.  I
> would tend to suspect that zsh-workers/17859 is the culprit.

Since I'm the author of the patch in zsh-workers/17859, I'm having a
look at it...

The above busy loop seems quite awful to me... But I'm looking into
the problem.

Phil.

Help needed with execcmd() (was: Re: Cant fg a suspended su (4.1.0-dev-7))

[Philippe Troin]

Could somebody familiar with Src/exec.c point out for me where I can
insert a statement where the following conditions are true:

  - I can find out if we are doing a real exec(), not a "fake" exec
    (BTW, what's a "fake" exec?), following a "exec blah" command
    line. For example, in execcmd(), I can find this out with the
    do_exec variable.

  - the terminal (SHTTY) is still open

  - there will be no more terminal or process group ids changes before
    we hit zexecve() (meaning no more attachtty(), nor setpgrp())

I can't seem to find a spot where all three conditions are true.

Thanks!
Phil.

Re: Help needed with execcmd() (was: Re: Cant fg a suspended su (4.1.0-dev-7))

[Bart Schaefer]

On Feb 27, 12:47am, Philippe Troin wrote:
} Subject: Help needed with execcmd() (was: Re: Cant fg a suspended su (4.1.
}
} Could somebody familiar with Src/exec.c point out for me where I can
} insert a statement where the following conditions are true:

I'm not sure I can tell you that, but ...

}   - I can find out if we are doing a real exec(), not a "fake" exec
}     (BTW, what's a "fake" exec?)

A "fake" exec means that the current shell has state that must explicitly
be cleaned up after the external command begins, so the shell can't just
replace its process image with that of the new process.  Instead it forks,
but then [eventually] exits to simulate an exec having taken place.

As noted in one of the comments in exec.c, this is similar to the case
where the command to be exec'd is a builtin.

}     following a "exec blah" command
}     line. For example, in execcmd(), I can find this out with the
}     do_exec variable.

The shell will never do a real exec on do_exec false, but do_exec true
does not imply the opposite.  A fake exec could still occur.  If you're
in the code beyond the comment that reads

    /* Make a copy of stderr for xtrace output before redirecting */

then (is_exec == 1 && forked == 0) means it's a real exec; before the
big comment about "Do we need to fork?" the test is more complex.
 
}   - the terminal (SHTTY) is still open

This should be true any time SHTTY > -1.

}   - there will be no more terminal or process group ids changes before
}     we hit zexecve() (meaning no more attachtty(), nor setpgrp())
} 
} I can't seem to find a spot where all three conditions are true.

I'm not 100% certain, but I don't think there is a spot where all three
conditions are true, because of the side-effects of the STTY environment
variable.  There may be a spot where all three plus "STTY is not set"
are true, but it'd have to be after all the redirections have been
processed, so probably it's never true until somewhere inside the
execute() function.

Re: Cant fg a suspended su (4.1.0-dev-7)

[Philippe Troin]

Peter Whaite <peta@cortexmachina.com> writes:

> I've noticed that I cannot resume a suspened su session lately (It
> happened in 4.1.0-dev-6 as well). 
> 
>        aragorn% /bin/su peta
>        Password: 
>        aragorn% echo $ZSH_VERSION
>        4.1.0-dev-7
>        aragorn% suspend
>        zsh: suspended (signal)  /bin/su peta
>        aragorn% fg %1
>        [1]  + continued  /bin/su peta
>        zsh: suspended (signal)  /bin/su peta
> 
> There is approximately a 1 second pause between the contined and the
> suspended messages.  
> 
> Any idea why the su command re-suspends after the continue?

I could not reproduce the problem on 4.1.0-dev-7, nor on CVS...

A few questions for Peter:

 - Which su are you using? GNU su, the shadow utilities's version of
   su or something else?

   GNU su will give you the version with `su --version'

   shadow su's version can be obtained with: 
     strings /bin/su | grep '\$Package'

 - Can you send us the output of 'ps xajf' ran from within the su
   shell... You might want to trim it. I'm interested in the PGID of
   the the su'ed zsh and its parent... For me it looks like this:

    PPID   PID  PGID   SID TTY      TPGID STAT   UID   TIME COMMAND
   26785 15644 15644 26785 pts/0    15770 S     1000   0:00  \_ ../inst
   15644 15753 15753 26785 pts/0    15770 S     1002   0:00      \_ zsh
   15753 15770 15770 26785 pts/0    15770 R     1002   0:00          \_ ps

Thanks,
Phil.

Re: Cant fg a suspended su (4.1.0-dev-7)

[Peter Whaite]

Philippe Troin said:
> 
> A few questions for Peter:
> 
>  - Which su are you using? GNU su, the shadow utilities's version of
>    su or something else?

% ls -l =su
-rwsr-xr-x    1 root     root        14112 Jan 16  2001 /bin/su*
% /bin/su --version

su (GNU sh-utils) 2.0
Written by David MacKenzie.

Copyright (C) 1999 Free Software Foundation, Inc.

>  - Can you send us the output of 'ps xajf' ran from within the su
>    shell... You might want to trim it. I'm interested in the PGID of
>    the the su'ed zsh and its parent... For me it looks like this:
> 
>     PPID   PID  PGID   SID TTY      TPGID STAT   UID   TIME COMMAND
>    26785 15644 15644 26785 pts/0    15770 S     1000   0:00  \_ ../inst
>    15644 15753 15753 26785 pts/0    15770 S     1002   0:00      \_ zsh
>    15753 15770 15770 26785 pts/0    15770 R     1002   0:00          \_ ps

% /bin/su peta
Password:
% ps xajf

 PPID   PID  PGID   SID TTY      TPGID STAT   UID   TIME COMMAND
20712 20728 20728 20728 pts/2    21112 S      501   0:00  |       \_ zsh
20728 21091 21091 20728 pts/2    21112 S        0   0:00  |           \_ /bin/su
21091 21095 21095 20728 pts/2    21112 S      501   0:00  |               \_ zsh
21095 21112 21112 20728 pts/2    21112 R      501   0:00  |                   \_

and also (leftovers?)...

    1 22655 22655 21257 ?           -1 T        0   0:00 /bin/su peta
22655 22657 22657 21257 ?           -1 T      501   0:00  \_ zsh
    1 22929 22929 22689 ?           -1 T        0   0:00 /bin/su peta
22929 22933 22933 22689 ?           -1 T      501   0:00  \_ zsh
    1 23049 23049 23020 ?           -1 T        0   0:00 /bin/su peta
23049 23051 23051 23020 ?           -1 T      501   0:00  \_ zsh


> Thanks,
> Phil.

I can confirm that it doesnt always happen.  On a FreeBSD system this
version of su

$FreeBSD: src/usr.bin/su/su.c,v 1.34.2.3 2001/08/17 15:44:42 ru Exp $

works fine.

Hope this helps
--
peta

Re: Cant fg a suspended su (4.1.0-dev-7)

[Philippe Troin]

Peter Whaite <peta@cortexmachina.com> writes:

> Philippe Troin said:
> > 
> > A few questions for Peter:
> > 
> >  - Which su are you using? GNU su, the shadow utilities's version of
> >    su or something else?
> 
> % ls -l =su
> -rwsr-xr-x    1 root     root        14112 Jan 16  2001 /bin/su*
> % /bin/su --version
> 
> su (GNU sh-utils) 2.0
> Written by David MacKenzie.
> 
> Copyright (C) 1999 Free Software Foundation, Inc.

Using the shadow utilities' su here... That's why I could not
reproduce the problem.
 
> >  - Can you send us the output of 'ps xajf' ran from within the su
> >    shell... You might want to trim it. I'm interested in the PGID of
> >    the the su'ed zsh and its parent... For me it looks like this:

8< snip >8

> 
> % /bin/su peta
> Password:
> % ps xajf
> 
>  PPID   PID  PGID   SID TTY      TPGID STAT   UID   TIME COMMAND
> 20712 20728 20728 20728 pts/2    21112 S      501   0:00  |       \_ zsh
> 20728 21091 21091 20728 pts/2    21112 S        0   0:00  |           \_ /bin/su
> 21091 21095 21095 20728 pts/2    21112 S      501   0:00  |               \_ zsh
> 21095 21112 21112 20728 pts/2    21112 R      501   0:00  |                   \_

Now I know what the problem is... GNU su does not exec() the sub-shell
and hangs around. The zsh-workers/17859 patch (which I'm responsible
for) explicitly makes sure zsh is its own process group leader for
interactive sessions. This cures quite a few problems but introduced
this bug.

Before zsh-workers/17859 was applied:

  the upper zsh would have had its own process group

  both su and the lower zsh share their process group

  Pros: suspend (as coded currently) works

  Cons: terminal-related signals get sent to both su and zsh. Su must
  contain special logic to ignore these signals, otherwise a (eg.)
  CTRL-C will kill the su process leaving the lower zsh completely
  disconnected from the upper zsh, which leads to chaos.

After zsh-workers/17859 was applied:

  the upper zsh would have had its own process group

  su has its own process group

  at startup, zsh creates its own process group

  Pros: no need to add some special logic to su (or any other
  application)

  Cons: suspend (as currently coded) does not work.

> and also (leftovers?)...

8< snip >8

Not needed.
 
> I can confirm that it doesnt always happen.  On a FreeBSD system this
> version of su
> 
> $FreeBSD: src/usr.bin/su/su.c,v 1.34.2.3 2001/08/17 15:44:42 ru Exp $
> 
> works fine.

Probably because FreeBSD's su does exec() on $SHELL (shadow su's
behavior) rather than fork()+exec() (GNU su behavior).
 
> Hope this helps

It did.

I'll check out how the other shells (bash and tcsh) handle this case
and will post a patch for bin_suspend() later.

Phil.

Re: Cant fg a suspended su (4.1.0-dev-7)

[Philippe Troin]

Philippe Troin <phil@fifi.org> writes:

> I'll check out how the other shells (bash and tcsh) handle this case
> and will post a patch for bin_suspend() later.

I was looking at bash, and they do not do anything special (like
reverting to the original pgrp) before suspending.

Can you try su'ing with bash? I was able to get it stuck with my local
test program here... but I do not have GNU su installed (yet).

Thanks,
Phil.

Re: Cant fg a suspended su (4.1.0-dev-7)

[Peter Whaite]

Philippe Troin said:
> Philippe Troin <phil@fifi.org> writes:
> 
> > I'll check out how the other shells (bash and tcsh) handle this case
> > and will post a patch for bin_suspend() later.
> 
> I was looking at bash, and they do not do anything special (like
> reverting to the original pgrp) before suspending.
> 
> Can you try su'ing with bash? I was able to get it stuck with my local
> test program here... but I do not have GNU su installed (yet).

Looks like bash can deal with GNU su.  So can tcsh. 

% su --shell=/bin/bash peta
Password: 
[peta@aragorn zsh]$ suspend
zsh: suspended (signal)  /bin/su --shell=/bin/bash peta
1 % fg
[1]  + continued  /bin/su --shell=/bin/bash peta

11779 11790 11790 11790 pts/2    12959 S      501   0:00      \_ -zsh
11790 12894 12894 11790 pts/2    12959 S        0   0:00          \_ /bin/su --shell=/bin/bash peta
12894 12903 12903 11790 pts/2    12959 S      501   0:00              \_ bash
12903 12959 12959 11790 pts/2    12959 R      501   0:00                  \_ ps xajfw


--
Peter Whaite
VP Research and Development, Cynovad - ShadeScan Division.
460 ste-Catherine O, bureau 942, Montreal, Quebec. H3B 1A7
Phone: (514) 395 3905 (#204)  Fax:   (514) 878 2874

Re: Cant fg a suspended su (4.1.0-dev-7)

[Philippe Troin]

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

Peter Whaite <peta@cortexmachina.com> writes:

> Philippe Troin said:
> > Philippe Troin <phil@fifi.org> writes:
> > 
> > > I'll check out how the other shells (bash and tcsh) handle this case
> > > and will post a patch for bin_suspend() later.
> > 
> > I was looking at bash, and they do not do anything special (like
> > reverting to the original pgrp) before suspending.
> > 
> > Can you try su'ing with bash? I was able to get it stuck with my local
> > test program here... but I do not have GNU su installed (yet).
> 
> Looks like bash can deal with GNU su.  So can tcsh. 
> 
> % su --shell=/bin/bash peta
> Password: 
> [peta@aragorn zsh]$ suspend
> zsh: suspended (signal)  /bin/su --shell=/bin/bash peta
> 1 % fg
> [1]  + continued  /bin/su --shell=/bin/bash peta
> 
> 11779 11790 11790 11790 pts/2    12959 S      501   0:00      \_ -zsh
> 11790 12894 12894 11790 pts/2    12959 S        0   0:00          \_ /bin/su --shell=/bin/bash peta
> 12894 12903 12903 11790 pts/2    12959 S      501   0:00              \_ bash
> 12903 12959 12959 11790 pts/2    12959 R      501   0:00                  \_ ps xajfw

Mrm, that's not GNU su. Or it's a very heavily patched version of GNU
su. I've check shellutils 2.0 and it does a straight exec(), and does
not leave a parent process like seen above...

So I cannot reproduce your exact problem, but I think I've fixed it
nevertheless... Can you try the enclosed patch? It's against the
latest CVS, but it applies with a little bit of fuzz to 4.1.0-dev-7.

Here is a transcript of a session which shows that it works. "spawn"
is a very simple program that does what I believe your version of su
does. Replace it with "su --shell=/path/to/zsh".

    phil@ceramic:~/y% ./spawn zsh/Src/zsh
    phil@ceramic:~/y[2]% suspend

    zsh: 13283 suspended  ./spawn zsh/Src/zsh
    phil@ceramic:~/y% bg
    [1]  + continued  ./spawn zsh/Src/zsh
    phil@ceramic:~/y% 
    [1]  + 13283 suspended (tty input)  ./spawn zsh/Src/zsh
    phil@ceramic:~/y% fg
    [1]  + continued  ./spawn zsh/Src/zsh
    phil@ceramic:~/y[2]% exec zsh/Src/zsh                         
    phil@ceramic:~/y[2]% suspend

    zsh: 13283 suspended  ./spawn zsh/Src/zsh
    zsh: exit 20
    phil@ceramic:~/y% bg
    [1]  + continued  ./spawn zsh/Src/zsh
    phil@ceramic:~/y% 
    [1]  + 13283 suspended (tty input)  ./spawn zsh/Src/zsh
    phil@ceramic:~/y% fg
    [1]  + continued  ./spawn zsh/Src/zsh
    phil@ceramic:~/y[2]% exit
    child exited with status 0
    phil@ceramic:~/y% 

You'll notice that:

 - suspend now works.

 - suspend STILL works across exec (nice touch)

 - bg'ing or resuming reports "suspended (tty input)" correctly,
   without the 1 second delay anymore.

Technical explanation:

 - zsh-workers/17859 makes the case why we always want to have
   interactive zsh instances to be the leader of their process group,
   so I won't come back to this.

 - zsh-workers/17859 created a new process group if zsh was started
   interactively without being a process group leader. This happens
   frequently we have have this typical process hierarchy:

      zsh
       \_ program running system("zsh") (eg. mail, dpkg, your su)
          \_ zsh

The fix consists in the following:

 - when the user invokes "suspend", and we created our own process
   group, we must go back to the process group we came from and send a
   SIGTSTP to the whole process group. When the process is resumed, it
   will reacquire its process group leader status.

 - also, when we run "exec somecommand", we must restore the original
   process group.

Patch walk-through:

 - introduced a new global variable, origpgrp, containing the initial
   process group. It is only set in init_io().

 - moved the process group acquiring and terminal acquiring code from
   init_io() to a new function, acquire_pgrp() in jobs.c. init_io()
   now calls acquire_pgrp().

 - created a new function in jobs.c, release_pgrp() that gives the
   terminal foreground process group back to origprgp and moves back
   zsh to this process group.

 - in bin_suspend(): before suspending, call release_pgrp(). Use
   killpg() to broadcast the signal to the whole process group, rather
   than just kill()ing ourselves (note: in most cases it's the same
   since zsh is alone in its process group, however in the above case
   where zsh is spawned by another process, the spawning process must
   also receive SIGTSTP, just as if CTRL-Z had been pressed). After
   zsh is resumed call acquire_pgrp() again which will (maybe) move
   back zsh to its own process group, and handle the terminal in the
   proper way (rather than playing tricks with sleep()).

 - in exec.c: we want to be sure that if zsh has moved to its own
   process group, it moves back to its original process group before
   exec'ing a final command. I had to add an extra parameter to
   entersubsh() indicating whether or not reverting to an eventual
   origpgrp is needed. It is always zero, except when called from
   execsubst().

   Note to Bart: at the stage where entersubsh() is called from
   execsubst(), the condition seems to be:

     (do_exec || (type >= WC_CURSH && last1 == 1)) && !forked

   You said that do_exec && !forked was sufficient, but do_exec is set
   to 1 two lines down if (type >= WC_CURSH && last1 == 1).

   Also, inside entersubsh(), where release_pgrp() is called, I have
   to check that (getpid() == mypgrp) in addition to revertpgrp being
   true. Otherwise some temporary zsh subprocesses (like the ones
   created during a pipe before they exec the actual executables) will
   wrongly revert their process group.

I've also enclosed the trivial "spawn" program, in case you don't have
the RH shellutils. And you might want to try it with other shells. I
believe zsh is the only shell that implements terminal handling
properly now: bash fails to suspend correctly with spawn, and tcsh
gets lost after it execs itself.

Peter, can you confirm that this patch makes zsh perform as expected
with your su?

If yes, then Bart, would you consider it for inclusion?

Phil.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: zsh-suspend-exec-fix.patch --]
[-- Type: text/x-patch, Size: 6713 bytes --]

Index: Src/exec.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
retrieving revision 1.48
diff -b -u -r1.48 exec.c
--- Src/exec.c	18 Dec 2002 16:57:02 -0000	1.48
+++ Src/exec.c	6 Mar 2003 03:52:57 -0000
@@ -1149,7 +1149,7 @@
 		    }
 		    else {
 			close(synch[0]);
-			entersubsh(Z_ASYNC, 0, 0);
+			entersubsh(Z_ASYNC, 0, 0, 0);
 			if (jobtab[list_pipe_job].procs) {
 			    if (setpgrp(0L, mypgrp = jobtab[list_pipe_job].gleader)
 				== -1) {
@@ -1258,7 +1258,7 @@
 	    } else {
 		zclose(pipes[0]);
 		close(synch[0]);
-		entersubsh(how, 2, 0);
+		entersubsh(how, 2, 0, 0);
 		close(synch[1]);
 		execcmd(state, input, pipes[1], how, 0);
 		_exit(lastval);
@@ -2060,7 +2060,7 @@
 	}
 	/* pid == 0 */
 	close(synch[0]);
-	entersubsh(how, (type != WC_SUBSH) && !(how & Z_ASYNC) ? 2 : 1, 0);
+	entersubsh(how, (type != WC_SUBSH) && !(how & Z_ASYNC) ? 2 : 1, 0, 0);
 	close(synch[1]);
 	forked = 1;
 	if (sigtrapped[SIGINT] & ZSIG_IGNORED)
@@ -2277,7 +2277,9 @@
 	 * exit) in case there is an error return.
 	 */
 	if (is_exec)
-	    entersubsh(how, (type != WC_SUBSH) ? 2 : 1, 1);
+	    entersubsh(how, (type != WC_SUBSH) ? 2 : 1, 1,
+		       (do_exec || (type >= WC_CURSH && last1 == 1)) 
+		       && !forked);
 	if (type >= WC_CURSH) {
 	    if (last1 == 1)
 		do_exec = 1;
@@ -2536,7 +2538,7 @@
 
 /**/
 static void
-entersubsh(int how, int cl, int fake)
+entersubsh(int how, int cl, int fake, int revertpgrp)
 {
     int sig, monitor;
 
@@ -2580,6 +2582,8 @@
     }
     if (!fake)
 	subsh = 1;
+    if (revertpgrp && getpid() == mypgrp)
+	release_pgrp();
     if (SHTTY != -1) {
 	shout = NULL;
 	zclose(SHTTY);
@@ -2769,7 +2773,7 @@
     zclose(pipes[0]);
     redup(pipes[1], 1);
     opts[MONITOR] = 0;
-    entersubsh(Z_SYNC, 1, 0);
+    entersubsh(Z_SYNC, 1, 0, 0);
     cmdpush(CS_CMDSUBST);
     execode(prog, 0, 1);
     cmdpop();
@@ -2900,7 +2904,7 @@
     /* pid == 0 */
     redup(fd, 1);
     opts[MONITOR] = 0;
-    entersubsh(Z_SYNC, 1, 0);
+    entersubsh(Z_SYNC, 1, 0, 0);
     cmdpush(CS_CMDSUBST);
     execode(prog, 0, 1);
     cmdpop();
@@ -2980,10 +2984,10 @@
 	zerr("can't open %s: %e", pnam, errno);
 	_exit(1);
     }
-    entersubsh(Z_ASYNC, 1, 0);
+    entersubsh(Z_ASYNC, 1, 0, 0);
     redup(fd, out);
 #else
-    entersubsh(Z_ASYNC, 1, 0);
+    entersubsh(Z_ASYNC, 1, 0, 0);
     redup(pipes[out], out);
     closem(0);   /* this closes pipes[!out] as well */
 #endif
@@ -3012,7 +3016,7 @@
 	zclose(pipes[out]);
 	return pipes[!out];
     }
-    entersubsh(Z_ASYNC, 1, 0);
+    entersubsh(Z_ASYNC, 1, 0, 0);
     redup(pipes[out], out);
     closem(0);	/* this closes pipes[!out] as well */
     cmdpush(CS_CMDSUBST);
Index: Src/init.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/init.c,v
retrieving revision 1.30
diff -b -u -r1.30 init.c
--- Src/init.c	27 Jan 2003 16:38:09 -0000	1.30
+++ Src/init.c	6 Mar 2003 03:52:57 -0000
@@ -354,7 +354,6 @@
 mod_export void
 init_io(void)
 {
-    long ttpgrp;
     static char outbuf[BUFSIZ], errbuf[BUFSIZ];
 
 #ifdef RSH_BUG_WORKAROUND
@@ -462,37 +461,8 @@
      */
     mypid = (zlong)getpid();
     if (opts[MONITOR] && interact && (SHTTY != -1)) {
-	if ((mypgrp = GETPGRP()) > 0) {
-	    sigset_t blockset, oldset;
-	    sigemptyset(&blockset);
-	    sigaddset(&blockset, SIGTTIN);
-	    sigaddset(&blockset, SIGTTOU);
-	    sigaddset(&blockset, SIGTSTP);
-	    oldset = signal_block(blockset);
-	    while ((ttpgrp = gettygrp()) != -1 && ttpgrp != mypgrp) {
-		mypgrp = GETPGRP();
-		if (mypgrp == mypid) {
-		    signal_setmask(oldset);
-		    attachtty(mypgrp); /* Might generate SIGT* */
-		    signal_block(blockset);
-		}
-		if (mypgrp == gettygrp())
-		    break;
-		signal_setmask(oldset);
-		read(0, NULL, 0); /* Might generate SIGT* */
-		signal_block(blockset);
-		mypgrp = GETPGRP();
-	    }
-	    if (mypgrp != mypid) {
-	        if (setpgrp(0, 0) == 0) {
-		    mypgrp = mypid;
-		    attachtty(mypgrp);
-                } else
-		    opts[MONITOR] = 0;
-	    }
-	    signal_setmask(oldset);
-	} else
-	    opts[MONITOR] = 0;
+	origpgrp = GETPGRP();
+        acquire_pgrp(); /* might also clear opts[MONITOR] */
     } else
 	opts[MONITOR] = 0;
 #else
Index: Src/jobs.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/jobs.c,v
retrieving revision 1.19
diff -b -u -r1.19 jobs.c
--- Src/jobs.c	21 Feb 2003 14:37:03 -0000	1.19
+++ Src/jobs.c	6 Mar 2003 03:52:58 -0000
@@ -30,6 +30,12 @@
 #include "zsh.mdh"
 #include "jobs.pro"
 
+/* the process group of the shell at startup (equal to mypgprp, except
+   when we started without being process group leader */
+
+/**/
+mod_export pid_t origpgrp;
+
 /* the process group of the shell */
 
 /**/
@@ -1663,16 +1669,16 @@
 	signal_default(SIGTTIN);
 	signal_default(SIGTSTP);
 	signal_default(SIGTTOU);
+
+	/* Move ourselves back to the process group we came from */
+	release_pgrp();
     }
+
     /* suspend ourselves with a SIGTSTP */
-    kill(0, SIGTSTP);
+    killpg(origpgrp, SIGTSTP);
+
     if (jobbing) {
-	/* stay suspended */
-	while (gettygrp() != mypgrp) {
-	    sleep(1);
-	    if (gettygrp() != mypgrp)
-		kill(0, SIGTTIN);
-	}
+	acquire_pgrp();
 	/* restore signal handling */
 	signal_ignore(SIGTTOU);
 	signal_ignore(SIGTSTP);
@@ -1695,4 +1701,60 @@
 	    jobtab[jobnum].procs->text && strpfx(s, jobtab[jobnum].procs->text))
 	    return jobnum;
     return -1;
+}
+
+
+/* make sure we are a process group leader by creating a new process
+   group if necessary */
+
+/**/
+void
+acquire_pgrp(void)
+{
+    long ttpgrp;
+    sigset_t blockset, oldset;
+
+    if ((mypgrp = GETPGRP()) > 0) {
+	sigemptyset(&blockset);
+	sigaddset(&blockset, SIGTTIN);
+	sigaddset(&blockset, SIGTTOU);
+	sigaddset(&blockset, SIGTSTP);
+	oldset = signal_block(blockset);
+	while ((ttpgrp = gettygrp()) != -1 && ttpgrp != mypgrp) {
+	    mypgrp = GETPGRP();
+	    if (mypgrp == mypid) {
+		signal_setmask(oldset);
+		attachtty(mypgrp); /* Might generate SIGT* */
+		signal_block(blockset);
+	    }
+	    if (mypgrp == gettygrp())
+		break;
+	    signal_setmask(oldset);
+	    read(0, NULL, 0); /* Might generate SIGT* */
+	    signal_block(blockset);
+	    mypgrp = GETPGRP();
+	}
+	if (mypgrp != mypid) {
+	    if (setpgrp(0, 0) == 0) {
+		mypgrp = mypid;
+		attachtty(mypgrp);
+	    } else
+		opts[MONITOR] = 0;
+	}
+	signal_setmask(oldset);
+    } else
+	opts[MONITOR] = 0;
+}
+
+/* revert back to the process group we came from (before acquire_pgrp) */
+
+/**/
+void
+release_pgrp(void)
+{
+    if (origpgrp != mypgrp) {
+	attachtty(origpgrp);
+	setpgrp(0, origpgrp);
+	mypgrp = origpgrp;
+    }
 }

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: spawn.c --]
[-- Type: text/x-csrc, Size: 457 bytes --]

   #include <unistd.h>
   #include <stdio.h>
   #include <sys/wait.h>
   #include <sys/types.h>

   int main(int argc, char *argv[])
   {
     int status;
     pid_t child = fork();
     if (child<0)
       perror("fork");
     if (child == 0)
       {
         if (execvp(argv[1], &argv[1])<0)
           perror("execvp");
         exit(1);
       }
     waitpid(child, &status, 0);
     printf("child exited with status %x\n", status);
     exit(0);
   }

Re: Cant fg a suspended su (4.1.0-dev-7)

[Peter Whaite]

Philippe Troin said:
> 
> Mrm, that's not GNU su. Or it's a very heavily patched version of GNU
> su. I've check shellutils 2.0 and it does a straight exec(), and does
> not leave a parent process like seen above...

Well thats strange.  We have (RedHat 7.1)

% rpm -q -f /bin/su
sh-utils-2.0-13
% rpm --verify -f /bin/su
%

On a RedHat 7.2 system we have sh-utils-2.0.11-5 and su doesnt exec
itself away there either.

> Peter, can you confirm that this patch makes zsh perform as expected
> with your su?

Yes it does work.
--
peta

Re: Cant fg a suspended su (4.1.0-dev-7)

[Philippe Troin]

Peter Whaite <peta@cortexmachina.com> writes:

> Philippe Troin said:
> > 
> > Mrm, that's not GNU su. Or it's a very heavily patched version of GNU
> > su. I've check shellutils 2.0 and it does a straight exec(), and does
> > not leave a parent process like seen above...
> 
> Well thats strange.  We have (RedHat 7.1)
> 
> % rpm -q -f /bin/su
> sh-utils-2.0-13
> % rpm --verify -f /bin/su
> %

Not really strange: shellutils (and GNU su) do not support PAM. Redhat
probably PAMified su and leaves a master process around to implement
the PAM session logout.

> On a RedHat 7.2 system we have sh-utils-2.0.11-5 and su doesnt exec
> itself away there either.
> 
> > Peter, can you confirm that this patch makes zsh perform as expected
> > with your su?
> 
> Yes it does work.

I assume you've tested both suspend and exec?

Bart, can you check-in the patch in CVS?

Thanks,
Phil.

Re: Cant fg a suspended su (4.1.0-dev-7)

[Peter Stephenson]

Peter Whaite wrote:
> Philippe Troin said:
> > Peter, can you confirm that this patch makes zsh perform as expected
> > with your su?
> 
> Yes it does work.

I've committed this.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 692070


**********************************************************************
The information transmitted is intended only for the person or
entity to which it is addressed and may contain confidential 
and/or privileged material. 
Any review, retransmission, dissemination or other use of, or
taking of any action in reliance upon, this information by 
persons or entities other than the intended recipient is 
prohibited.  
If you received this in error, please contact the sender and 
delete the material from any computer.
**********************************************************************

Re: Cant fg a suspended su (4.1.0-dev-7)

[Philippe Troin]

Peter Stephenson <pws@csr.com> writes:

> Peter Whaite wrote:
> > Philippe Troin said:
> > > Peter, can you confirm that this patch makes zsh perform as expected
> > > with your su?
> > 
> > Yes it does work.
> 
> I've committed this.

Thanks!

Phil.

Re: Cant fg a suspended su (4.1.0-dev-7)

[Philippe Troin]

Peter Whaite <peta@cortexmachina.com> writes:

> I've noticed that I cannot resume a suspened su session lately (It
> happened in 4.1.0-dev-6 as well). 
> 
>        aragorn% /bin/su peta
>        Password: 
>        aragorn% echo $ZSH_VERSION
>        4.1.0-dev-7
>        aragorn% suspend
>        zsh: suspended (signal)  /bin/su peta
>        aragorn% fg %1
>        [1]  + continued  /bin/su peta
>        zsh: suspended (signal)  /bin/su peta
> 
> There is approximately a 1 second pause between the contined and the
> suspended messages.  

Okie, I have the fix, but I need more time to finish testing
it... While I was perusing bash's sources, I found out that we could
also improve pgid handling when doing builtin_exec(). On the way
too...

Phil.

Re: Cant fg a suspended su (4.1.0-dev-7)

[Bart Schaefer]

On Feb 25, 10:18pm, Philippe Troin wrote:
}
} While I was perusing bash's sources, I found out that we could
} also improve pgid handling when doing builtin_exec(). On the way
} too...

Is anybody else a bit leery of actually copying code from the bash
sources?

Zsh is not GPL'd and I don't want it to become so.

Re: Cant fg a suspended su (4.1.0-dev-7)

[Peter Stephenson]

"Bart Schaefer" wrote:
> Is anybody else a bit leery of actually copying code from the bash
> sources?
> 
> Zsh is not GPL'd and I don't want it to become so.

Yes, we certainly can't copy code from bash.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 692070


**********************************************************************
The information transmitted is intended only for the person or
entity to which it is addressed and may contain confidential 
and/or privileged material. 
Any review, retransmission, dissemination or other use of, or
taking of any action in reliance upon, this information by 
persons or entities other than the intended recipient is 
prohibited.  
If you received this in error, please contact the sender and 
delete the material from any computer.
**********************************************************************

Re: Cant fg a suspended su (4.1.0-dev-7)

[Philippe Troin]

"Bart Schaefer" <schaefer@brasslantern.com> writes:

> On Feb 25, 10:18pm, Philippe Troin wrote:
> }
> } While I was perusing bash's sources, I found out that we could
> } also improve pgid handling when doing builtin_exec(). On the way
> } too...
> 
> Is anybody else a bit leery of actually copying code from the bash
> sources?
> 
> Zsh is not GPL'd and I don't want it to become so.

Who said I was copying code from bash? I just mentionned I was looking
how bash was behaving and how its behavior was implemented. By doing
this I found out that we did one thing wrong w.r.t. exec. That's all.

BTW, I found out that bash has a couple of problems when dealing with
the same case. I obviously do not want zsh to have the same problems.

I don't believe that checking out how a given feature is implemented
in a GPL'ed program counts as "copying code" or "infecting zsh with
GPL" or whatever.

Tell me if I'm wrong.

Phil.