From ff8493ef9d33e7c348293587637c70f82bb90aba Mon Sep 17 00:00:00 2001
From: Laurent Bercot <ska-skaware@skarnet.org>
Date: Fri, 19 Dec 2014 00:25:16 +0000
Subject:  s6-notifywhen up now sends U and exits on the first newline.  Doc
 updated to reflect it.

---
 doc/notifywhenup.html                    | 18 +++++++++++++-----
 doc/s6-notifywhenup.html                 |  6 +++---
 src/daemontools-extras/s6-notifywhenup.c | 21 ++++++++++-----------
 3 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/doc/notifywhenup.html b/doc/notifywhenup.html
index 40b0593..f73e2aa 100644
--- a/doc/notifywhenup.html
+++ b/doc/notifywhenup.html
@@ -54,21 +54,29 @@ is reliably up - because only they know when it is the case.
  <li> Daemons can use the <tt>ftrigw_notify()</tt> function, provided in
 <a href="libftrigw.html">the ftrigw library</a>. This is extremely
 simple and efficient, but requires specific s6 support in the daemon. </li>
- <li> Daemons can write something to a file descriptor of their choice,
+ <li> Daemons can write a line to a file descriptor of their choice,
 then close that file descriptor, when they're ready to serve. This is
 a generic mechanism that some daemons already implement, and does not
 require anything specific in the daemon's code. The administrator can
 then run the daemon under <a href="s6-notifywhenup.html">s6-notifywhenup</a>,
 which will properly catch the daemon's message and notify all the subscribers
-with a 'U' event, meaning that the service is now up with no possible race
-condition. </li>
+with a 'U' event, meaning that the service is now up. <br /> <br />
+ Note that there is <em>still</em> a small race condition remaining:
+if the daemon writes a line then instantly dies, and the supervisor
+picks up the death before the <a href="s6-notifywhenup.html">s6-notifywhenup</a>
+program picks up the line, it is possible for the event sequence written
+to the fifodir to be wrong - 'd' before 'U'. This should be extremely
+rare, but unfortunately the race condition is unavoidable. The only
+way to be absolutely race-free is to have the daemon perform its
+readiness notification itself, which requires specific support.
+ </li>
 </ol>
 
 <p>
  The second method should really be implemented in every long-running
 program providing a service. When it is not the case, it's impossible
-to provide race-free startup notifications, and subscribers should be
-content with the unreliable 'u' events provided by s6-supervise.
+to provide reliable startup notifications, and subscribers should then
+be content with the unreliable 'u' events provided by s6-supervise.
 </p>
 
 </body>
diff --git a/doc/s6-notifywhenup.html b/doc/s6-notifywhenup.html
index ad7ef8e..f192ca4 100644
--- a/doc/s6-notifywhenup.html
+++ b/doc/s6-notifywhenup.html
@@ -37,9 +37,9 @@ needed.
 <ul>
  <li> s6-notifywhenup forks and executes <em>prog...</em> as the
 parent, with a pipe from <em>prog...</em>'s stdout to the child. </li>
- <li> The child waits for EOF on the pipe. When it gets it, if there
-have been other characters written before the EOF, it sends a 'U'
-event to the <tt>./event</tt> fifodir. </li>
+ <li> The child waits for a newline (<tt>\n</tt>) to be written
+on the pipe. When it gets it, it sends a 'U' event to the
+<tt>./event</tt> fifodir. </li>
  <li> The child exits 0. </li>
 </ul>
 
diff --git a/src/daemontools-extras/s6-notifywhenup.c b/src/daemontools-extras/s6-notifywhenup.c
index 5c151ed..583b784 100644
--- a/src/daemontools-extras/s6-notifywhenup.c
+++ b/src/daemontools-extras/s6-notifywhenup.c
@@ -3,6 +3,7 @@
 #include <unistd.h>
 #include <errno.h>
 #include <skalibs/uint.h>
+#include <skalibs/bytestr.h>
 #include <skalibs/sgetopt.h>
 #include <skalibs/strerr2.h>
 #include <skalibs/allreadwrite.h>
@@ -19,25 +20,23 @@ static int run_child (int fd, char const *fifodir, unsigned int timeout)
   char dummy[4096] ;
   iopause_fd x = { .fd = fd, .events = IOPAUSE_READ } ;
   tain_t deadline ;
-  int haswritten = 0 ;
-  register int r = 0 ;
   if (!tain_now_g()) strerr_diefu1sys(111, "tain_now") ;
   if (timeout) tain_from_millisecs(&deadline, timeout) ;
   else deadline = tain_infinite_relative ;
   tain_add_g(&deadline, &deadline) ;
-  while (!r)
+  for (;;)
   {
-    r = iopause_g(&x, 1, &deadline) ;
+    register int r = iopause_g(&x, 1, &deadline) ;
     if (r < 0) strerr_diefu1sys(111, "iopause") ;
     if (!r) return 99 ;
-    while (r > 0)
-    {
-      r = sanitize_read(fd_read(fd, dummy, 4096)) ; /* talk to the hand */
-      if (r > 0) haswritten = 1 ;
-    }
+    r = sanitize_read(fd_read(fd, dummy, 4096)) ;
+    if (r < 0)
+      if (errno == EPIPE) return 0 ;
+      else strerr_diefu1sys(111, "read from parent") ;
+    else if (r)
+      if (byte_chr(dummy, r, '\n') < r) break ;
   }
-  if (errno != EPIPE) strerr_diefu1sys(111, "read from parent") ;
-  if (haswritten) ftrigw_notify(fifodir, 'U') ;
+  ftrigw_notify(fifodir, 'U') ;
   return 0 ;
 }
 
-- 
cgit v1.3.1