zsh-3.0.3: another problem in Src/jobs.c

zsh-3.0.3: another problem in Src/jobs.c

[SUZUKI Hisao]

Jo napot kivanok!
(...or "Jo reggelt kivanok")

	I reported a problem of zsh-3.0.3/Src/jobs.c by e-mail
<9706180710.AA10798@oz.fantasy.otsl.oki.co.jp> yesterday.  Now I
found another problem.  It is like this:

	ares:~% stty
	speed 9600 baud; line = 0;
	intr = ^G; erase = ^H;
	-brkint ixoff -imaxbel
	-iexten -echoctl
	ares:~% emacs -nw &
	[1] 2537
	ares:~% 
	[1]  + suspended (tty output)  emacs -nw
	ares:~% fg
	[1]  + continued  emacs -nw
	
	zsh: suspended  emacs -nw
	ares:~% stty
	speed 9600 baud; line = 0;
	intr = ^G; quit = <undef>; erase = ^H; susp = <undef>; lnext = <undef>;
	flush = <undef>;
	-brkint inlcr ixoff -imaxbel
	-iexten -echoctl

This is similar to what I reported yesterday in that the tty
setting is modified unexpectedly by a background job.  Really
they are the same _annoying_ problem of the same cause in a
sense.
	Again, for the time being, I made a hack to fix it
rather in a clean way (at least I think so :-) this time.  I
hope the problem will be fixed soon in any way.

----------------------------------------------------------------
*** zsh-3.0.3/Src/jobs.c.ORIG	Tue Jun  3 05:11:27 1997
--- zsh-3.0.3/Src/jobs.c	Thu Jun 19 02:35:37 1997
***************
*** 121,135 ****
      int job;
      int val = 0, status = 0;
      int somestopped = 0, inforeground = 0;
  
      for (pn = jn->procs; pn; pn = pn->next) {
  	if (pn->status == SP_RUNNING)      /* some processes in this job are running       */
  	    return;                        /* so no need to update job table entry         */
  	if (WIFSTOPPED(pn->status))        /* some processes are stopped                   */
  	    somestopped = 1;               /* so job is not done, but entry needs updating */
! 	if (!pn->next)                     /* last job in pipeline determines exit status  */
  	    val = (WIFSIGNALED(pn->status)) ? 0200 | WTERMSIG(pn->status) :
  		WEXITSTATUS(pn->status);
  	if (pn->pid == jn->gleader)        /* if this process is process group leader      */
  	    status = pn->status;
      }
--- 121,140 ----
      int job;
      int val = 0, status = 0;
      int somestopped = 0, inforeground = 0;
+     pid_t pgrp = -1;
+     int is_foreground = 0;	/* is this job in the foreground actually? */
+     int is_stopped = 0;		/* is this job stopped? */
  
      for (pn = jn->procs; pn; pn = pn->next) {
  	if (pn->status == SP_RUNNING)      /* some processes in this job are running       */
  	    return;                        /* so no need to update job table entry         */
  	if (WIFSTOPPED(pn->status))        /* some processes are stopped                   */
  	    somestopped = 1;               /* so job is not done, but entry needs updating */
! 	if (!pn->next) {  /* last job in pipeline determines exit status  */
  	    val = (WIFSIGNALED(pn->status)) ? 0200 | WTERMSIG(pn->status) :
  		WEXITSTATUS(pn->status);
+ 	    is_stopped = WIFSTOPPED(pn->status);
+ 	}
  	if (pn->pid == jn->gleader)        /* if this process is process group leader      */
  	    status = pn->status;
      }
***************
*** 154,168 ****
  	}
      }
  
!     if (shout && !ttyfrozen && !jn->stty_in_env)
! 	gettyinfo(&shttyinfo);
! 
      if (isset(MONITOR)) {
! 	pid_t pgrp = gettygrp();           /* get process group of tty      */
! 
  	/* is this job in the foreground of an interactive shell? */
! 	if (mypgrp != pgrp && inforeground &&
! 	    (jn->gleader == pgrp || (pgrp > 1 && kill(-pgrp, 0) == -1))) {
  	    attachtty(mypgrp);
  	    adjustwinsize();   /* check window size and adjust if necessary */
  	}
--- 159,179 ----
  	}
      }
  
!     if (shout && !ttyfrozen && !jn->stty_in_env && !is_stopped) {
! 	pgrp = gettygrp();	/* get process group of tty */
! 	is_foreground = (jn->gleader == pgrp ||
! 			 (pgrp > 1 && kill(-pgrp, 0) == -1));
! 	if (is_foreground)
! 	    gettyinfo(&shttyinfo);
!     }
      if (isset(MONITOR)) {
! 	if (pgrp == -1) {
! 	    pgrp = gettygrp();
! 	    is_foreground = (jn->gleader == pgrp ||
! 			     (pgrp > 1 && kill(-pgrp, 0) == -1));
! 	}
  	/* is this job in the foreground of an interactive shell? */
! 	if (mypgrp != pgrp && inforeground && is_foreground) {
  	    attachtty(mypgrp);
  	    adjustwinsize();   /* check window size and adjust if necessary */
  	}
----------------------------------------------------------------

Regards,

Hisao Suzuki <suzuki@otsl.oki.co.jp>

Re: zsh-3.0.3: another problem in Src/jobs.c

[Zoltan Hidvegi]

> Jo napot kivanok!
> (...or "Jo reggelt kivanok")

Just for those, who do not speak Hungarian, these are just greetings
meaning good day and good morning :-).  Where did you learn these (I'm
just curious)?

> 	I reported a problem of zsh-3.0.3/Src/jobs.c by e-mail
> <9706180710.AA10798@oz.fantasy.otsl.oki.co.jp> yesterday.  Now I
> found another problem.  It is like this:

It seems that it is really a problem with your patch.  You perhaps missed
Peter's one-liner fix for the problem (you have to remove your patched before
that applies.

And I should apologize for the delay of zsh-3.0.4.  I have almost made a
release but then came two new fixes from Peter az Zefram.

Zoltan


Here is Peter's patch:

*** ./Src/jobs.c.old	Wed Jun  4 17:51:43 1997
--- ./Src/jobs.c	Wed Jun  4 17:53:39 1997
***************
*** 155,161 ****
  	}
      }
  
!     if (shout && !ttyfrozen && !jn->stty_in_env)
  	gettyinfo(&shttyinfo);
  
      if (isset(MONITOR)) {
--- 155,161 ----
  	}
      }
  
!     if (shout && !ttyfrozen && !jn->stty_in_env && inforeground)
  	gettyinfo(&shttyinfo);
  
      if (isset(MONITOR)) {

Re: zsh-3.0.3: another problem in Src/jobs.c

[Andrei Tcherepanov]

Hi!

Seems this (Peter's ) patch not fix 100% problem, becouse after
applying it the same problem (stty and bakground jobs) still
happed rundomly or   after working some time;
I cannot catch the moment where my stty goes bad.
( all talks about 3.1.2 )

BTW. IMHO
This happed becouse from 3.1.1 to 3.1.2 was long time of programming
and not 100 users testing. may be need to produce alphas-betas 
more often with some (not 100) bug fix in it ? 


Zoltan Hidvegi wrote:
[snip]
> >       I reported a problem of zsh-3.0.3/Src/jobs.c by e-mail
> > <9706180710.AA10798@oz.fantasy.otsl.oki.co.jp> yesterday.  Now I
> > found another problem.  It is like this:
> 
> It seems that it is really a problem with your patch.  You perhaps missed
> Peter's one-liner fix for the problem (you have to remove your patched before
> that applies.
> 
> And I should apologize for the delay of zsh-3.0.4.  I have almost made a
> release but then came two new fixes from Peter az Zefram.
> 
> Zoltan
> 
> Here is Peter's patch:
[patch about inforeground removed]
-- 
Thanks,
  Andrei ( tandr@ptc.com )

Patch for zle_refresh.c

[Geoff Wing]

On 19 Jun 1997 17:09:00 +1000, Zoltan Hidvegi <hzoli@ny.frontiercomm.net> wrote:
:And I should apologize for the delay of zsh-3.0.4.  I have almost made a
:release but then came two new fixes from Peter az Zefram.

Well, I better give you this patch then.  It's against 3.0.3 (I think).

Note: this is a fix to avoid a coredump - it's a kludge because I haven't 
looked into the reasons behind it yet so I don't know if something else is
buggy or just this.  Also, a couple of people - Andrej Borsenkow & Zefram -
have indicated another problem with the display which I haven't got round to
investigating properly yet; I can't reproduce it from their methods.


*** zle_refresh.c.org	Tue Jun  3 15:11:25 1997
--- zle_refresh.c	Thu Jun 19 18:13:46 1997
***************
*** 758,763 ****
--- 758,765 ----
  void
  moveto(int ln, int cl)
  {
+     int c;
+ 
      if (vcs == winw) {
  	vln++, vcs = 0;
  	if (!hasam) {
***************
*** 765,776 ****
  	    zputc('\n', shout);
  	} else {
  	    if ((vln < nlnct) && nbuf[vln] && *nbuf[vln])
! 		zputc(*nbuf[vln], shout);
  	    else
! 		zputc(' ', shout);
  	    zputc('\r', shout);
  	    if ((vln < olnct) && obuf[vln] && *obuf[vln])
! 		*obuf[vln] = *nbuf[vln];
  	}
      }
  
--- 767,779 ----
  	    zputc('\n', shout);
  	} else {
  	    if ((vln < nlnct) && nbuf[vln] && *nbuf[vln])
! 		c = *nbuf[vln];
  	    else
! 		c = ' ';
! 	    zputc(c, shout);
  	    zputc('\r', shout);
  	    if ((vln < olnct) && obuf[vln] && *obuf[vln])
! 		*obuf[vln] = c;
  	}
      }
  

-- 
Geoff Wing [mason@primenet.com.au]   Technical Manager
  Phone    : +61-3-9818 2977         PrimeNet - Internet Consultancy
  Facsimile: +61-3-9819 3788         Web : <URL:http://www.primenet.com.au/>
  Mobile   : 0412 162 441

Re: zsh-3.0.3: another problem in Src/jobs.c

[Peter Stephenson]

SUZUKI Hisao wrote:
> 	Again, for the time being, I made a hack to fix it
> rather in a clean way (at least I think so :-) this time.  I
> hope the problem will be fixed soon in any way.

This may well be more comprehensive than my fix.  I simply did the
minimum possible to remove the known problems at the time.  I can
believe a more sophisticated test is necessary.

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

Re: zsh-3.0.3: another problem in Src/jobs.c

[SUZUKI Hisao]

>> Jo napot kivanok!
>> (...or "Jo reggelt kivanok")
>
>Just for those, who do not speak Hungarian, these are just greetings
>meaning good day and good morning :-).  Where did you learn these (I'm
>just curious)?

I am an admirer of Kodaly and Bartok, who were both Hungarian
composers in (early) 20th century music, so I have been learning
Hungarian in hope of understanding their works thoroughly (and I
found it a very interesting language, which somewhat resembles
Japanese in several respects...)

By the way,

[snip]
>Here is Peter's patch:
>
>*** ./Src/jobs.c.old	Wed Jun  4 17:51:43 1997
>--- ./Src/jobs.c	Wed Jun  4 17:53:39 1997
[snip]
>!     if (shout && !ttyfrozen && !jn->stty_in_env && inforeground)
>  	gettyinfo(&shttyinfo);
[snip]

Aha!  A simple and elegant patch!  I am slightly worried since
it does not check the process group, but actually it seems to be
working very well.

However, I noticed a little bug (or, at least, a singular
behavior) just now.  The variable "inforeground" referred in the
patch will not be set true if the last command in a pipeline is
in the current shell.  Hence, though it is artificial,

	% stty erase ^H | :

has no effect since the last command is a shell builtin ":",
whereas

	% stty erase ^H | cat

will modify the tty setting since the last command is "cat".

I do not think whether the last command is in the shell or not
should be relevant in this case.  The following patch would be
better:

----------------------------------------------------------------
*** zsh-3.0.3/Src/jobs.c.ORIG	Tue Jun  3 05:11:27 1997
--- zsh-3.0.3/Src/jobs.c	Thu Jun 19 16:37:42 1997
***************
*** 154,160 ****
  	}
      }
  
!     if (shout && !ttyfrozen && !jn->stty_in_env)
  	gettyinfo(&shttyinfo);
  
      if (isset(MONITOR)) {
--- 154,160 ----
  	}
      }
  
!     if (shout && !ttyfrozen && !jn->stty_in_env && !somestopped && job == thisjob)
  	gettyinfo(&shttyinfo);
  
      if (isset(MONITOR)) {
----------------------------------------------------------------

Regards,

Hisao Suzuki <suzuki@otsl.oki.co.jp>

Re: zsh-3.0.3: another problem in Src/jobs.c

[Zoltan Hidvegi]

SUZUKI Hisao wrote:
> [snip]
> >!     if (shout && !ttyfrozen && !jn->stty_in_env && inforeground)
> >  	gettyinfo(&shttyinfo);
> [snip]
> 
> Aha!  A simple and elegant patch!  I am slightly worried since
> it does not check the process group, but actually it seems to be
> working very well.

It should not depend on process groups.  This should work if MONITOR is
unset and on systems without job control.

> However, I noticed a little bug (or, at least, a singular
> behavior) just now.  The variable "inforeground" referred in the
> patch will not be set true if the last command in a pipeline is
> in the current shell.  Hence, though it is artificial,
> 
> 	% stty erase ^H | :
> 
> has no effect since the last command is a shell builtin ":",

Yes.  And actually, the tty settings should also be saved when a
foreground process is suspended.

The patch below is for the original zsh-3.1.2 and zsh-3.0.3, without
Hisao's patch.  I do hope that this covers all cases now.

Zoltan


*** Src/jobs.c	1997/06/03 03:52:01	3.1.2.7
--- Src/jobs.c	1997/06/20 03:37:03
***************
*** 138,145 ****
      job = jn - jobtab;   /* compute job number */
  
      if (somestopped) {
! 	if (jn->stty_in_env && !jn->ty) {
! 	    jn->ty = (struct ttyinfo *) zalloc(sizeof(struct ttyinfo));
  	    gettyinfo(jn->ty);
  	}
  	if (jn->stat & STAT_STOPPED)
--- 138,146 ----
      job = jn - jobtab;   /* compute job number */
  
      if (somestopped) {
! 	if ((jn->stty_in_env || ttyfrozen) && shout && job == thisjob) {
! 	    if (!jn->ty)
! 		jn->ty = (struct ttyinfo *) zalloc(sizeof(struct ttyinfo));
  	    gettyinfo(jn->ty);
  	}
  	if (jn->stat & STAT_STOPPED)
***************
*** 155,161 ****
  	}
      }
  
!     if (shout && !ttyfrozen && !jn->stty_in_env)
  	gettyinfo(&shttyinfo);
  
      if (isset(MONITOR)) {
--- 156,162 ----
  	}
      }
  
!     if (shout && !ttyfrozen && !jn->stty_in_env && job == thisjob)
  	gettyinfo(&shttyinfo);
  
      if (isset(MONITOR)) {

Re: zsh-3.0.3: another problem in Src/jobs.c

[SUZUKI Hisao]

>> in the current shell.  Hence, though it is artificial,
>> 
>> 	% stty erase ^H | :
>> 
>> has no effect since the last command is a shell builtin ":",
>
>Yes.  And actually, the tty settings should also be saved when a
>foreground process is suspended.
>
>The patch below is for the original zsh-3.1.2 and zsh-3.0.3, without
>Hisao's patch.  I do hope that this covers all cases now.

I am afraid it does not yet.  The tty setting goes bad after a
session like this:

	% emacs -nw &
	% fg
	[type ^Z to suspend the emacs]
	% stty

(see e-mail <9706190624.AA05850@oz.fantasy.otsl.oki.co.jp> from
me on Thu, 19 Jun 97 15:24:32 JST)

My patch of <9706192332.AA00517@oz.fantasy.otsl.oki.co.jp> works
fine in these cases.  Perhaps, for the present implementation,
the tty setting should _not_ be saved into "shttyinfo" when a
foreground process is stopped...

Regards,

Hisao Suzuki <suzuki@otsl.oki.co.jp>