Bang completion kills zsh in emacs process buffer

Bang completion kills zsh in emacs process buffer

[Dwight Shih]

I understand why the bug occurs, but it's been 4 or 5 years since I've
 been inside the zsh code and I'm unclear on the best way to correct the
 problem.
 
 Here's the environment:
    Mac OS X 10.2.6
    zsh-4.1.1
    emacs 21.3.50
 
 My emacs was built from CVS sources on 17 August, as the Mac OS X
 windows
 support not yet available in an official distribution. However, it looks
 like this could happen in any version of emacs running with native
 windows support.
 
 To reproduce:
    start emacs
    M-x shell     #if necessary M-x customize-variable
                          explicit-shell-file-name to zsh binary
    $ pwd
    $ echo !$     #zsh will now die
 
 In the debugger, zsh dies at hist.c:1092 trying to do a zputs(prt,shout)
 because the file pointer shout is null.
 
 As best I can tell, shout is normally initialized in init_shout, called
 from init_io at init.c:452. Walking through init_io, SHTTY never gets
 opened because the shell doesn't seem to be running in a tty (I would
 expect this to be true in any windowed emacs). So when it comes time to
 call init_shout, we conclude that since we're not using a tty, we're not
 going to need shout.
 
 I would appreciate it if someone who understood the logic behind
 interactive and non-interactive shells and the file pointers involved
 would take a look and tell me whether it's better to always set shout in
 init_io (either by direct assignment to stdout or dup) or test for
 shout==null in hist.c
 
 Thanks,
 
 Dwight Shih 

Re: Bang completion kills zsh in emacs process buffer

[Peter Stephenson]

"Dwight Shih" wrote:
>  To reproduce:
>   start emacs
>   M-x shell
>  $ pwd
>  $ echo !$      #zsh will now die
>
>  In the debugger, zsh dies at hist.c:1092 trying to do a zputs(prt,shout)
>  because the file pointer shout is null.

The confusion comes because somehow zsh thinks it's non-interactive but
is still using history --- which is basically what you said.

It surprises me a bit that it's trying to use bang-history at all in this
case.  It's usually turned off non-interactively.  Ideally we need to
track down why it's doing that.  I can't seem to reproduce it --- partly
because shell mode is stealing my !'s for some purpose of its own I can
only guess at, which indicates some of the reasons I never use Emacs
shell mode.

It might help to see the options it has set; the options related to
interactive or non-interactive operations are interactive, shinstdin and
monitor.  This is the code in hbegin which I think should be stopping it
using history for non-interactive shells:

    else if (dohist != 2)
	stophist = (!interact || unset(SHINSTDIN)) ? 2 : 0;

(interact is a definition for isset(INTERACTIVE); it would be much
clearer and hardly longers to use the latter throughout.  Possibly it
used to mean something else.)

So stophist should be 2 in this case (we should use enums for this sort
of flag but this code goes back to the dawn of time).  If you feel
inclined to track this down (which would be much appreciated), finding
out if stophist is getting set to 2, and if it is why we are doing
bang-history anyway, would be a good path to take.

Testing for shout != NULL in hist.c is an entirely benign workaround if
no-one is able to find out why it's using history at all, but I don't
think it's the root of the problem.

-- 
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: Bang completion kills zsh in emacs process buffer

[Dwight Shih]

Peter,

zsh thinks that it's an interactive shell because emacs invokes it with
the -i option.

Dwight

On Fri, 19 Sep 2003 17:18:23 +0100, "Peter Stephenson" <pws@csr.com>
said:
> "Dwight Shih" wrote:
> >  To reproduce:
> >   start emacs
> >   M-x shell
> >  $ pwd
> >  $ echo !$      #zsh will now die
> >
> >  In the debugger, zsh dies at hist.c:1092 trying to do a zputs(prt,shout)
> >  because the file pointer shout is null.
> 
> The confusion comes because somehow zsh thinks it's non-interactive but
> is still using history --- which is basically what you said.
> 
> It surprises me a bit that it's trying to use bang-history at all in this
> case.  It's usually turned off non-interactively.  Ideally we need to
> track down why it's doing that.  I can't seem to reproduce it --- partly
> because shell mode is stealing my !'s for some purpose of its own I can
> only guess at, which indicates some of the reasons I never use Emacs
> shell mode.
> 
> It might help to see the options it has set; the options related to
> interactive or non-interactive operations are interactive, shinstdin and
> monitor.  This is the code in hbegin which I think should be stopping it
> using history for non-interactive shells:
> 
>     else if (dohist != 2)
> 	stophist = (!interact || unset(SHINSTDIN)) ? 2 : 0;
> 
> (interact is a definition for isset(INTERACTIVE); it would be much
> clearer and hardly longers to use the latter throughout.  Possibly it
> used to mean something else.)
> 
> So stophist should be 2 in this case (we should use enums for this sort
> of flag but this code goes back to the dawn of time).  If you feel
> inclined to track this down (which would be much appreciated), finding
> out if stophist is getting set to 2, and if it is why we are doing
> bang-history anyway, would be a good path to take.
> 
> Testing for shout != NULL in hist.c is an entirely benign workaround if
> no-one is able to find out why it's using history at all, but I don't
> think it's the root of the problem.
> 
> -- 
> 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: Bang completion kills zsh in emacs process buffer

[Peter Stephenson]

"Dwight Shih" wrote:
> Peter,
> 
> zsh thinks that it's an interactive shell because emacs invokes it with
> the -i option.

OK, I think I see what's going on now.

!-history is in use because:
 - the shell is interactive, and
 - shell input is standard input

shout gets set if:
  - the shell is interactive, and
  - SHTTY is set

SHTTY is set if and only we are talking to a tty on fd 0, and we aren't.
We can't very well set it or shout in that case.  (We could in principle
look harder for a tty, but that's not the issue here --- there just
isn't one.)

In that case, I think the right thing to do might be the following.
Presumably this bug has been there forever.

Comments?

Index: Src/hist.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/hist.c,v
retrieving revision 1.46
diff -u -r1.46 hist.c
--- Src/hist.c	11 Aug 2003 13:19:55 -0000	1.46
+++ Src/hist.c	19 Sep 2003 17:17:09 -0000
@@ -1091,9 +1091,16 @@
 
 	ptr = ztrdup(chline);
 	if ((flag & (HISTFLAG_DONE | HISTFLAG_RECALL)) == HISTFLAG_DONE) {
-	    zputs(ptr, shout);
-	    fputc('\n', shout);
-	    fflush(shout);
+	    /*
+	     * If fd 0 isn't a tty, we may not have a shout, even
+	     * though the shell is interactive.  Observed with
+	     * Emacs shell mode, which makes the shell interactive
+	     * explicitly.
+	     */
+	    FILE *fout = shout ? shout : stdout;
+	    zputs(ptr, fout);
+	    fputc('\n', fout);
+	    fflush(fout);
 	}
 	if (flag & HISTFLAG_RECALL) {
 	    zpushnode(bufstack, ptr);

-- 
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: Bang completion kills zsh in emacs process buffer

[Peter Stephenson]

"Dwight Shih" wrote:
> But grepping about the
> source shows that shout is often treated as synonymous with interactive.
> For example, checkrmall will silently succeed if shout is null. Command
> correction (setopt correct) also just writes to shout. I think that we'd
> be better off setting shout whenever we have an interactive shell.

As long as we can rely on things not testing shout to see if there
terminal is set up... usually SHTTY is used for that purpose, so maybe
this is OK, but possibly we are increasing the danger from the existing
conflict of purposes between the various options, SHTTY and shout.

Other bits of the code seemd to think stderr rather than stdout was a
better substitute for shout, which is probably true.

It looks like the only place we need to be careful when closing shout is
the one in init, below.

Index: Src/init.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/init.c,v
retrieving revision 1.35
diff -u -r1.35 init.c
--- Src/init.c	3 Sep 2003 10:15:36 -0000	1.35
+++ Src/init.c	19 Sep 2003 18:13:57 -0000
@@ -379,7 +379,13 @@
 #endif
 
     if (shout) {
-	fclose(shout);
+	/*
+	 * Check if shout was set to stderr, if so don't close it.
+	 * We do this if we are interactive but don't have a
+	 * terminal.
+	 */
+	if (shout != stderr)
+	    fclose(shout);
 	shout = 0;
     }
     if (SHTTY != -1) {
@@ -448,9 +454,9 @@
 
     /* We will only use zle if shell is interactive, *
      * SHTTY != -1, and shout != 0                   */
-    if (interact && SHTTY != -1) {
+    if (interact) {
 	init_shout();
-	if(!shout)
+	if(!SHTTY || !shout)
 	    opts[USEZLE] = 0;
     } else
 	opts[USEZLE] = 0;
@@ -475,6 +481,14 @@
 init_shout(void)
 {
     static char shoutbuf[BUFSIZ];
+
+    if (SHTTY == -1)
+    {
+	/* Since we're interative, it's nice to have somewhere to write. */
+	shout = stderr;
+	return;
+    }
+
 #if defined(JOB_CONTROL) && defined(TIOCSETD) && defined(NTTYDISC)
     int ldisc = NTTYDISC;
 
Index: Src/jobs.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/jobs.c,v
retrieving revision 1.21
diff -u -r1.21 jobs.c
--- Src/jobs.c	2 May 2003 10:25:33 -0000	1.21
+++ Src/jobs.c	19 Sep 2003 18:13:57 -0000
@@ -322,8 +322,9 @@
 	}
     }
 
-    if (shout && !ttyfrozen && !jn->stty_in_env && !zleactive &&
-	job == thisjob && !somestopped && !(jn->stat & STAT_NOSTTY))
+    if (shout && shout != stderr && !ttyfrozen && !jn->stty_in_env &&
+	!zleactive && job == thisjob && !somestopped &&
+	!(jn->stat & STAT_NOSTTY)) 
 	gettyinfo(&shttyinfo);
 
     if (isset(MONITOR)) {

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