Re: Test hang in Y01

[Peter Stephenson <pws@xxxxxxx>]

On Sun, 11 Jan 2009 21:25:15 -0500
"Vin Shelton" <acs@xxxxxxxxxxxxxxxxxxxx> wrote:
> It turns out that for me, the enabling factor is --enable-zsh-mem; if
> I configure with that, I get the test hang.  Without that option, Y01
> completes without a hitch.

It appeared to need both this and debugging active.  I think the underlying
failure is a crash in completion, which I can provoke very readily after
running compinit.  It would be great if someone else could do some prodding
to locate the memory error in question---I'm not going to be able to do
this at home as my home machine gave up on --enable-zsh-mem some time ago.

Unfortunately, my machine here (Fedora 8, reasonably up to date as far as
that goes) has stopped being able to read symbols from .so files and
instead issues meaningless complaints along the lines of

Missing separate debuginfo for /home/pws/src/zsh-debug/zsh/modules/zsh/zle.so
Try: yum --enablerepo='*-debuginfo' install /usr/lib/debug/.build-id/25/f01fa65e4c2fc031c9f53901f46e3f3b654c31

Clearly the message at least is Fedora-specific.  Anyone know what to do
about this?  (If switching to Debian or Ubuntu works I'm starting seriously
to consider it...)

The following patch merely makes the test symptoms more sensible.  I hope
this will allow someone to automate the search for the cause.

First, the infinite loop: the read in "zpty -r" was, as I expected, getting
an error, actually EIO.  However, it wasn't a race; the subshell had simply
gone away.  I think it makes no sense to continue the loop in ptyread()
unless the error is one of the ones we handle specially afterward, EAGAIN
and EWOULDBLOCK (possibly you can argue for EINTR, please do if you want).
Otherwise we should return at that point.

Second, "zpty -r" is designed and documented to return status 0 if it read
as far as it could and successfully input something, even if the pattern
it's been passed didn't match.  This seems a pretty odd piece of design
to me, but as it's apparently supposed to do that I've added a must match
flag, -m, and used that in the test system where we want to be quite sure
we've read what we're expecting.

Now the test fails, but more gracefully.

Index: Doc/Zsh/mod_zpty.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/mod_zpty.yo,v
retrieving revision 1.7
diff -u -r1.7 mod_zpty.yo
--- Doc/Zsh/mod_zpty.yo	25 Jan 2008 16:48:23 -0000	1.7
+++ Doc/Zsh/mod_zpty.yo	13 Jan 2009 11:44:24 -0000
@@ -38,7 +38,7 @@
 were typed, so beware when sending special tty driver characters such as
 word-erase, line-kill, and end-of-file.
 )
-item(tt(zpty) tt(-r) [ tt(-t) ] var(name) [ var(param) [ var(pattern) ] ])(
+item(tt(zpty) tt(-r) [ tt(-mt) ] var(name) [ var(param) [ var(pattern) ] ])(
 The tt(-r) option can be used to read the output of the command var(name).
 With only a var(name) argument, the output read is copied to the standard
 output.  Unless the pseudo-terminal is non-blocking, copying continues
@@ -54,10 +54,11 @@
 If a var(pattern) is given as well, output is read until the whole string
 read matches the var(pattern), even in the non-blocking case.  The return
 status is zero if the string read matches the pattern, or if the command
-has exited but at least one character could still be read.  As of this
-writing, a maximum of one megabyte of output can be consumed this way; if
-a full megabyte is read without matching the pattern, the return status is
-non-zero.
+has exited but at least one character could still be read.  If the option
+tt(-m) is present, the return status is zero only if the pattern matches.
+As of this writing, a maximum of one megabyte of output can be consumed
+this way; if a full megabyte is read without matching the pattern, the
+return status is non-zero.
 
 In all cases, the return status is non-zero if nothing could be read, and
 is tt(2) if this is because the command has finished.
Index: Src/Modules/zpty.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Modules/zpty.c,v
retrieving revision 1.40
diff -u -r1.40 zpty.c
--- Src/Modules/zpty.c	18 Nov 2008 10:14:35 -0000	1.40
+++ Src/Modules/zpty.c	13 Jan 2009 11:44:24 -0000
@@ -485,9 +485,9 @@
 }
 
 static int
-ptyread(char *nam, Ptycmd cmd, char **args, int noblock)
+ptyread(char *nam, Ptycmd cmd, char **args, int noblock, int mustmatch)
 {
-    int blen, used, seen = 0, ret = 0;
+    int blen, used, seen = 0, ret = 0, matchok = 0;
     char *buf;
     Patprog prog = NULL;
 
@@ -589,10 +589,24 @@
 	}
 	buf[used] = '\0';
 
-	if (!prog && (ret <= 0 || (*args && buf[used - 1] == '\n')))
-	    break;
+	if (!prog) {
+	    if (ret <= 0 || (*args && buf[used - 1] == '\n'))
+		break;
+	} else {
+	    if (ret < 0
+#ifdef EWOULDBLOCK
+		&& errno != EWOULDBLOCK
+#else
+#ifdef EAGAIN
+		&& errno != EAGAIN
+#endif
+#endif
+		)
+		break;
+	}
     } while (!(errflag || breaks || retflag || contflag) &&
-	     used < READ_MAX && !(prog && ret && pattry(prog, buf)));
+	     used < READ_MAX &&
+	     !(prog && ret && (matchok = pattry(prog, buf))));
 
     if (prog && ret < 0 &&
 #ifdef EWOULDBLOCK
@@ -613,7 +627,9 @@
     else if (used)
 	write(1, buf, used);
 
-    return (seen ? 0 : cmd->fin + 1);
+    if (seen && (!prog || matchok || !mustmatch))
+	return 0;
+    return cmd->fin + 1;
 }
 
 static int
@@ -679,16 +695,19 @@
 bin_zpty(char *nam, char **args, Options ops, UNUSED(int func))
 {
     if ((OPT_ISSET(ops,'r') && OPT_ISSET(ops,'w')) ||
-	((OPT_ISSET(ops,'r') || OPT_ISSET(ops,'w')) && 
+	((OPT_ISSET(ops,'r') || OPT_ISSET(ops,'w')) &&
 	 (OPT_ISSET(ops,'d') || OPT_ISSET(ops,'e') ||
 	  OPT_ISSET(ops,'b') || OPT_ISSET(ops,'L'))) ||
-	(OPT_ISSET(ops,'w') && OPT_ISSET(ops,'t')) ||
+	(OPT_ISSET(ops,'w') && (OPT_ISSET(ops,'t') || OPT_ISSET(ops,'m'))) ||
 	(OPT_ISSET(ops,'n') && (OPT_ISSET(ops,'b') || OPT_ISSET(ops,'e') ||
 				OPT_ISSET(ops,'r') || OPT_ISSET(ops,'t') ||
-				OPT_ISSET(ops,'d') || OPT_ISSET(ops,'L'))) ||
+				OPT_ISSET(ops,'d') || OPT_ISSET(ops,'L') ||
+				OPT_ISSET(ops,'m'))) ||
 	(OPT_ISSET(ops,'d') && (OPT_ISSET(ops,'b') || OPT_ISSET(ops,'e') ||
-				OPT_ISSET(ops,'L') || OPT_ISSET(ops,'t'))) ||
-	(OPT_ISSET(ops,'L') && (OPT_ISSET(ops,'b') || OPT_ISSET(ops,'e')))) {
+				OPT_ISSET(ops,'L') || OPT_ISSET(ops,'t') ||
+				OPT_ISSET(ops,'m'))) ||
+	(OPT_ISSET(ops,'L') && (OPT_ISSET(ops,'b') || OPT_ISSET(ops,'e') ||
+				OPT_ISSET(ops,'m')))) {
 	zwarnnam(nam, "illegal option combination");
 	return 1;
     }
@@ -706,7 +725,8 @@
 	    return 2;
 
 	return (OPT_ISSET(ops,'r') ?
-		ptyread(nam, p, args + 1, OPT_ISSET(ops,'t')) :
+		ptyread(nam, p, args + 1, OPT_ISSET(ops,'t'),
+			OPT_ISSET(ops, 'm')) :
 		ptywrite(p, args + 1, OPT_ISSET(ops,'n')));
     } else if (OPT_ISSET(ops,'d')) {
 	Ptycmd p;
@@ -780,7 +800,7 @@
 
 
 static struct builtin bintab[] = {
-    BUILTIN("zpty", 0, bin_zpty, 0, -1, 0, "ebdrwLnt", NULL),
+    BUILTIN("zpty", 0, bin_zpty, 0, -1, 0, "ebdmrwLnt", NULL),
 };
 
 static struct features module_features = {
Index: Test/comptest
===================================================================
RCS file: /cvsroot/zsh/zsh/Test/comptest,v
retrieving revision 1.17
diff -u -r1.17 comptest
--- Test/comptest	12 Oct 2008 19:20:01 -0000	1.17
+++ Test/comptest	13 Jan 2009 11:44:24 -0000
@@ -80,7 +80,7 @@
 
   print -lr - "$@" > $tmp
   zpty -w zsh ". $tmp"
-  zpty -r zsh log_eval "*<PROMPT>*" || {
+  zpty -r -m zsh log_eval "*<PROMPT>*" || {
     print "prompt hasn't appeared."
     return 1
   }
@@ -90,7 +90,7 @@
 comptest () {
   input="$*"
   zpty -n -w zsh "$input"$'\C-Z'
-  zpty -r zsh log "*<WIDGET><finish>*<PROMPT>*" || {
+  zpty -r -m zsh log "*<WIDGET><finish>*<PROMPT>*" || {
     print "failed to invoke finish widget."
     return 1
   }


-- 
Peter Stephenson <pws@xxxxxxx>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070