From d48b25fec7303580c573e688820514844e5e0e39 Mon Sep 17 00:00:00 2001
From: Davin McCall <davmac@davmac.org>
Date: Sun, 29 Dec 2019 14:16:12 +1000
Subject: [PATCH] Mark main log closed on write failure

Record the main log as being closed (external_log_open false) if there
is a write failure while trying to write the log buffer. This allows the
log to be re-opened by restarting the logging service.

Also we avoid waiting for the log buffer to flush if it has failed.
---
 src/dinit-log.cc                | 29 +++++++++++++++++++++++++----
 src/dinit.cc                    |  2 +-
 src/tests/test-dinit.cc         |  1 +
 src/tests/test-includes/dinit.h |  2 ++
 4 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/src/dinit-log.cc b/src/dinit-log.cc
index 89c9f05..9d86913 100644
--- a/src/dinit-log.cc
+++ b/src/dinit-log.cc
@@ -25,6 +25,7 @@
 // message (if any) and then assigns the console to a waiting service.
 
 extern eventloop_t event_loop;
+extern bool external_log_open;
 
 static bool log_current_line[2];  // Whether the current line is being logged (for console, main log)
 static bool log_format_syslog[2] = { false, true };
@@ -46,7 +47,7 @@ class buffered_log_stream : public eventloop_t::fd_watcher_impl<buffered_log_str
     // Outgoing:
     bool partway = false;     // if we are partway throught output of a log message
     bool discarded = false;   // if we have discarded a message
-    bool release = true;      // if we should inhibit output and release console
+    bool release = true;      // if we should inhibit output and release console when possible
 
     // A "special message" is not stored in the circular buffer; instead
     // it is delivered from an external buffer not managed by BufferedLogger.
@@ -113,14 +114,15 @@ class buffered_log_stream : public eventloop_t::fd_watcher_impl<buffered_log_str
         discarded = true;
     }
 
+    void watch_removed() noexcept override;
+
     private:
     void release_console();
 };
-}
 
 // Two log streams:
 // (One for main log, one for console)
-static buffered_log_stream log_stream[2];
+buffered_log_stream log_stream[2];
 
 void buffered_log_stream::release_console()
 {
@@ -142,10 +144,14 @@ void buffered_log_stream::flush_for_release()
     
     // Try to flush any messages that are currently buffered. (Console is non-blocking
     // so it will fail gracefully).
-    if (fd_event(event_loop, fd, dasynq::OUT_EVENTS) == rearm::DISARM) {
+    rearm rearm_val = fd_event(event_loop, fd, dasynq::OUT_EVENTS);
+    if (rearm_val == rearm::DISARM) {
         // Console has already been released at this point.
         set_enabled(event_loop, false);
     }
+    if (rearm_val == rearm::REMOVE) {
+        deregister(event_loop);
+    }
     // fd_event didn't want to disarm, so must be partway through a message; will
     // release when it's finished.
 }
@@ -258,6 +264,21 @@ rearm buffered_log_stream::fd_event(eventloop_t &loop, int fd, int flags) noexce
     return rearm::REARM;
 }
 
+void buffered_log_stream::watch_removed() noexcept
+{
+    if (fd > STDERR_FILENO) {
+        close(fd);
+        fd = -1;
+    }
+    // Here we rely on there only being two logs, console and "main"; we can check if we are the
+    // main log via identity:
+    if (&log_stream[DLOG_MAIN] == this) {
+        external_log_open = false;
+    }
+}
+
+} // end namespace
+
 // Initialise the logging subsystem
 // Potentially throws std::bad_alloc or std::system_error
 void init_log(service_set *sset, bool syslog_format)
diff --git a/src/dinit.cc b/src/dinit.cc
index 1c78278..2942a0a 100644
--- a/src/dinit.cc
+++ b/src/dinit.cc
@@ -73,7 +73,7 @@ static bool am_system_init = false; // true if we are the system init process
 
 static bool did_log_boot = false;
 static bool control_socket_open = false;
-static bool external_log_open = false;
+bool external_log_open = false;
 int active_control_conns = 0;
 
 // Control socket path. We maintain a string (control_socket_str) in case we need
diff --git a/src/tests/test-dinit.cc b/src/tests/test-dinit.cc
index ab22ab6..6fc3442 100644
--- a/src/tests/test-dinit.cc
+++ b/src/tests/test-dinit.cc
@@ -6,6 +6,7 @@
 eventloop_t event_loop;
 
 int active_control_conns = 0;
+bool external_log_open = false;
 
 /*
 These are provided in header instead:
diff --git a/src/tests/test-includes/dinit.h b/src/tests/test-includes/dinit.h
index 5901346..a689da7 100644
--- a/src/tests/test-includes/dinit.h
+++ b/src/tests/test-includes/dinit.h
@@ -109,6 +109,8 @@ class eventloop_t
         }
 
         virtual rearm fd_event(eventloop_t & loop, int fd, int flags) = 0;
+
+        virtual void watch_removed() noexcept { }
     };
 
     template <typename Derived> class fd_watcher_impl : public fd_watcher
-- 
2.25.1