Confirming X02zlevi test failures

Confirming X02zlevi test failures

[Bart Schaefer]

Oliver - just in case it matters -

*** /tmp/zsh.ztst.out.18135     Wed Nov 19 08:40:29 2014
--- /tmp/zsh.ztst.tout.18135    Wed Nov 19 08:40:30 2014
***************
*** 1,3 ****
! BUFFER: i
! 
! CURSOR: 2
--- 1,2 ----
! BUFFER: aIo
! CURSOR: 3
Test ../../zsh-5.0/Test/X02zlevi.ztst failed: output differs from expected as shown above for:
  zletest $'\e' $'~aI\e' $'~o\e' \~
Was testing: swap case on a blank line

As I think Jun T. mentioned, this can be fixed by (bindkey -r \\e~) after
which the tests continue through to the "daw" failure that's already been
reported a couple of times.

-- 
Barton E. Schaefer

Re: Confirming X02zlevi test failures

[Ray Andrews]

On 11/19/2014 08:50 AM, Bart Schaefer wrote:
> As I think Jun T. mentioned, this can be fixed by (bindkey -r \\e~) after
> which the tests continue through to the "daw" failure that's already been
> reported a couple of times.
I only noticed myself reporting that yesterday. Is there more traffic 
somewhere else?  If so
I should monitor that so as to avoid redundant reports.

Re: Confirming X02zlevi test failures

[Oliver Kiddle]

Bart wrote:
> 
> As I think Jun T. mentioned, this can be fixed by (bindkey -r \\e~) after
> which the tests continue through to the "daw" failure that's already been
> reported a couple of times.

Removing the binding is just a workaround. While it is not the feature
that test is there to test, it'd really be better to understand why
KEYTIMEOUT isn't working in the test cases. I've tried fiddling with
stty settings and using sleep and that hasn't helped.

Oliver

Re: Confirming X02zlevi test failures

[Bart Schaefer]

On Nov 20, 12:05am, Oliver Kiddle wrote:
}
} While it is not the feature that test is there to test, it'd really be
} better to understand why KEYTIMEOUT isn't working in the test cases.

I instrumented raw_getbyte() and calc_timeout().  Found a couple of
things.

One, there's still a race condition with the PTY.  In random cases,
the cursor motion output that's sent by ZLE can be seen by the slave
PTY as if it were input from the master PTY:

0 0 88	<- first time into calc_timeout(), no timeout, 88 is garbage
^[	<- saw the escape character
1 1 1	<- we should time out in 1/100th second
^H	<- OOPS!  Read the backspace (output) as input?!
~	<- And there's the tilde

The second thing I found is that, possibly for the same reason, the
delayzetterm flag is (inconsistently in terms of zletest input, but
consistently for this particular test) getting set, which causes
KEYTIMEOUT to be ignored.  The comment for delayzetterm says:

         * Problems can occur on some systems when switching from
         * canonical to non-canonical input.  The former is usually
         * set while running programmes, but the latter is necessary
         * for zle.  If there is input in canonical mode, then we
         * need to read it without setting up the terminal.  Furthermore,
         * while that input gets processed there may be more input
         * being typed (i.e. further typeahead).  This means that
         * we can't set up the terminal for zle *at all* until
         * we are sure there is no more typeahead to come.  So
         * if there is typeahead, we set the flag delayzsetterm.
         * Then getbyte() performs another FIONREAD call; if that is
         * 0, we have finally used up all the typeahead, and it is
         * safe to alter the terminal, which we do at that point.

So ... for some reason zsetterm() believes that there is typeahead and
delays switching out of the canonical mode for too long.  This seems
to happen only when ESC is the first character written to the PTY, but
I'm not 100% sure of that.

Unfortunately, I don't know what to do about this, yet.  Thoughts?

Re: Confirming X02zlevi test failures

[Jun T.]

2014/11/20 15:20, Bart Schaefer <schaefer@brasslantern.com> wrote:

> delayzetterm flag is (inconsistently in terms of zletest input, but
> consistently for this particular test) getting set,

# I first suspected that FIONREAD was not working correctly.
# So I built zsh without using FIONREAD, but it didn't work.

Next I replaced TCSADRAIN by TCSANOW in settyinfo() (utils.c, line 1554),
and now 'swap case on a blank line' in X02zlevi.ztst seems to succeed.
But I don't know what kind of unwanted side effects it may cause.

Moreover, if 'werase undef' is removed from comptest, line 37, then
X02zlevi.ztst fails again at
'line based put before followed by character based yank-pop'.

Re: Confirming X02zlevi test failures

[Bart Schaefer]

On Nov 21, 12:42am, Jun T. wrote:
}
} Next I replaced TCSADRAIN by TCSANOW in settyinfo() (utils.c, line 1554),
} and now 'swap case on a blank line' in X02zlevi.ztst seems to succeed.
} But I don't know what kind of unwanted side effects it may cause.

It'll cause typeahead to be misinterpreted when using a "real" terminal
device, I fear.  It took a lot of work back in the day to get that tty
draining stuff right.
 
} Moreover, if 'werase undef' is removed from comptest, line 37, then
} X02zlevi.ztst fails again at
} 'line based put before followed by character based yank-pop'.

I begin to suspect that what needs to happen is that zpty needs to be a
lot more aggressive internally about consuming (and buffering up) the
slave output as fast as it appears, even if a zpty read call has not
yet been made from the shell.  Clearly the slave can "see" it's own
output if the master doesn't manage to grab it first.

Re: Confirming X02zlevi test failures

[Oliver Kiddle]

Bart wrote:
>
> I begin to suspect that what needs to happen is that zpty needs to be a
> lot more aggressive internally about consuming (and buffering up) the
> slave output as fast as it appears, even if a zpty read call has not
> yet been made from the shell.  Clearly the slave can "see" it's own
> output if the master doesn't manage to grab it first.

That might be worth a try.

I tried the following change to see if it would have an effect.
It results in zpty getting a device such as /dev/pts/2 instead of
/dev/ttyp0 on FreeBSD. I suspect it might also avoid the need to kldload
pty but would need to reboot a remote machine to confirm. So I think it
is useful anyway. Unfortunately it has no effect on the bug.

Oliver

diff --git a/Src/Modules/zpty.c b/Src/Modules/zpty.c
index d119658..63c79a7 100644
--- a/Src/Modules/zpty.c
+++ b/Src/Modules/zpty.c
@@ -189,7 +189,11 @@ get_pty(int master, int *retfd)
 #endif
 
     if (master) {
+#ifdef HAVE_POSIX_OPENPT
+	if ((mfd = posix_openpt(O_RDWR|O_NOCTTY)) < 0)
+#else
 	if ((mfd = open("/dev/ptmx", O_RDWR|O_NOCTTY)) < 0)
+#endif
 	    return 1;
 
 	if (grantpt(mfd) || unlockpt(mfd) || !(name = ptsname(mfd))) {
diff --git a/configure.ac b/configure.ac
index 306a005..56c4cfb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1291,6 +1291,7 @@ AC_CHECK_FUNCS(strftime strptime mktime timelocal \
 	       pcre_compile pcre_study pcre_exec \
 	       nl_langinfo \
 	       erand48 open_memstream \
+	       posix_openpt \
 	       wctomb iconv \
 	       grantpt unlockpt ptsname \
 	       htons ntohs \

Re: Confirming X02zlevi test failures

[Jun T.]

2014/11/21 02:18, Bart Schaefer <schaefer@brasslantern.com> wrote:
> I begin to suspect that what needs to happen is that zpty needs to be a
> lot more aggressive internally about consuming (and buffering up) the
> slave output ...

If I add 'zpty_flush' before 'zpty -w', then the test succeeds on my Mac
without 'bindkey -r "\e~"'; but it doesn't work on FreeBSD.

On Mac, the zpty_flush must be in the 'for input' loop, and before
the 'read -t', as in the patch below (for the 1st hunk, see my previous
post, 33741).



diff --git a/Test/comptest b/Test/comptest
index c67237a..1f4dac6 100644
--- a/Test/comptest
+++ b/Test/comptest
@@ -34,7 +34,7 @@ comptestinit () {
 "fpath=( $fpath )" \
 "bindkey -$comptest_keymap" \
 'LISTMAX=10000000
-stty 38400 columns 80 rows 24 werase undef tabs
+stty 38400 columns 80 rows 24 tabs -icanon -iexten
 TERM=vt100
 KEYTIMEOUT=1
 setopt zle
@@ -162,9 +162,9 @@ comptest () {
 zletest () {
   local first=0
   for input; do
+    zpty_flush Before zletest
     # sleep for $KEYTIMEOUT
     (( first++ )) && read -t 0.011 -k 1 < /dev/null
-    # zpty_flush Before zletest
     zpty -n -w zsh "$input"
   done
   zpty -n -w zsh $'\C-X'

Re: Confirming X02zlevi test failures

[Bart Schaefer]

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

On Nov 21, 2014 10:18 AM, "Jun T." <takimoto-j@kba.biglobe.ne.jp> wrote:
>
> If I add 'zpty_flush' before 'zpty -w', then the test succeeds on my Mac
> without 'bindkey -r "\e~"'; but it doesn't work on FreeBSD.

I found that adding the zpty_flush made the test succeed some of the time
rather than consistently failing, but it's still not reliable.  E.g.,
sometimes the second ESC~ fails instead of the first, sometimes both
succeed.

Re: Confirming X02zlevi test failures

[Jun T.]

2014/11/22 03:56, Bart Schaefer <schaefer@brasslantern.com> wrote:
> I found that adding the zpty_flush made the test succeed some of the time
> rather than consistently failing, but it's still not reliable

Sorry, I run the test only 10 times.

I run the test 1000 times, with about 5% failure (Mac).

If I commented out 'zpty_flush After zletest', then 2/1000 failed.
Then I changed 'read -t 0.011' to 'read -t 0.02' and got 1000/1000 success.

On FreeBSD, I got 1000/1000 success only by commenting out
'zpty_flush After zletest'.

Re: Confirming X02zlevi test failures

[Bart Schaefer]

On Nov 23, 10:37am, Jun T. wrote:
}
} If I commented out 'zpty_flush After zletest', then 2/1000 failed.
} Then I changed 'read -t 0.011' to 'read -t 0.02' and got 1000/1000 success.
} 
} On FreeBSD, I got 1000/1000 success only by commenting out
} 'zpty_flush After zletest'.

OK, so maybe what we needed all along instead of those zpty_flush calls
was to have a timeout delay.

Can you post what you ended up with so I can be sure of trying the same
thing?

Re: Confirming X02zlevi test failures

[Jun T.]

2014/11/23 12:3, Bart Schaefer <schaefer@brasslantern.com> wrote:
> OK, so maybe what we needed all along instead of those zpty_flush calls
> was to have a timeout delay.

Sorry, my post was not clear enough.
I need BOTH zpty_flush and '-t 0.02' to get 1000/1000 success.
With zpty_flush only, I got 998/1000 success on Mac (1000/1000 on FreeBSD).

So this is a kind of yet another workaround.
But the result supports your guess that clearing the buffer of the slave
tty would be important.


diff --git a/Test/comptest b/Test/comptest
index c67237a..5964114 100644
--- a/Test/comptest
+++ b/Test/comptest
@@ -34,7 +34,7 @@ comptestinit () {
 "fpath=( $fpath )" \
 "bindkey -$comptest_keymap" \
 'LISTMAX=10000000
-stty 38400 columns 80 rows 24 werase undef tabs
+stty 38400 columns 80 rows 24 tabs -icanon -iexten
 TERM=vt100
 KEYTIMEOUT=1
 setopt zle
@@ -162,9 +162,9 @@ comptest () {
 zletest () {
   local first=0
   for input; do
+    zpty_flush Before zletest
     # sleep for $KEYTIMEOUT
-    (( first++ )) && read -t 0.011 -k 1 < /dev/null
-    # zpty_flush Before zletest
+    (( first++ )) && read -t 0.02 -k 1 < /dev/null
     zpty -n -w zsh "$input"
   done
   zpty -n -w zsh $'\C-X'
@@ -172,6 +172,6 @@ zletest () {
     print "failed to invoke finish widget."
     return 1
   }
-  # zpty_flush After zletest
+  zpty_flush After zletest
   print -lr "${(@)${(@ps:\r\n:)log##*<WIDGET><finish>}[2,-2]}"
 }

Re: Confirming X02zlevi test failures

[Oliver Kiddle]

"Jun T." wrote:
> If I commented out 'zpty_flush After zletest', then 2/1000 failed.
> Then I changed 'read -t 0.011' to 'read -t 0.02' and got 1000/1000 success.

One thing to be wary of when using the vi test is that there is no
counter test in there: we don't have a test of username completion on
\e~ with a suitable lack of delay between the two keystrokes.

If we can make it work, we should probably have two earlier tests
explicitly testing KEYTIMEOUT.

Bart wrote:
> I begin to suspect that what needs to happen is that zpty needs to be a
> lot more aggressive internally about consuming (and buffering up) the
> slave output as fast as it appears

I decided to see if I could reproduce the problem using expect. The
results somewhat convince me that tweaking zpty isn't going to help.
With expect it is easier to adjust the delays and parameters. But the
results aren't always making much sense, either on Linux or BSD. On
Linux, switching between precisely 1000 and 999 micro seconds was making
a difference. I've attached the expect script. The second parameter to
send_slow and -s option to send control the delays but other, seemingly
innocuous, changes can also make a difference.

> The second thing I found is that, possibly for the same reason, the
> delayzetterm flag is (inconsistently in terms of zletest input, but
> consistently for this particular test) getting set, which causes
> KEYTIMEOUT to be ignored.  The comment for delayzetterm says:

I've been looking at this too. I noticed instances where the
FIONREAD in getbyte() was returning 0, it then called zsetterm again
which does another FIONREAD ioctl but received a value of 5. There's an
unavoidable race condition if we can't stop more characters coming in
between the FIONREAD and the settyinfo. Even so, it seems a bit
pointless to do the ioctl twice. I've attached a patch to make getbyte
just call zsetterm again and rely on it trying the ioctl again. Not that
this helps us much.

Are you just using strategically placed dputs for instrumentation? I
seem to get nothing from them after the terminal has been configured.
truss/ltrace are not much use. dtrace is just hanging for me but I've
never tried it on BSD before. I'll try gdb with a command file. I've
just realised on reading the e-mail before sending that the dtrace
problem is because the prompt is # for root instead of the % that expect
is expecting. I couldn't find a way to enable it for a normal user on
BSD.

Oliver

diff --git a/Src/Zle/zle_main.c b/Src/Zle/zle_main.c
index d157e36..a38f55b 100644
--- a/Src/Zle/zle_main.c
+++ b/Src/Zle/zle_main.c
@@ -238,9 +238,9 @@ zsetterm(void)
 	 * we can't set up the terminal for zle *at all* until
 	 * we are sure there is no more typeahead to come.  So
 	 * if there is typeahead, we set the flag delayzsetterm.
-	 * Then getbyte() performs another FIONREAD call; if that is
-	 * 0, we have finally used up all the typeahead, and it is
-	 * safe to alter the terminal, which we do at that point.
+	 * Then getbyte() calls here to performs another FIONREAD call;
+	 * if that is 0, we have finally used up all the typeahead, and
+	 * it is safe to alter the terminal, which we do at that point.
 	 */
 	delayzsetterm = 1;
 	return;
@@ -884,12 +884,8 @@ getbyte(long do_keytmout, int *timeout)
 	ret = STOUC(kungetbuf[--kungetct]);
     else {
 #ifdef FIONREAD
-	if (delayzsetterm) {
-	    int val;
-	    ioctl(SHTTY, FIONREAD, (char *)&val);
-	    if (!val)
-		zsetterm();
-	}
+	if (delayzsetterm)
+	    zsetterm();
 #endif
 	for (;;) {
 	    int q = queue_signal_level();

#<text/plain
#!/usr/local/bin/expect -f
set timeout -1
set send_slow { 1 .05 }
spawn -noecho -nottyinit -nottycopy inst/bin/zsh -f
expect %
send "bindkey -s '\\e~' user\r"
expect %
#send "KEYTIMEOUT=40\r"
#expect %
send -s ": o\033~\r"
expect {
    O { puts "\nswap case" }
    user { puts "\nusername completion" }
}
send "exit\r"

Re: Confirming X02zlevi test failures

[Bart Schaefer]

On Nov 23,  4:44pm, Jun T. wrote:
} 
} -stty 38400 columns 80 rows 24 werase undef tabs
} +stty 38400 columns 80 rows 24 tabs -icanon -iexten

I wonder if this isn't more to do with the success than the changes
in flushing/timing?  With -icanon here, the initial state of the
terminal may not have to be changed before the first bytes are read
from the master connection.


On Nov 23, 10:00am, Oliver Kiddle wrote:
}
} Are you just using strategically placed dputs for instrumentation? I
} seem to get nothing from them after the terminal has been configured.

I both added some dputs() and also exported ZSH_DEBUG_LOG in the caller
environment.  The file named there collects everything nicely.

-- 
Barton E. Schaefer

Re: Confirming X02zlevi test failures

[Bart Schaefer]

On Nov 23,  4:44pm, Jun T. wrote:
}
} Sorry, my post was not clear enough.
} I need BOTH zpty_flush and '-t 0.02' to get 1000/1000 success.
} With zpty_flush only, I got 998/1000 success on Mac (1000/1000 on FreeBSD).

With the patch from 33769 applied, I get about 30% failure rate of X02
(running just that test repeatedly), e.g.,

    repeat 100 make check TESTNUM=X02

Re: Confirming X02zlevi test failures

[Jun T.]

2014/11/21 02:18, Bart Schaefer <schaefer@brasslantern.com> wrote:

> Clearly the slave can "see" it's own
> output if the master doesn't manage to grab it first.

I also added some dputs() to zle_main.c as in the patch below, but
I could not reproduce your observation; it seems (at least on my
Mac) the slave zsh sees only what the master has written.
But the test fails nonetheless.

Typical dputs() output during testing 'swap case on a blank line' is
like the following (on Mac, select() is used in place of poll()):

 T 1 1 1 0
 S 0 10000     select() timeouts
 T 0 0 0 0
 R <^[>

 T 1 1 1 0
 S 1 10000      does NOT timeout
 R <~>

 T 0 0 0 1      delayzsetterm=1
 R <a>

 T 0 0 0 1
 R <I>

 T 0 0 0 1
 R <^[>

 T 1 1 1 1
 R <~>          select() not called since delayzsetterm=1

For the 1st tilde, select() is called with the correct timeout, but it
returns 1 (i.e., 1 descriptor is ready for read) rather than timeouts.
This may be due to that some data has been received by the slave tty
while it is in the canonical mode...?

The reason that delayzsetterm becomes 1 may be that bash_complete-word
calls zrefresh(). I guess this problem will go away if the 1st ESC-tilde
is correctly handeld.
But once delayzsetterm becomes 1, it is not reset to 0 until zsetterm()
is called again.


diff --git a/Src/Zle/zle_main.c b/Src/Zle/zle_main.c
index d157e36..31b577d 100644
--- a/Src/Zle/zle_main.c
+++ b/Src/Zle/zle_main.c
@@ -531,6 +531,8 @@ raw_getbyte(long do_keytmout, char *cptr)
 #endif
 
     calc_timeout(&tmout, do_keytmout);
+    dputs("T %d %d %d %d", do_keytmout, tmout.tp,
+	    tmout.tp ? tmout.exp100ths : 0, delayzsetterm);
 
     /*
      * Handle timeouts and watched fd's.  If a watched fd or a function
@@ -635,6 +637,7 @@ raw_getbyte(long do_keytmout, char *cptr)
 	    selret = select(fdmax+1, (SELECT_ARG_2_T) & foofd,
 			    NULL, NULL, tvptr);
 	    winch_block();
+	    dputs("S %d %d",selret,tvptr->tv_usec);
 # endif
 	    /*
 	     * Make sure a user interrupt gets passed on straight away.
@@ -853,6 +856,7 @@ raw_getbyte(long do_keytmout, char *cptr)
     winch_unblock();
     ret = read(SHTTY, cptr, 1);
     winch_block();
+    dputs("R <%c>\n",*cptr);
 
     return ret;
 }

Re: Confirming X02zlevi test failures

[Jun T.]

Sorry for disturbing.
I modified the dputs() as below, and got:

 T 1 1 1 0 0
 S 0 10000
 T 0 0 0 0 10
 R <^[>

 T 1 1 1 0 9
 S 1 10000
 R <~>
(snip)
 T 0 0 0 1 1
 R <^X>

 T 1 1 1 0 0
 S 0 10000
 T 0 0 0 0 0
 R <^@>

So all the 10 characters (ESC ~ a I ESC ~ o ESC ~ ^X) are already
available when reading the 1st ESC.


diff --git a/Src/Zle/zle_main.c b/Src/Zle/zle_main.c
index d157e36..38197ce 100644
--- a/Src/Zle/zle_main.c
+++ b/Src/Zle/zle_main.c
@@ -531,6 +531,12 @@ raw_getbyte(long do_keytmout, char *cptr)
 #endif
 
     calc_timeout(&tmout, do_keytmout);
+    {
+	int val;
+	ioctl(SHTTY, FIONREAD, (char *)&val);
+	dputs("T %d %d %d %d %d", do_keytmout, tmout.tp,
+		tmout.tp ? tmout.exp100ths : 0, delayzsetterm, val);
+    }
 
     /*
      * Handle timeouts and watched fd's.  If a watched fd or a function
@@ -635,6 +641,7 @@ raw_getbyte(long do_keytmout, char *cptr)
 	    selret = select(fdmax+1, (SELECT_ARG_2_T) & foofd,
 			    NULL, NULL, tvptr);
 	    winch_block();
+	    dputs("S %d %d",selret,tvptr->tv_usec);
 # endif
 	    /*
 	     * Make sure a user interrupt gets passed on straight away.
@@ -853,6 +860,7 @@ raw_getbyte(long do_keytmout, char *cptr)
     winch_unblock();
     ret = read(SHTTY, cptr, 1);
     winch_block();
+    dputs("R <%c>\n",*cptr);
 
     return ret;
 }

Re: Confirming X02zlevi test failures

[Bart Schaefer]

On Tue, Nov 25, 2014 at 9:32 AM, Jun T. <takimoto-j@kba.biglobe.ne.jp> wrote:
> Sorry for disturbing.

No need to apologize.

> So all the 10 characters (ESC ~ a I ESC ~ o ESC ~ ^X) are already
> available when reading the 1st ESC.

That implies that a simple write() on the master side of the pty
device is not sufficient to cause the input to be received on the
slave side.

I don't presently have any ideas what to do with that knowledge ...

Re: Confirming X02zlevi test failures

[Jun T.]

commit c4110f7f4eac347fdbce71c286659a77beb138f7 added a workaround
to Test/comptest, but for the test to succeed on my Mac the line
zpty_flush After zletest
must also be commented out.
But even this is not enough, as reported below:

On 2014/11/24, at 9:11, Bart Schaefer <schaefer@brasslantern.com> wrote:
> With the patch from 33769 applied, I get about 30% failure rate of X02

Jun

Re: Confirming X02zlevi test failures

[Oliver Kiddle]

"Jun T." wrote:
> commit c4110f7f4eac347fdbce71c286659a77beb138f7 added a workaround
> to Test/comptest, but for the test to succeed on my Mac the line
> zpty_flush After zletest
> must also be commented out.
> But even this is not enough, as reported below:

Sorry, I interpreted 33765 (and my own test results) to indicate that
this was fairly reliable. And I wasn't making much progress on a
non-workaround fix myself.

I'm fairly stuck as far as this issue goes. The test input is clearly
being handled with the default (shttyinfo and ICANON) terminal settings
in force but the terminal settings seem to get set and unset all over
the place.

The delayzsetterm stuff is not related. Digging through the history
shows that the "some systems" on which the workaround was needed are
DG/UX and Ultrix. Any objections to just ripping this out as per the
patch below?

Oliver


diff --git a/Src/Zle/zle_main.c b/Src/Zle/zle_main.c
index a38f55b..caa052b 100644
--- a/Src/Zle/zle_main.c
+++ b/Src/Zle/zle_main.c
@@ -187,10 +187,6 @@ mod_export char *zlenoargs[1] = { NULL };
 
 static char **raw_lp, **raw_rp;
 
-#ifdef FIONREAD
-static int delayzsetterm;
-#endif
-
 /*
  * File descriptors we are watching as well as the terminal fd. 
  * These are all for reading; we don't watch for writes or exceptions.
@@ -210,9 +206,6 @@ mod_export void
 zsetterm(void)
 {
     struct ttyinfo ti;
-#if defined(FIONREAD)
-    int val;
-#endif
 
     if (fetchttyinfo) {
 	/*
@@ -224,30 +217,6 @@ zsetterm(void)
 	fetchttyinfo = 0;
     }
 
-#if defined(FIONREAD)
-    ioctl(SHTTY, FIONREAD, (char *)&val);
-    if (val) {
-	/*
-	 * Problems can occur on some systems when switching from
-	 * canonical to non-canonical input.  The former is usually
-	 * set while running programmes, but the latter is necessary
-	 * for zle.  If there is input in canonical mode, then we
-	 * need to read it without setting up the terminal.  Furthermore,
-	 * while that input gets processed there may be more input
-	 * being typed (i.e. further typeahead).  This means that
-	 * we can't set up the terminal for zle *at all* until
-	 * we are sure there is no more typeahead to come.  So
-	 * if there is typeahead, we set the flag delayzsetterm.
-	 * Then getbyte() calls here to performs another FIONREAD call;
-	 * if that is 0, we have finally used up all the typeahead, and
-	 * it is safe to alter the terminal, which we do at that point.
-	 */
-	delayzsetterm = 1;
-	return;
-    } else
-	delayzsetterm = 0;
-#endif
-
 /* sanitize the tty */
 #ifdef HAS_TIO
     shttyinfo.tio.c_lflag |= ICANON | ECHO;
@@ -343,7 +312,7 @@ zsetterm(void)
 	ti.ltchars.t_dsuspc = ti.ltchars.t_lnextc = -1;
 #endif
 
-#if defined(TTY_NEEDS_DRAINING) && defined(TIOCOUTQ) && defined(HAVE_SELECT)
+#if defined(TIOCOUTQ) && defined(HAVE_SELECT)
     if (baud) {			/**/
 	int n = 0;
 
@@ -541,11 +510,7 @@ raw_getbyte(long do_keytmout, char *cptr)
      * timeouts may be external, so we may have both a permanent watched
      * fd and a long-term timeout.
      */
-    if ((nwatch || tmout.tp != ZTM_NONE)
-#ifdef FIONREAD
-	&& ! delayzsetterm
-#endif
-	) {
+    if ((nwatch || tmout.tp != ZTM_NONE)) {
 #if defined(HAVE_SELECT) || defined(HAVE_POLL)
 	int i, errtry = 0, selret;
 # ifdef HAVE_POLL
@@ -883,10 +848,6 @@ getbyte(long do_keytmout, int *timeout)
     if (kungetct)
 	ret = STOUC(kungetbuf[--kungetct]);
     else {
-#ifdef FIONREAD
-	if (delayzsetterm)
-	    zsetterm();
-#endif
 	for (;;) {
 	    int q = queue_signal_level();
 	    dont_queue_signals();

Re: Confirming X02zlevi test failures

[Peter Stephenson]

On Wed, 26 Nov 2014 15:51:19 +0100
Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
> The delayzsetterm stuff is not related. Digging through the history
> shows that the "some systems" on which the workaround was needed are
> DG/UX and Ultrix. Any objections to just ripping this out as per the
> patch below?

Sounds sensible to me, unless someone can dredge up a modern system like
this.  But the number of people using anything other than freeware Linux
/ BSD variants, or MacOS, or occasionally Solaris (hi, Danek), seems to
have tailed off.

pws

Re: Confirming X02zlevi test failures

[Bart Schaefer]

On Nov 26,  3:45pm, Peter Stephenson wrote:
} Subject: Re: Confirming X02zlevi test failures
}
} On Wed, 26 Nov 2014 15:51:19 +0100
} Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
} > The delayzsetterm stuff is not related. Digging through the history
} > shows that the "some systems" on which the workaround was needed are
} > DG/UX and Ultrix. Any objections to just ripping this out as per the
} > patch below?
} 
} Sounds sensible to me, unless someone can dredge up a modern system like
} this.

It shouldn't be hard to write a test case though it requires a person to
drive it.  Open /dev/tty, set icanon, sleep while the user enters some
typeahead, set -icanon and then see if you can read the typeahead.  Then
try the icanon flop in the opposite order.  It might even be possible to
do it with a script and stty rather than a C program, although it might
also require that all the frobbing and reading use the same (not a dup of
the same) file descriptor.

Re: Confirming X02zlevi test failures

[Oliver Kiddle]

Bart wrote:
> It shouldn't be hard to write a test case though it requires a person to
> drive it.  Open /dev/tty, set icanon, sleep while the user enters some
> typeahead, set -icanon and then see if you can read the typeahead.  Then
> try the icanon flop in the opposite order.  It might even be possible to
> do it with a script and stty rather than a C program, although it might
> also require that all the frobbing and reading use the same (not a dup of
> the same) file descriptor.

I'd have thought simply checking that zsh doesn't lose typeahead would
be sufficient but since I already had a test program that was not far
from what you describe, I've modified and attached it. Is opening
/dev/tty instead of fd 0 necessary? You'll need to manually swap the
icanons. It's probably best to try to get an EOF in before the 3 seconds
have elapsed. Let me know if you see a problem on any system you use.

Oliver

#include <unistd.h>
#include <stdio.h>
#include <errno.h>
#include <termios.h>

void
gettyinfo(struct termios *ti)
{
    if (tcgetattr(0, ti) == -1)
	fprintf(stderr, "bad tcgets: %d", errno);
}

void
settyinfo(struct termios *ti)
{
    while (tcsetattr(0, TCSADRAIN, ti) == -1 && errno == EINTR)
	    ;
}

int main() {
    struct termios ti;
    char cptr[10];
    ssize_t size;

    gettyinfo(&ti);
    ti.c_lflag &= ~ICANON;
    settyinfo(&ti);
    sleep(3);
    ti.c_lflag |= ICANON;
    settyinfo(&ti);

    size = read(0, cptr, 9);
    if (size > 0) {
	cptr[size] = '\0';
	printf("\nGot: %s\n", cptr);
    }
}

Re: Confirming X02zlevi test failures

[Jun T.]

It seems fixing the zpty problem in BSD/Darwin is not easy.

If we must resort to a (hopefully temporary) workaround, 
I think 'bindkey -r "\e~"' is better than zpty_flush()
because the latter seems to be not 100% reliable.

In the patch below, instead of putting 'bindkey -r "\e~"'
in comptest, it is called just before the failing test in
X02zlevi, in order to minimize the interference with other
tests which are also using comptest.

Jun


diff --git a/Test/X02zlevi.ztst b/Test/X02zlevi.ztst
index 8f93902..0c346a4 100644
--- a/Test/X02zlevi.ztst
+++ b/Test/X02zlevi.ztst
@@ -37,6 +37,7 @@
 >3
 >CURSOR: 5
 
+  zpty_run 'bindkey -r "\e~"'
   zletest $'\e' $'~aI\e' $'~o\e' \~
 0:swap case on a blank line
 >BUFFER: i
diff --git a/Test/comptest b/Test/comptest
index 1f4dac6..9c92f96 100644
--- a/Test/comptest
+++ b/Test/comptest
@@ -162,7 +162,7 @@ comptest () {
 zletest () {
   local first=0
   for input; do
-    zpty_flush Before zletest
+    # zpty_flush Before zletest
     # sleep for $KEYTIMEOUT
     (( first++ )) && read -t 0.011 -k 1 < /dev/null
     zpty -n -w zsh "$input"

Re: Confirming X02zlevi test failures

[Oliver Kiddle]

On 1 Dec, "Jun T." wrote:
> It seems fixing the zpty problem in BSD/Darwin is not easy.

Yes, I'm not able to get any further on it.

> If we must resort to a (hopefully temporary) workaround, 
> I think 'bindkey -r "\e~"' is better than zpty_flush()
> because the latter seems to be not 100% reliable.

I'm happy if you want to apply this by the way.

Oliver