Fix case of process termination before readiness notification.

Adds a test.

diff --git a/src/proc-service.cc b/src/proc-service.cc
index d46a50d13fe07b3f802c5dc442596aebe483e114..9cb94cdcde60a0278e04a8247114a5bea6df44e4 100644 (file)
--- a/src/proc-service.cc
+++ b/src/proc-service.cc
@@ -119,9 +119,12 @@ rearm ready_notify_watcher::fd_event(eventloop_t &, int fd, int flags) noexcept
         }
         else if (r == 0 || errno != EAGAIN) {
             service->failed_to_start(false, false);
+            service->set_state(service_state_t::STOPPING);
+            service->bring_down();
         }
     }
     else {
+        // Just keep consuming data from the pipe:
         int r = bp_sys::read(fd, buf, sizeof(buf));
         if (r == 0) {
             // Process closed write end or terminated
@@ -176,7 +179,7 @@ void process_service::handle_exit_status(bp_sys::exit_status exit_status) noexce
         notification_fd = -1;
     }
 
-    if (exit_status.did_exit_clean() && service_state != service_state_t::STOPPING) {
+    if (!exit_status.did_exit_clean() && service_state != service_state_t::STOPPING) {
         if (did_exit) {
             log(loglevel_t::ERROR, "Service ", get_name(), " process terminated with exit code ",
                     exit_status.get_exit_status());
@@ -188,13 +191,10 @@ void process_service::handle_exit_status(bp_sys::exit_status exit_status) noexce
     }
 
     if (service_state == service_state_t::STARTING) {
-        if (exit_status.did_exit_clean()) {
-            started();
-        }
-        else {
-            stop_reason = stopped_reason_t::FAILED;
-            failed_to_start();
-        }
+        // If state is STARTING, we must be waiting for readiness notification; the process has
+        // terminated before becoming ready.
+        stop_reason = stopped_reason_t::FAILED;
+        failed_to_start();
     }
     else if (service_state == service_state_t::STOPPING) {
         // We won't log a non-zero exit status or termination due to signal here -
@@ -219,6 +219,13 @@ void process_service::handle_exit_status(bp_sys::exit_status exit_status) noexce
 void process_service::exec_failed(int errcode) noexcept
 {
     log(loglevel_t::ERROR, get_name(), ": execution failed: ", strerror(errcode));
+
+    if (notification_fd != -1) {
+        readiness_watcher.deregister(event_loop);
+        bp_sys::close(notification_fd);
+        notification_fd = -1;
+    }
+
     if (get_state() == service_state_t::STARTING) {
         stop_reason = stopped_reason_t::EXECFAILED;
         failed_to_start();

diff --git a/src/tests/proctests.cc b/src/tests/proctests.cc
index c3d245c884a6d6cbf82b80c4fdebc95821864c9c..a4484d95573d1805fb3058ed596ee6e23a6853d3 100644 (file)
--- a/src/tests/proctests.cc
+++ b/src/tests/proctests.cc
@@ -371,6 +371,50 @@ void test_proc_start_execfail()
     sset.remove_service(&p);
 }
 
+// Test no ready notification before process terminates
+void test_proc_notify_fail()
+{
+    using namespace std;
+
+    service_set sset;
+
+    string command = "test-command";
+    list<pair<unsigned,unsigned>> command_offsets;
+    command_offsets.emplace_back(0, command.length());
+    std::list<prelim_dep> depends;
+
+    process_service p {&sset, "testproc", std::move(command), command_offsets, depends};
+    init_service_defaults(p);
+    p.set_notification_fd(3);
+    sset.add_service(&p);
+
+    p.start(true);
+    sset.process_queues();
+
+    assert(p.get_state() == service_state_t::STARTING);
+
+    base_process_service_test::exec_succeeded(&p);
+    sset.process_queues();
+
+    assert(p.get_state() == service_state_t::STARTING);
+
+    int nfd = base_process_service_test::get_notification_fd(&p);
+    assert(nfd > 0);
+
+    // Signal EOF on notify fd:
+    event_loop.regd_fd_watchers[nfd]->fd_event(event_loop, nfd, dasynq::IN_EVENTS);
+
+    assert(p.get_state() == service_state_t::STOPPING);
+
+    base_process_service_test::handle_exit(&p, 0);
+    sset.process_queues();
+
+    assert(p.get_state() == service_state_t::STOPPED);
+    assert(event_loop.active_timers.size() == 0);
+
+    sset.remove_service(&p);
+}
+
 // Test stop timeout
 void test_proc_stop_timeout()
 {
@@ -853,6 +897,7 @@ int main(int argc, char **argv)
     RUN_TEST(test_proc_start_timeout, "   ");
     RUN_TEST(test_proc_start_timeout2, "  ");
     RUN_TEST(test_proc_start_execfail, "  ");
+    RUN_TEST(test_proc_notify_fail, "     ");
     RUN_TEST(test_proc_stop_timeout, "    ");
     RUN_TEST(test_proc_smooth_recovery1, "");
     RUN_TEST(test_proc_smooth_recovery2, "");