diff --git a/src/Flare.cc b/src/Flare.cc index 4229ffb63c..166917d914 100644 --- a/src/Flare.cc +++ b/src/Flare.cc @@ -13,8 +13,11 @@ Flare::Flare() { } -static void bad_pipe_op(const char* which) +static void bad_pipe_op(const char* which, bool signal_safe) { + if ( signal_safe ) + abort(); + char buf[256]; bro_strerror_r(errno, buf, sizeof(buf)); @@ -27,7 +30,7 @@ static void bad_pipe_op(const char* which) } } -void Flare::Fire() +void Flare::Fire(bool signal_safe) { char tmp = 0; @@ -49,14 +52,14 @@ void Flare::Fire() // Interrupted: try again. continue; - bad_pipe_op("write"); + bad_pipe_op("write", signal_safe); } // No error, but didn't write a byte: try again. } } -int Flare::Extinguish() +int Flare::Extinguish(bool signal_safe) { int rval = 0; char tmp[256]; @@ -80,7 +83,7 @@ int Flare::Extinguish() // Interrupted: try again. continue; - bad_pipe_op("read"); + bad_pipe_op("read", signal_safe); } return rval; diff --git a/src/Flare.h b/src/Flare.h index ebe902c172..4c781387bf 100644 --- a/src/Flare.h +++ b/src/Flare.h @@ -26,15 +26,19 @@ public: /** * Put the object in the "ready" state. + * @param signal_safe whether to skip error-reporting functionality that + * is not async-signal-safe */ - void Fire(); + void Fire(bool signal_safe = false); /** * Take the object out of the "ready" state. + * @param signal_safe whether to skip error-reporting functionality that + * is not async-signal-safe * @return number of bytes read from the pipe, corresponds to the number * of times Fire() was called. */ - int Extinguish(); + int Extinguish(bool signal_safe = false); private: Pipe pipe; diff --git a/src/Supervisor.cc b/src/Supervisor.cc index 4de9e88267..4c4484029e 100644 --- a/src/Supervisor.cc +++ b/src/Supervisor.cc @@ -65,6 +65,7 @@ struct Stem { void LogError(const char* format, ...) const __attribute__((format(printf, 2, 3))); + int last_signal = -1; std::unique_ptr signal_flare; std::unique_ptr pipe; std::map nodes; @@ -77,13 +78,12 @@ static Stem* stem = nullptr; static RETSIGTYPE stem_sig_handler(int signo) { - // TODO: signal safety - DBG_STEM("Stem received signal: %d", signo); + stem->last_signal = signo; if ( stem->shutting_down ) return RETSIGVAL; - stem->signal_flare->Fire(); + stem->signal_flare->Fire(true); if ( signo == SIGTERM ) stem->shutting_down = true; @@ -93,9 +93,7 @@ static RETSIGTYPE stem_sig_handler(int signo) static RETSIGTYPE supervisor_sig_handler(int signo) { - // TODO: signal safety - DBG_LOG(DBG_SUPERVISOR, "received signal: %d", signo); - supervisor->ObserveChildSignal(); + supervisor->ObserveChildSignal(signo); return RETSIGVAL; } @@ -126,8 +124,8 @@ static std::string make_create_message(const Supervisor::NodeConfig& node) } Supervisor::Supervisor(Supervisor::Config cfg, - std::unique_ptr pipe, - pid_t arg_stem_pid) + std::unique_ptr pipe, + pid_t arg_stem_pid) : config(std::move(cfg)), stem_pid(arg_stem_pid), stem_pipe(std::move(pipe)) { DBG_LOG(DBG_SUPERVISOR, "forked stem process %d", stem_pid); @@ -171,9 +169,10 @@ Supervisor::~Supervisor() while ( ProcessMessages() != 0 ); } -void Supervisor::ObserveChildSignal() +void Supervisor::ObserveChildSignal(int signo) { - signal_flare.Fire(); + last_signal = signo; + signal_flare.Fire(true); } void Supervisor::ReapStem() @@ -216,13 +215,19 @@ void Supervisor::ReapStem() void Supervisor::HandleChildSignal() { + if ( last_signal >= 0 ) + { + DBG_LOG(DBG_SUPERVISOR, "Supervisor received signal %d", last_signal); + last_signal = -1; + } + bool had_child_signal = signal_flare.Extinguish(); if ( had_child_signal ) { ReapStem(); - DBG_LOG(DBG_SUPERVISOR, "processed SIGCHLD %s", + DBG_LOG(DBG_SUPERVISOR, "Supervisor processed child signal %s", stem_pid ? "(spurious)" : ""); } @@ -657,6 +662,12 @@ std::optional Stem::Poll() } } + if ( last_signal >= 0 ) + { + DBG_STEM("Stem received signal: %d", last_signal); + last_signal = -1; + } + if ( getppid() == 1 ) { // TODO: better way to detect loss of parent than polling ? diff --git a/src/Supervisor.h b/src/Supervisor.h index 2d74164e52..01429e6508 100644 --- a/src/Supervisor.h +++ b/src/Supervisor.h @@ -80,7 +80,7 @@ public: pid_t StemPID() const { return stem_pid; } - void ObserveChildSignal(); + void ObserveChildSignal(int signo); RecordVal* Status(std::string_view node_name); std::string Create(const RecordVal* node); @@ -113,6 +113,7 @@ private: Config config; pid_t stem_pid; std::unique_ptr stem_pipe; + int last_signal = -1; bro::Flare signal_flare; NodeMap nodes; std::string msg_buffer;