Re: zpty non-functional?

[Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx>]

On Sun, 25 Aug 2013 15:01:04 -0700
Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> wrote:
> This leads me to conclude that it's a race condition:  The master writer
> may read back what it wrote, up until the point that there is a process
> on the slave side consuming it instead.

That all sounds entirely convincing, thanks for the work.
 
> Whether this means we want to copy the synch[] pipe hack from exec.c to
> prevent zpty from returning before the child process is running, or just
> document the possible race, I will leave up to consensus (or PWS).

Here's a synch, as well as a test that was showing up the problem
(though not every time).  I've tried to catch as many funny cases for
read/write as I could think of; this may be overkill but should be at
worst harmless (unless there's still something I've missed).

diff --git a/Src/Modules/zpty.c b/Src/Modules/zpty.c
index 3821194..fca0cc1 100644
--- a/Src/Modules/zpty.c
+++ b/Src/Modules/zpty.c
@@ -293,8 +293,8 @@ static int
 newptycmd(char *nam, char *pname, char **args, int echo, int nblock)
 {
     Ptycmd p;
-    int master, slave, pid, oineval = ineval;
-    char *oscriptname = scriptname;
+    int master, slave, pid, oineval = ineval, ret;
+    char *oscriptname = scriptname, syncch;
     Eprog prog;
 
     /* code borrowed from bin_eval() */
@@ -398,6 +398,20 @@ newptycmd(char *nam, char *pname, char **args, int echo, int nblock)
 	setsparam("TTY", ztrdup(ttystrname));
 
 	opts[INTERACTIVE] = 0;
+
+	syncch = 0;
+	do {
+	    ret = write(1, &syncch, 1);
+	} while (ret != 1 && (
+#ifdef EWOULDBLOCK
+	    errno == EWOULDBLOCK ||
+#else
+#ifdef EAGAIN
+	    errno == EAGAIN ||
+#endif
+#endif
+	    errno == EINTR));
+
 	execode(prog, 1, 0, "zpty");
 	stopmsg = 2;
 	mypid = 0; /* trick to ensure we _exit() */
@@ -433,6 +447,18 @@ newptycmd(char *nam, char *pname, char **args, int echo, int nblock)
     scriptname = oscriptname;
     ineval = oineval;
 
+    do {
+	ret = read(master, &syncch, 1);
+    } while (ret != 1 && (
+#ifdef EWOULDBLOCK
+	    errno == EWOULDBLOCK ||
+#else
+#ifdef EAGAIN
+	    errno == EAGAIN ||
+#endif
+#endif
+	    errno == EINTR));
+
     return 0;
 }
 
diff --git a/Test/V08zpty.ztst b/Test/V08zpty.ztst
index d9d24c5..5b08fc2 100644
--- a/Test/V08zpty.ztst
+++ b/Test/V08zpty.ztst
@@ -12,9 +12,17 @@
 %test
 
   zpty cat cat
+  zpty -w cat a line of text
+  var=
+  zpty -r cat var && print -r -- ${var%%$'\r\n'}
+  zpty -d cat
+0:zpty with a process that does not set up the terminal: internal write
+>a line of text
+
+  zpty cat cat
   print a line of text | zpty -w cat
   var=
   zpty -r cat var && print -r -- ${var%%$'\r\n'}
   zpty -d cat
-0:zpty with a process that does not set up the terminal
+0:zpty with a process that does not set up the terminal: write via stdin
 >a line of text

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