From 359ccafc68fbc6fa39a2c2c9eb08e4cd5397e3b9 Mon Sep 17 00:00:00 2001 From: dwolav200 Date: Thu, 23 Apr 2026 11:16:14 -0400 Subject: [PATCH 1/5] RDKEMW-16763 : rf4ce init failure causes deadlock --- src/ctrlm_main.cpp | 6 +++++- src/rf4ce/ctrlm_rf4ce_network.cpp | 22 ++++++++++++++++++++-- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/ctrlm_main.cpp b/src/ctrlm_main.cpp index c3da8d6d..84db47f5 100644 --- a/src/ctrlm_main.cpp +++ b/src/ctrlm_main.cpp @@ -1235,7 +1235,11 @@ void ctrlm_on_network_assert(ctrlm_network_id_t network_id) { } // g_main_loop_quit() will be called in ctrlm_signal_handler(SIGTERM) g_ctrlm.return_code = -1; - ctrlm_signal_handler(SIGTERM); + int rc = kill(getpid(), SIGTERM); + if(rc != 0) { + XLOGD_ERROR("Failed to send SIGTERM to self. Error code: %d", rc); + } + // give main() time to clean up sleep(5); // Exit here in case main fails to exit diff --git a/src/rf4ce/ctrlm_rf4ce_network.cpp b/src/rf4ce/ctrlm_rf4ce_network.cpp index 5009c592..16ae139c 100644 --- a/src/rf4ce/ctrlm_rf4ce_network.cpp +++ b/src/rf4ce/ctrlm_rf4ce_network.cpp @@ -35,6 +35,7 @@ #include #include #include +#include #include #include "ctrlm.h" #include "ctrlm_log.h" @@ -411,8 +412,25 @@ ctrlm_hal_result_t ctrlm_obj_network_rf4ce_t::hal_init_request(GThread *ctrlm_ma // Block until initialization is complete or a timeout occurs XLOGD_INFO("Waiting for %s initialization...", name_get()); - sem_wait(&semaphore_); - sem_destroy(&semaphore_); + struct timespec timeout; + clock_gettime(CLOCK_REALTIME, &timeout); + timeout.tv_sec += 60; // this operation should complete in under 10 seconds under normal circumstances + + errno = 0; + int sem_result = sem_timedwait(&semaphore_, &timeout); + + if(sem_result == -1) { + if(errno == ETIMEDOUT) { + XLOGD_ERROR("Timeout waiting for %s initialization", name_get()); + } else if(errno == EINTR) { + XLOGD_ERROR("Interrupted while waiting for %s initialization", name_get()); + } else { + XLOGD_ERROR("Error waiting for %s initialization: %s", name_get(), strerror(errno)); + } + init_result_ = CTRLM_HAL_RESULT_ERROR; + } else { + sem_destroy(&semaphore_); + } ready_ = (CTRLM_HAL_RESULT_SUCCESS == init_result_); From 4e6b41507c23d69c139f5c2ad47e0f49351e6b28 Mon Sep 17 00:00:00 2001 From: dwolav200 Date: Thu, 23 Apr 2026 11:46:43 -0400 Subject: [PATCH 2/5] fixing copilot concerns --- src/ctrlm_main.cpp | 12 +++++++----- src/rf4ce/ctrlm_rf4ce_network.cpp | 19 ++++++++++++++----- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/src/ctrlm_main.cpp b/src/ctrlm_main.cpp index 84db47f5..e99ec661 100644 --- a/src/ctrlm_main.cpp +++ b/src/ctrlm_main.cpp @@ -1122,23 +1122,23 @@ void ctrlm_signal_handler(int signal) { switch(signal) { case SIGTERM: case SIGINT: { - XLOGD_INFO("Received %s", signal == SIGINT ? "SIGINT" : "SIGTERM"); + XLOGD_SAFE_INFO("Received %s", signal == SIGINT ? "SIGINT" : "SIGTERM"); ctrlm_quit_main_loop(); break; } case SIGQUIT: { - XLOGD_INFO("Received SIGQUIT"); + XLOGD_SAFE_INFO("Received SIGQUIT"); #ifdef BREAKPAD_SUPPORT ctrlm_crash(); #endif break; } case SIGPIPE: { - XLOGD_ERROR("Received SIGPIPE. Pipe is broken"); + XLOGD_SAFE_ERROR("Received SIGPIPE. Pipe is broken"); break; } default: - XLOGD_ERROR("Received unhandled signal %d", signal); + XLOGD_SAFE_ERROR("Received unhandled signal %d", signal); break; } } @@ -1235,9 +1235,11 @@ void ctrlm_on_network_assert(ctrlm_network_id_t network_id) { } // g_main_loop_quit() will be called in ctrlm_signal_handler(SIGTERM) g_ctrlm.return_code = -1; + + errno = 0; int rc = kill(getpid(), SIGTERM); if(rc != 0) { - XLOGD_ERROR("Failed to send SIGTERM to self. Error code: %d", rc); + XLOGD_ERROR("Failed to send SIGTERM to self <%s>", strerror(errno)); } // give main() time to clean up diff --git a/src/rf4ce/ctrlm_rf4ce_network.cpp b/src/rf4ce/ctrlm_rf4ce_network.cpp index 16ae139c..46acc762 100644 --- a/src/rf4ce/ctrlm_rf4ce_network.cpp +++ b/src/rf4ce/ctrlm_rf4ce_network.cpp @@ -413,11 +413,20 @@ ctrlm_hal_result_t ctrlm_obj_network_rf4ce_t::hal_init_request(GThread *ctrlm_ma // Block until initialization is complete or a timeout occurs XLOGD_INFO("Waiting for %s initialization...", name_get()); struct timespec timeout; - clock_gettime(CLOCK_REALTIME, &timeout); - timeout.tv_sec += 60; // this operation should complete in under 10 seconds under normal circumstances - - errno = 0; - int sem_result = sem_timedwait(&semaphore_, &timeout); + + int sem_result = 0; + int rc = clock_gettime(CLOCK_REALTIME, &timeout); + if(rc != 0) { + // If we fail to get the current time, we should still wait on the semaphore, but we will wait indefinitely instead of timing out + XLOGD_ERROR("Failed to get current time <%s>. wait indefinitely", strerror(errno)); + errno = 0; + sem_result = sem_wait(&semaphore_); + } else { + timeout.tv_sec += 60; // this operation should complete in under 10 seconds under normal circumstances + + errno = 0; + sem_result = sem_timedwait(&semaphore_, &timeout); + } if(sem_result == -1) { if(errno == ETIMEDOUT) { From e4ca7bdef2978a9e673fc76ce1620d0bf8d2e13b Mon Sep 17 00:00:00 2001 From: dwolav200 Date: Thu, 23 Apr 2026 14:25:23 -0400 Subject: [PATCH 3/5] adjust signal processing --- src/ctrlm_main.cpp | 60 ++++++++++++++++++++++++++++++---------------- 1 file changed, 40 insertions(+), 20 deletions(-) diff --git a/src/ctrlm_main.cpp b/src/ctrlm_main.cpp index e99ec661..dc6769e7 100644 --- a/src/ctrlm_main.cpp +++ b/src/ctrlm_main.cpp @@ -23,8 +23,10 @@ #include #include #include +#include #include #include +#include #include #include #include @@ -347,6 +349,8 @@ static gboolean ctrlm_authservice_expired(gpointer user_data); static gboolean ctrlm_ntp_check(gpointer user_data); static void ctrlm_signals_register(void); static void ctrlm_signal_handler(int signal); +static gboolean ctrlm_unix_signal_terminate(gpointer user_data); +static gboolean ctrlm_unix_signal_quit(gpointer user_data); static void ctrlm_main_iarm_call_status_get_(ctrlm_main_iarm_call_status_t *status); static void ctrlm_main_iarm_call_property_get_(ctrlm_main_iarm_call_property_t *property); @@ -1098,47 +1102,61 @@ gboolean ctrlm_authservice_poll(gpointer user_data) { } #endif +// GLib main-loop callbacks for Unix signals — run in the main loop context so +// they are safe to call g_main_loop_quit() and other non-async-signal-safe APIs. +static gboolean ctrlm_unix_signal_terminate(gpointer user_data) { + int sig = GPOINTER_TO_INT(user_data); + XLOGD_INFO("Received %s", sig == SIGINT ? "SIGINT" : "SIGTERM"); + ctrlm_quit_main_loop(); + return G_SOURCE_CONTINUE; +} + +static gboolean ctrlm_unix_signal_quit(gpointer user_data) { + XLOGD_INFO("Received SIGQUIT"); +#ifdef BREAKPAD_SUPPORT + ctrlm_crash(); +#endif + return G_SOURCE_CONTINUE; +} + void ctrlm_signals_register(void) { - // Handle these signals + // Use g_unix_signal_add() so callbacks run inside the GLib main loop context + // rather than from an async signal handler, avoiding undefined behavior from + // calling non-async-signal-safe functions (e.g. g_main_loop_quit). XLOGD_INFO("Registering SIGINT..."); - if(signal(SIGINT, ctrlm_signal_handler) == SIG_ERR) { - XLOGD_ERROR("Unable to register for SIGINT."); - } + g_unix_signal_add(SIGINT, ctrlm_unix_signal_terminate, GINT_TO_POINTER(SIGINT)); XLOGD_INFO("Registering SIGTERM..."); - if(signal(SIGTERM, ctrlm_signal_handler) == SIG_ERR) { - XLOGD_ERROR("Unable to register for SIGTERM."); - } + g_unix_signal_add(SIGTERM, ctrlm_unix_signal_terminate, GINT_TO_POINTER(SIGTERM)); XLOGD_INFO("Registering SIGQUIT..."); - if(signal(SIGQUIT, ctrlm_signal_handler) == SIG_ERR) { - XLOGD_ERROR("Unable to register for SIGQUIT."); - } - XLOGD_INFO("Registering SIGPIPE..."); - if(signal(SIGPIPE, ctrlm_signal_handler) == SIG_ERR) { - XLOGD_ERROR("Unable to register for SIGPIPE."); - } + g_unix_signal_add(SIGQUIT, ctrlm_unix_signal_quit, NULL); + // Ignore SIGPIPE — broken-pipe errors are handled at the call site via errno. + XLOGD_INFO("Ignoring SIGPIPE..."); + signal(SIGPIPE, SIG_IGN); } +// Direct-call fallback only (e.g. from ctrlm_on_network_assert when kill() fails). +// No longer registered as an OS signal handler, so non-async-signal-safe calls are safe. void ctrlm_signal_handler(int signal) { switch(signal) { case SIGTERM: case SIGINT: { - XLOGD_SAFE_INFO("Received %s", signal == SIGINT ? "SIGINT" : "SIGTERM"); + XLOGD_INFO("Received %s", signal == SIGINT ? "SIGINT" : "SIGTERM"); ctrlm_quit_main_loop(); break; } case SIGQUIT: { - XLOGD_SAFE_INFO("Received SIGQUIT"); + XLOGD_INFO("Received SIGQUIT"); #ifdef BREAKPAD_SUPPORT ctrlm_crash(); #endif break; } case SIGPIPE: { - XLOGD_SAFE_ERROR("Received SIGPIPE. Pipe is broken"); + XLOGD_ERROR("Received SIGPIPE. Pipe is broken"); break; } default: - XLOGD_SAFE_ERROR("Received unhandled signal %d", signal); + XLOGD_ERROR("Received unhandled signal %d", signal); break; } } @@ -1233,13 +1251,15 @@ void ctrlm_on_network_assert(ctrlm_network_id_t network_id) { // Invalidate main thread so terminate does not attempt to terminate it g_ctrlm.main_thread = NULL; } - // g_main_loop_quit() will be called in ctrlm_signal_handler(SIGTERM) + // g_main_loop_quit() will be called when the SIGTERM GLib source fires, + // or directly via ctrlm_signal_handler() in the kill() fallback below. g_ctrlm.return_code = -1; errno = 0; int rc = kill(getpid(), SIGTERM); if(rc != 0) { - XLOGD_ERROR("Failed to send SIGTERM to self <%s>", strerror(errno)); + XLOGD_ERROR("Failed to send SIGTERM to self <%s> - invoking shutdown handler directly", strerror(errno)); + ctrlm_signal_handler(SIGTERM); } // give main() time to clean up From 5ab83b9dd4bb65b33a188b88df64f2f36a28f793 Mon Sep 17 00:00:00 2001 From: dwolav200 Date: Thu, 23 Apr 2026 14:46:22 -0400 Subject: [PATCH 4/5] copilot vs agent --- src/ctrlm_main.cpp | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/ctrlm_main.cpp b/src/ctrlm_main.cpp index dc6769e7..e60cd8a1 100644 --- a/src/ctrlm_main.cpp +++ b/src/ctrlm_main.cpp @@ -1124,11 +1124,17 @@ void ctrlm_signals_register(void) { // rather than from an async signal handler, avoiding undefined behavior from // calling non-async-signal-safe functions (e.g. g_main_loop_quit). XLOGD_INFO("Registering SIGINT..."); - g_unix_signal_add(SIGINT, ctrlm_unix_signal_terminate, GINT_TO_POINTER(SIGINT)); + if(0 == g_unix_signal_add(SIGINT, ctrlm_unix_signal_terminate, GINT_TO_POINTER(SIGINT))) { + XLOGD_ERROR("Unable to register for SIGINT."); + } XLOGD_INFO("Registering SIGTERM..."); - g_unix_signal_add(SIGTERM, ctrlm_unix_signal_terminate, GINT_TO_POINTER(SIGTERM)); + if(0 == g_unix_signal_add(SIGTERM, ctrlm_unix_signal_terminate, GINT_TO_POINTER(SIGTERM))) { + XLOGD_ERROR("Unable to register for SIGTERM."); + } XLOGD_INFO("Registering SIGQUIT..."); - g_unix_signal_add(SIGQUIT, ctrlm_unix_signal_quit, NULL); + if(0 == g_unix_signal_add(SIGQUIT, ctrlm_unix_signal_quit, NULL)) { + XLOGD_ERROR("Unable to register for SIGQUIT."); + } // Ignore SIGPIPE — broken-pipe errors are handled at the call site via errno. XLOGD_INFO("Ignoring SIGPIPE..."); signal(SIGPIPE, SIG_IGN); @@ -1136,7 +1142,7 @@ void ctrlm_signals_register(void) { // Direct-call fallback only (e.g. from ctrlm_on_network_assert when kill() fails). // No longer registered as an OS signal handler, so non-async-signal-safe calls are safe. -void ctrlm_signal_handler(int signal) { +static void ctrlm_signal_handler(int signal) { switch(signal) { case SIGTERM: case SIGINT: { From 437f4cb578c554fcf783a9cc96c60d08c23806e7 Mon Sep 17 00:00:00 2001 From: dwolav200 Date: Thu, 23 Apr 2026 15:13:48 -0400 Subject: [PATCH 5/5] more and more --- src/ctrlm_main.cpp | 9 +++++++-- src/rf4ce/ctrlm_rf4ce_network.cpp | 9 ++++++--- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/ctrlm_main.cpp b/src/ctrlm_main.cpp index e60cd8a1..a656d66c 100644 --- a/src/ctrlm_main.cpp +++ b/src/ctrlm_main.cpp @@ -1137,7 +1137,11 @@ void ctrlm_signals_register(void) { } // Ignore SIGPIPE — broken-pipe errors are handled at the call site via errno. XLOGD_INFO("Ignoring SIGPIPE..."); - signal(SIGPIPE, SIG_IGN); + errno = 0; + if(SIG_ERR == signal(SIGPIPE, SIG_IGN)) { + int errsv = errno; + XLOGD_ERROR("Unable to ignore SIGPIPE <%s>", strerror(errsv)); + } } // Direct-call fallback only (e.g. from ctrlm_on_network_assert when kill() fails). @@ -1264,7 +1268,8 @@ void ctrlm_on_network_assert(ctrlm_network_id_t network_id) { errno = 0; int rc = kill(getpid(), SIGTERM); if(rc != 0) { - XLOGD_ERROR("Failed to send SIGTERM to self <%s> - invoking shutdown handler directly", strerror(errno)); + int errsv = errno; + XLOGD_ERROR("Failed to send SIGTERM to self <%s> - invoking shutdown handler directly", strerror(errsv)); ctrlm_signal_handler(SIGTERM); } diff --git a/src/rf4ce/ctrlm_rf4ce_network.cpp b/src/rf4ce/ctrlm_rf4ce_network.cpp index 46acc762..11a81c3c 100644 --- a/src/rf4ce/ctrlm_rf4ce_network.cpp +++ b/src/rf4ce/ctrlm_rf4ce_network.cpp @@ -415,14 +415,16 @@ ctrlm_hal_result_t ctrlm_obj_network_rf4ce_t::hal_init_request(GThread *ctrlm_ma struct timespec timeout; int sem_result = 0; + errno = 0; int rc = clock_gettime(CLOCK_REALTIME, &timeout); if(rc != 0) { + int errsv = errno; // If we fail to get the current time, we should still wait on the semaphore, but we will wait indefinitely instead of timing out - XLOGD_ERROR("Failed to get current time <%s>. wait indefinitely", strerror(errno)); + XLOGD_ERROR("Failed to get current time <%s>. wait indefinitely", strerror(errsv)); errno = 0; sem_result = sem_wait(&semaphore_); } else { - timeout.tv_sec += 60; // this operation should complete in under 10 seconds under normal circumstances + timeout.tv_sec += 60; // this operation has been tested to complete in about 6 seconds in worst case scenario (set the timeout to 10x) errno = 0; sem_result = sem_timedwait(&semaphore_, &timeout); @@ -434,7 +436,8 @@ ctrlm_hal_result_t ctrlm_obj_network_rf4ce_t::hal_init_request(GThread *ctrlm_ma } else if(errno == EINTR) { XLOGD_ERROR("Interrupted while waiting for %s initialization", name_get()); } else { - XLOGD_ERROR("Error waiting for %s initialization: %s", name_get(), strerror(errno)); + int errsv = errno; + XLOGD_ERROR("Error waiting for %s initialization <%s>", name_get(), strerror(errsv)); } init_result_ = CTRLM_HAL_RESULT_ERROR; } else {