PROMPT_SP bug with local in precmd

PROMPT_SP bug with local in precmd

[Vincent Lefevre]

I can reproduce this both under Linux and Mac OS X:

ypig% setopt PRINT_EXIT_VALUE
ypig% echo -n foo; false
foo%
zsh: exit 1
ypig% precmd() { local v=bar }  
ypig% echo -n foo; false      
foozsh: exit 1
ypig% precmd() { v=bar }      
ypig% echo -n foo; false
foo%
zsh: exit 1
ypig%

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

Re: PROMPT_SP bug with local in precmd

[Bart Schaefer]

On Nov 26, 12:51pm, Vincent Lefevre wrote:
}
} ypig% precmd() { local v=bar }  
} ypig% echo -n foo; false      
} foozsh: exit 1

It's not "local" that triggers this, it's any command:

torch% precmd() { : }
torch% echo -n foo; false
foozsh: exit 1
torch% precmd() { echo -n Hi }
Hi%
torch% echo -n foo; false
foozsh: exit 1
Hi%
torch% precmd() { /bin/echo -n Hi }
Hi%
torch% echo -n foo; false
foozsh: exit 1
Hi%
zsh: exit 1
torch% 


This makes me suspect it's not so much a bug as a race condition -- when
there is a command in precmd, the order in which the exit values from
children (even fake children) are collected and reported is changed.
Note how "zsh: exit 1" is printed twice when the precmd is a forked
process instead of all builtins.

My conclusion from this is that "foozsh: exit 1" is what you really
ought to be seeing all the time, and "zsh: exit 1" on its own line is a
side-effect of a stdio buffer never having been flushed.

Re: PROMPT_SP bug with local in precmd

[Peter Stephenson]

On Fri, 26 Nov 2010 09:38:10 -0800
Bart Schaefer <schaefer@brasslantern.com> wrote:
> My conclusion from this is that "foozsh: exit 1" is what you really
> ought to be seeing all the time, and "zsh: exit 1" on its own line is a
> side-effect of a stdio buffer never having been flushed.

I think the following demonstrates the opposite, i.e. the exit message
now always appears on a newline, but as it's race prone it's hard
to be sure. I hope I'm not undoing any magic with this.

Index: Src/utils.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/utils.c,v
retrieving revision 1.249
diff -p -u -r1.249 utils.c
--- Src/utils.c	26 Nov 2010 18:50:22 -0000	1.249
+++ Src/utils.c	26 Nov 2010 22:48:46 -0000
@@ -1293,6 +1293,7 @@ preprompt(void)
 	opts[PROMPTPERCENT] = percents;
 	zputs(str, shout);
 	fprintf(shout, "%*s\r%*s\r", (int)columns - w - !hasxn, "", w, "");
+	fflush(shout);
 	free(str);
     }
 
-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/

Re: PROMPT_SP bug with local in precmd

[Wayne Davison]

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

On Fri, Nov 26, 2010 at 2:53 PM, Peter Stephenson <
p.w.stephenson@ntlworld.com> wrote:

> I hope I'm not undoing any magic with this.
>

That added fflush() seems like a good idea to me. I had assumed that shout
wasn't buffered, but that appears to not be the case.  Since the exit-output
cited above is going to stderr (I'm assuming that is coming from exec.c
around line 3188), your added flush should ensure that the PROMPT_SP "magic"
always gets output prior to the following use of stderr, and that should
make it work more consistently.

..wayne..

Re: PROMPT_SP bug with local in precmd

[Bart Schaefer]

On Nov 26, 10:53pm, Peter Stephenson wrote:
} Subject: Re: PROMPT_SP bug with local in precmd
}
} On Fri, 26 Nov 2010 09:38:10 -0800
} Bart Schaefer <schaefer@brasslantern.com> wrote:
} > My conclusion from this is that "foozsh: exit 1" is what you really
} > ought to be seeing all the time, and "zsh: exit 1" on its own line is a
} > side-effect of a stdio buffer never having been flushed.
} 
} I think the following demonstrates the opposite, i.e. the exit message
} now always appears on a newline but as it's race prone it's hard
} to be sure.

Hmm.  Now I get:

torch% setopt printexitvalue
torch% echo -n foo; false
foo%
zsh: exit 1
torch% precmd() { /bin/echo -n Hi }
torch%                             
torch% echo -n foo; false          
foo%
zsh: exit 1
Hizsh: exit 1
torch% 

Note that immediately after creating the precmd function, the "Hi" is
NOT visible.  (This is with Wayne's hasxn patch as well as with your
fflush).  However, it *was* output -- the prompt just backed up and
trod over it.  So PROMPT_SP kicks in before precmd does, which is I
think what Wayne asserted it should do.

However with a non-builtin command in the precmd, I'm still getting
"zsh: exit 1" printed twice, so I think we're both correct -- that
is, there's *another* place where a fflush() is needed.  Looks like
just after line 3188 in Src/exec.c.

With fflush() added there as well, I consistently get this:

torch% setopt printexitvalue
torch% echo -n foo; false
foozsh: exit 1
torch% precmd() { /bin/echo -n Hi }
torch% echo -n foo; false          
foozsh: exit 1
torch% 

Note there's now only one "zsh: exit 1" and the prompt now consistently
covers up the output from precmd.

} I hope I'm not undoing any magic with this.

Ditto.

Re: PROMPT_SP bug with local in precmd

[Peter Stephenson]

On Fri, 26 Nov 2010 18:27:34 -0800
Bart Schaefer <schaefer@brasslantern.com> wrote:
> However with a non-builtin command in the precmd, I'm still getting
> "zsh: exit 1" printed twice, so I think we're both correct -- that
> is, there's *another* place where a fflush() is needed.  Looks like
> just after line 3188 in Src/exec.c.
> 
> With fflush() added there as well, I consistently get this:
> 
> torch% setopt printexitvalue
> torch% echo -n foo; false
> foozsh: exit 1
> torch% precmd() { /bin/echo -n Hi }
> torch% echo -n foo; false          
> foozsh: exit 1
> torch% 
> 
> Note there's now only one "zsh: exit 1" and the prompt now consistently
> covers up the output from precmd.
> 
> } I hope I'm not undoing any magic with this.
> 
> Ditto.

I've committed mine, so if you commit yours we can spend ages on mutual
recrimination.  (This is the Internet Age.)

-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/

Re: PROMPT_SP bug with local in precmd

[Vincent Lefevre]

On 2010-11-26 09:38:10 -0800, Bart Schaefer wrote:
> On Nov 26, 12:51pm, Vincent Lefevre wrote:
> }
> } ypig% precmd() { local v=bar }  
> } ypig% echo -n foo; false      
> } foozsh: exit 1
> 
> It's not "local" that triggers this, it's any command:

Actually, variable assignment (without "local") doesn't trigger
the problem. That's why I thought it was due to "local".

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