Make sure to read until EOF from subprocesses

src/plugin/plugin_process_handler.cpp

@@ -43,6 +43,7 @@ namespace linuxdeploy {
                 std::string stream_name_;
                 ldLog log_;
                 bool print_prefix_in_next_iteration_;
+                bool eof = false;
 
                 pipe_to_be_logged(int pipe_fd, std::string stream_name) : reader_(pipe_fd),
                                                                           stream_name_(std::move(stream_name)),
@@ -62,12 +63,17 @@ namespace linuxdeploy {
                     // since we have our own ldLog instance for every pipe, we can get away with this rather small read buffer
                     subprocess::subprocess_result_buffer_t intermediate_buffer(4096);
 
+                    if (pipe_to_be_logged.eof) {
+                        break;
+                    }
+
                     // (try to) read from pipe
                     const auto bytes_read = pipe_to_be_logged.reader_.read(intermediate_buffer);
 
-                    // no action required in case we have not read anything from the pipe
-                    if (bytes_read <= 0) {
-                        continue;
+                    // 0 means EOF
+                    if (bytes_read == 0) {
+                        pipe_to_be_logged.eof = true;
+                        break;
                     }
 
                     // we just trim the buffer to the bytes we read (makes the code below easier)
@@ -119,7 +125,12 @@ namespace linuxdeploy {
                 if (proc.is_running()) {
                     // reduce load on CPU
                     std::this_thread::sleep_for(std::chrono::milliseconds(50));
-                } else {
+                }
+
+                // once all buffers are EOF, we can stop reading
+                if (std::all_of(pipes_to_be_logged.begin(), pipes_to_be_logged.end(), [](const pipe_to_be_logged& pipe_state) {
+                    return pipe_state.eof;
+                })) {
                     break;
                 }
             }

src/subprocess/pipe_reader.cpp

@@ -17,16 +17,21 @@ pipe_reader::pipe_reader(int pipe_fd) : pipe_fd_(pipe_fd) {
 }
 
 size_t pipe_reader::read(std::vector<std::string::value_type>& buffer) const {
-    ssize_t rv = ::read(pipe_fd_, buffer.data(), buffer.size());
+    for (;;) {
+        ssize_t rv = ::read(pipe_fd_, buffer.data(), buffer.size());
 
-    if (rv == -1) {
-        // no data available
-        if (errno == EAGAIN)
-            return 0;
+        if (rv == -1) {
+            switch (errno) {
+                // retry in case data is currently not available
+                case EINTR:
+                case EAGAIN:
+                    continue;
+                default:
+                    // TODO: introduce custom subprocess_error
+                    throw std::runtime_error{"unexpected error reading from pipe: " + std::string(strerror(errno))};
+            }
+        }
 
-        // TODO: introduce custom subprocess_error
-        throw std::runtime_error{"unexpected error reading from pipe: " + std::string(strerror(errno))};
+        return rv;
     }
-
-    return rv;
 }

src/subprocess/subprocess.cpp

@@ -27,55 +27,70 @@ namespace linuxdeploy {
         subprocess_result subprocess::run() const {
             process proc{args_, env_};
 
+            class PipeState {
+            public:
+                pipe_reader reader;
+                subprocess_result_buffer_t  buffer;
+                bool eof = false;
+
+                explicit PipeState(int fd) : reader(fd) {}
+            };
+
             // create pipe readers and empty buffers for both stdout and stderr
             // we manage them in this (admittedly, kind of complex-looking) array so we can later easily perform the
             // operations in a loop
-            std::array<std::pair<pipe_reader, subprocess_result_buffer_t>, 2> buffers{
-                std::make_pair(pipe_reader(proc.stdout_fd()), subprocess_result_buffer_t{}),
-                std::make_pair(pipe_reader(proc.stderr_fd()), subprocess_result_buffer_t{}),
+            std::array<PipeState, 2> buffers = {
+                PipeState(proc.stdout_fd()),
+                PipeState(proc.stderr_fd())
             };
 
             for (;;) {
-                for (auto& pair : buffers) {
-                    // make code more readable
-                    auto& reader = pair.first;
-                    auto& buffer = pair.second;
-
+                for (auto& pipe_state : buffers) {
                     // read some bytes into smaller intermediate buffer to prevent either of the pipes to overflow
                     // the results are immediately appended to the main buffer
                     subprocess_result_buffer_t intermediate_buffer(4096);
 
                     // (try to) read all available data from pipe
                     for (;;) {
-                        const auto bytes_read = reader.read(intermediate_buffer);
+                        if (pipe_state.eof) {
+                            break;
+                        }
 
+                        const auto bytes_read = pipe_state.reader.read(intermediate_buffer);
+
+                        // 0 means EOF
                         if (bytes_read == 0) {
+                            pipe_state.eof = true;
                             break;
                         }
 
                         // append to main buffer
-                        buffer.reserve(buffer.size() + bytes_read);
+                        pipe_state.buffer.reserve(pipe_state.buffer.size() + bytes_read);
                         std::copy(intermediate_buffer.begin(), (intermediate_buffer.begin() + bytes_read),
-                                  std::back_inserter(buffer));
+                                  std::back_inserter(pipe_state.buffer));
                     }
                 }
 
-                // do-while might be a little more elegant, but we can save this one unnecessary sleep, so...
                 if (proc.is_running()) {
-                    // reduce load on CPU
+                    // reduce load on CPU until EOF
                     std::this_thread::sleep_for(std::chrono::milliseconds(50));
-                } else {
+                }
+
+                // once all buffers are EOF, we can stop reading
+                if (std::all_of(buffers.begin(), buffers.end(), [](const PipeState& pipe_state) {
+                    return pipe_state.eof;
+                })) {
                     break;
                 }
             }
 
             // make sure contents are null-terminated
-            buffers[0].second.emplace_back('\0');
-            buffers[1].second.emplace_back('\0');
+            buffers[0].buffer.emplace_back('\0');
+            buffers[1].buffer.emplace_back('\0');
 
             auto exit_code = proc.close();
 
-            return subprocess_result{exit_code, buffers[0].second, buffers[1].second};
+            return subprocess_result{exit_code, buffers[0].buffer, buffers[1].buffer};
         }
 
         std::string subprocess::check_output() const {