From ff42dae7cc38eb8756d881aa1e0a719310fcd036 Mon Sep 17 00:00:00 2001
From: Oliver Giles <ohw.giles@gmail.com>
Date: Sat, 9 Dec 2017 20:27:08 +0200
Subject: [PATCH] read remaining data in run pipe when reaping

This fixes a bug where the last pieces of console output were lost. In that
case, the event loop scheduled the ended child process's SIGCHLD handler
before the handler to read the last of the process's output. We work around
that by doing an additional (non-blocking) read in the SIGCHLD handler
---
 src/laminar.cpp | 26 +++++++++++++++++++-------
 src/laminar.h   |  1 +
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/src/laminar.cpp b/src/laminar.cpp
index cb8f9d9..0c18f31 100644
--- a/src/laminar.cpp
+++ b/src/laminar.cpp
@@ -505,27 +505,39 @@ std::shared_ptr<Run> Laminar::queueJob(std::string name, ParamMap params) {
 bool Laminar::stepRun(std::shared_ptr<Run> run) {
     bool complete = run->step();
     if(!complete) {
-        srv->addDescriptor(run->fd, [=](const char* b,size_t n){
-            std::string s(b,n);
-            run->log += s;
-            for(LaminarClient* c : clients) {
-                if(c->scope.wantsLog(run->name, run->build))
-                    c->sendMessage(s);
-            }
+        srv->addDescriptor(run->fd, [this, run](const char* b,size_t n){
+            handleRunLog(run, std::string(b,n));
         });
     }
     return complete;
 }
 
+void Laminar::handleRunLog(std::shared_ptr<Run> run, std::string s) {
+    run->log += s;
+    for(LaminarClient* c : clients) {
+        if(c->scope.wantsLog(run->name, run->build))
+            c->sendMessage(s);
+    }
+}
+
 // Reaps a zombie and steps the corresponding Run to its next state.
 // Should be called on SIGCHLD
 void Laminar::reapAdvance() {
     int ret = 0;
     pid_t pid;
+    static thread_local char buf[1024];
     while((pid = waitpid(-1, &ret, WNOHANG)) > 0) {
         LLOG(INFO, "Reaping", pid);
         auto it = activeJobs.get<0>().find(pid);
         std::shared_ptr<Run> run = *it;
+        // The main event loop might schedule this SIGCHLD handler before the final
+        // output handler (from addDescriptor). In that case, because it keeps a
+        // shared_ptr to the run it would successfully add to the log output buffer,
+        // but by then reapAdvance would have stored the log and ensured no-one cares.
+        // Preempt this case by forcing a final (non-blocking) read here.
+        for(ssize_t n = read(run->fd, buf, 1024); n > 0; n = read(run->fd, buf, 1024)) {
+            handleRunLog(run, std::string(buf, n));
+        }
         bool completed = true;
         activeJobs.get<0>().modify(it, [&](std::shared_ptr<Run> run){
             run->reaped(ret);
diff --git a/src/laminar.h b/src/laminar.h
index 28113ec..4419d7a 100644
--- a/src/laminar.h
+++ b/src/laminar.h
@@ -62,6 +62,7 @@ private:
     void reapAdvance();
     void assignNewJobs();
     bool stepRun(std::shared_ptr<Run> run);
+    void handleRunLog(std::shared_ptr<Run> run, std::string log);
     void runFinished(Run*);
     bool nodeCanQueue(const Node&, const Run&) const;
     // expects that Json has started an array