@cryptotaxi247 / netdata-1 / commits / 7d6dcf48d

Fix race condition on shutdown (#21563)

* Initialize `discovery_thread` properly and adjust `exited` flag handling. Ensure thread-safe cleanup and synchronization primitive usage during thread creation. * Update src/collectors/cgroups.plugin/sys_fs_cgroup.c Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Use proper memory ordering * Do proper cleanup of mutex / cond var * Improve cleanup --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Stelios Fragkakis committed Jan 16, 2026 at 10:23 UTC 7d6dcf48d7493cf07900810a6426e82db8a7e181
2 files changed +24 -8
src/collectors/cgroups.plugin/cgroup-discovery.c
+1 -1
@@ -1356,5 +1356,5 @@ void cgroup_discovery_worker(void *ptr)
1356 cgroup_cleanup_ebpf_integration();
1357 worker_unregister();
1358 service_exits();
1359 - __atomic_store_n(&discovery_thread.exited,1,__ATOMIC_RELAXED);
1359 + __atomic_store_n(&discovery_thread.exited, 1, __ATOMIC_RELEASE);
1360 }
src/collectors/cgroups.plugin/sys_fs_cgroup.c
+23 -7
@@ -63,7 +63,9 @@ struct cgroups_systemd_config_setting cgroups_systemd_options[] = {
63 { .name = NULL, .setting = SYSTEMD_CGROUP_ERR },
64 };
65
66 -struct discovery_thread discovery_thread;
66 +struct discovery_thread discovery_thread = {
67 + .exited = 1, // Start as "exited" until properly initialized
68 +};
69
70
71 /* on Fed systemd is not in PATH for some reason */
@@ -1332,9 +1334,9 @@ static void cgroup_main_cleanup(void *pptr) {
1334
1335 usec_t max = 2 * USEC_PER_SEC, step = 50000;
1336
1335 - if (!__atomic_load_n(&discovery_thread.exited, __ATOMIC_RELAXED)) {
1337 + if (!__atomic_load_n(&discovery_thread.exited, __ATOMIC_ACQUIRE)) {
1338 collector_info("waiting for discovery thread to finish...");
1337 - while (!__atomic_load_n(&discovery_thread.exited, __ATOMIC_RELAXED) && max > 0) {
1339 + while (!__atomic_load_n(&discovery_thread.exited, __ATOMIC_ACQUIRE) && max > 0) {
1340 netdata_mutex_lock(&discovery_thread.mutex);
1341 netdata_cond_signal(&discovery_thread.cond_var);
1342 netdata_mutex_unlock(&discovery_thread.mutex);
@@ -1343,8 +1345,14 @@ static void cgroup_main_cleanup(void *pptr) {
1345 }
1346 }
1347 // We should be done, but just in case, avoid blocking shutdown
1346 - if (__atomic_load_n(&discovery_thread.exited, __ATOMIC_RELAXED))
1347 - (void) nd_thread_join(discovery_thread.thread);
1348 + // Only join and destroy synchronization primitives if thread has exited
1349 + if (__atomic_load_n(&discovery_thread.exited, __ATOMIC_ACQUIRE)) {
1350 + if (discovery_thread.thread) {
1351 + (void) nd_thread_join(discovery_thread.thread);
1352 + netdata_cond_destroy(&discovery_thread.cond_var);
1353 + netdata_mutex_destroy(&discovery_thread.mutex);
1354 + }
1355 + }
1356
1357 static_thread->enabled = NETDATA_MAIN_THREAD_EXITED;
1358 }
@@ -1390,21 +1398,29 @@ void cgroups_main(void *ptr) {
1398 // for the other nodes, the origin server should register it
1399 cgroup_netdev_link_init();
1400
1393 - discovery_thread.exited = 0;
1394 -
1401 if (netdata_mutex_init(&discovery_thread.mutex)) {
1402 collector_error("CGROUP: cannot initialize mutex for discovery thread");
1403 return;
1404 }
1405 if (netdata_cond_init(&discovery_thread.cond_var)) {
1406 collector_error("CGROUP: cannot initialize conditional variable for discovery thread");
1407 + netdata_mutex_destroy(&discovery_thread.mutex);
1408 return;
1409 }
1410
1411 + // Mark thread as "running" only after mutex/cond are initialized
1412 + // but before creating the thread. This ensures cleanup won't try
1413 + // to access uninitialized synchronization primitives.
1414 + // Use RELEASE ordering so readers with ACQUIRE see initialized mutex/cond.
1415 + __atomic_store_n(&discovery_thread.exited, 0, __ATOMIC_RELEASE);
1416 +
1417 discovery_thread.thread = nd_thread_create("CGDISCOVER", NETDATA_THREAD_OPTION_DEFAULT, cgroup_discovery_worker, NULL);
1418
1419 if (!discovery_thread.thread) {
1420 collector_error("CGROUP: cannot create thread worker");
1421 + __atomic_store_n(&discovery_thread.exited, 1, __ATOMIC_RELEASE); // Reset since thread wasn't created
1422 + netdata_cond_destroy(&discovery_thread.cond_var);
1423 + netdata_mutex_destroy(&discovery_thread.mutex);
1424 return;
1425 }
1426