fix: memory leak from signal handling event in daemon (#5695)

daemon/daemon-posix.cc

@@ -52,10 +52,9 @@ static void handle_signals(evutil_socket_t fd, short /*what*/, void* arg)
     }
 }
 
-bool tr_daemon::setup_signals()
+bool tr_daemon::setup_signals(struct event*& sig_ev)
 {
     sigset_t mask = {};
-    struct event* sigev = nullptr;
     struct event_base* base = ev_base_;
 
     sigemptyset(&mask);
@@ -70,25 +69,16 @@ bool tr_daemon::setup_signals()
     if (sigfd_ < 0)
         return false;
 
-    sigev = event_new(base, sigfd_, EV_READ | EV_PERSIST, handle_signals, this);
-    if (sigev == nullptr)
-    {
-        close(sigfd_);
-        return false;
-    }
+    sig_ev = event_new(base, sigfd_, EV_READ | EV_PERSIST, handle_signals, this);
 
-    if (event_add(sigev, nullptr) < 0)
-    {
-        event_del(sigev);
-        close(sigfd_);
-        return false;
-    }
-
-    return true;
+    return sig_ev != nullptr && event_add(sig_ev, nullptr) >= 0;
 }
 
 #else /* no signalfd API, use evsignal */
 
+namespace
+{
+
 static void reconfigureMarshall(evutil_socket_t /*fd*/, short /*events*/, void* arg)
 {
     static_cast<tr_daemon*>(arg)->reconfigure();
@@ -99,30 +89,45 @@ static void stopMarshall(evutil_socket_t /*fd*/, short /*events*/, void* arg)
     static_cast<tr_daemon*>(arg)->stop();
 }
 
-static bool setup_signal(struct event_base* base, int sig, void (*callback)(evutil_socket_t, short, void*), void* arg)
+static bool setup_signal(
+    struct event_base* base,
+    struct event*& sig_ev,
+    int sig,
+    void (*callback)(evutil_socket_t, short, void*),
+    void* arg)
 {
-    struct event* sigev = evsignal_new(base, sig, callback, arg);
+    sig_ev = evsignal_new(base, sig, callback, arg);
 
-    if (sigev == nullptr)
-        return false;
-
-    if (evsignal_add(sigev, nullptr) < 0)
-    {
-        event_free(sigev);
-        return false;
-    }
-
-    return true;
+    return sig_ev != nullptr && evsignal_add(sig_ev, nullptr) >= 0;
 }
 
-bool tr_daemon::setup_signals()
+} // anonymous namespace
+
+bool tr_daemon::setup_signals(struct event*& sig_ev)
 {
-    return setup_signal(ev_base_, SIGHUP, reconfigureMarshall, this) && setup_signal(ev_base_, SIGINT, stopMarshall, this) &&
-        setup_signal(ev_base_, SIGTERM, stopMarshall, this);
+    return setup_signal(ev_base_, sig_ev, SIGHUP, reconfigureMarshall, this) &&
+        setup_signal(ev_base_, sig_ev, SIGINT, stopMarshall, this) &&
+        setup_signal(ev_base_, sig_ev, SIGTERM, stopMarshall, this);
 }
 
 #endif /* HAVE_SYS_SIGNALFD_H */
 
+void tr_daemon::cleanup_signals(struct event* sig_ev) const
+{
+    if (sig_ev != nullptr)
+    {
+        event_del(sig_ev);
+        event_free(sig_ev);
+    }
+
+#ifdef HAVE_SYS_SIGNALFD_H
+    if (sigfd_ >= 0)
+    {
+        close(sigfd_);
+    }
+#endif /* HAVE_SYS_SIGNALFD_H */
+}
+
 bool tr_daemon::spawn(bool foreground, int* exit_code, tr_error** error)
 {
     *exit_code = 1;

daemon/daemon-win32.cc

@@ -196,11 +196,15 @@ static VOID WINAPI service_main(DWORD /*argc*/, LPWSTR* /*argv*/)
     update_service_status(SERVICE_STOPPED, NO_ERROR, exit_code, 0, 0);
 }
 
-bool tr_daemon::setup_signals()
+bool tr_daemon::setup_signals([[maybe_unused]] struct event*& sig_ev)
 {
     return true;
 }
 
+void tr_daemon::cleanup_signals([[maybe_unused]] struct event* sig_ev) const
+{
+}
+
 bool tr_daemon::spawn(bool foreground, int* exit_code, tr_error** error)
 {
     daemon = this;

daemon/daemon.cc

@@ -687,6 +687,7 @@ int tr_daemon::start([[maybe_unused]] bool foreground)
     bool pidfile_created = false;
     tr_session* session = nullptr;
     struct event* status_ev = nullptr;
+    struct event* sig_ev = nullptr;
     auto watchdir = std::unique_ptr<Watchdir>{};
     char const* const cdir = this->config_dir_.c_str();
 
@@ -695,7 +696,7 @@ int tr_daemon::start([[maybe_unused]] bool foreground)
     /* setup event state */
     ev_base_ = event_base_new();
 
-    if (ev_base_ == nullptr || !setup_signals())
+    if (ev_base_ == nullptr || !setup_signals(sig_ev))
     {
         auto const error_code = errno;
         auto const errmsg = fmt::format(
@@ -703,6 +704,7 @@ int tr_daemon::start([[maybe_unused]] bool foreground)
             fmt::arg("error", tr_strerror(error_code)),
             fmt::arg("error_code", error_code));
         printMessage(logfile_, TR_LOG_ERROR, MyName, errmsg, __FILE__, __LINE__);
+        cleanup_signals(sig_ev);
         return 1;
     }
 
@@ -855,6 +857,8 @@ CLEANUP:
         event_free(status_ev);
     }
 
+    cleanup_signals(sig_ev);
+
     event_base_free(ev_base_);
 
     tr_sessionSaveSettings(my_session_, cdir, &settings_);

daemon/daemon.h

@@ -57,6 +57,7 @@ private:
 
     bool parse_args(int argc, char const* const* argv, bool* dump_settings, bool* foreground, int* exit_code);
     bool reopen_log_file(char const* filename);
-    bool setup_signals();
+    bool setup_signals(struct event*& sig_ev);
+    void cleanup_signals(struct event* sig_ev) const;
     void report_status();
 };