Improve thread join handling (#21421)
* Fix thread join handling to prevent race conditions during shutdown * Improve thread cleanup to ensure proper handling of thread states during join
Stelios Fragkakis committed
Dec 8, 2025 at 21:06 UTC
bb05696e076dc3574a4a7bfb9db341de1ee49f91
1 file changed
+28
-28
src/libnetdata/threads/threads.c
+28
-28
@@ -266,18 +266,11 @@ void nd_thread_join_threads()
266
ND_THREAD *nti;
267
do {
268
spinlock_lock(&threads_globals.exited.spinlock);
269
-
269
nti = threads_globals.exited.list;
271
-
272
- if (nti) {
273
- DOUBLE_LINKED_LIST_REMOVE_ITEM_UNSAFE(threads_globals.exited.list, nti, prev, next);
274
- nti->list = ND_THREAD_LIST_NONE;
275
- nd_log_daemon(NDLP_DEBUG, "nd_thread_join_threads: Joining thread with id %d (%s) during shutdown", nti->tid, nti->tag);
276
- }
277
-
270
spinlock_unlock(&threads_globals.exited.spinlock);
271
280
- // handles null
272
+ // nd_thread_join() handles NULL and will remove from list and free atomically
273
+ // to avoid race with other callers joining the same thread
274
nd_thread_join(nti);
275
276
} while (nti);
@@ -458,8 +451,16 @@ int nd_thread_join(ND_THREAD *nti) {
451
if(!nti)
452
return ESRCH;
453
461
- if(nd_thread_status_check(nti, NETDATA_THREAD_STATUS_JOINED))
462
- return 0;
454
+ // Atomically check and set JOINED flag to prevent race conditions
455
+ // where two threads both try to join the same thread
456
+ NETDATA_THREAD_OPTIONS old_options;
457
+ do {
458
+ old_options = __atomic_load_n(&nti->options, __ATOMIC_ACQUIRE);
459
+ if(old_options & NETDATA_THREAD_STATUS_JOINED)
460
+ return 0;
461
+ } while(!__atomic_compare_exchange_n(&nti->options, &old_options,
462
+ old_options | NETDATA_THREAD_STATUS_JOINED,
463
+ false, __ATOMIC_ACQ_REL, __ATOMIC_ACQUIRE));
464
465
int ret;
466
@@ -493,26 +494,25 @@ int nd_thread_join(ND_THREAD *nti) {
494
}
495
}
496
496
- if(ret == 0) {
497
- // we successfully joined the thread (or cleaned up after Windows fast-exit)
498
- nd_thread_status_set(nti, NETDATA_THREAD_STATUS_JOINED);
497
+ // Always clean up the thread structure - if uv_thread_join() failed,
498
+ // retrying won't help (thread doesn't exist, not joinable, or logic error)
499
+ // JOINED flag was already set atomically at the start of this function
500
500
- spinlock_lock(&threads_globals.running.spinlock);
501
- if(nti->list == ND_THREAD_LIST_RUNNING) {
502
- DOUBLE_LINKED_LIST_REMOVE_ITEM_UNSAFE(threads_globals.running.list, nti, prev, next);
503
- nti->list = ND_THREAD_LIST_NONE;
504
- }
505
- spinlock_unlock(&threads_globals.running.spinlock);
506
-
507
- spinlock_lock(&threads_globals.exited.spinlock);
508
- if(nti->list == ND_THREAD_LIST_EXITED) {
509
- DOUBLE_LINKED_LIST_REMOVE_ITEM_UNSAFE(threads_globals.exited.list, nti, prev, next);
510
- nti->list = ND_THREAD_LIST_NONE;
511
- }
512
- spinlock_unlock(&threads_globals.exited.spinlock);
501
+ spinlock_lock(&threads_globals.running.spinlock);
502
+ if(nti->list == ND_THREAD_LIST_RUNNING) {
503
+ DOUBLE_LINKED_LIST_REMOVE_ITEM_UNSAFE(threads_globals.running.list, nti, prev, next);
504
+ nti->list = ND_THREAD_LIST_NONE;
505
+ }
506
+ spinlock_unlock(&threads_globals.running.spinlock);
507
514
- freez(nti);
508
+ spinlock_lock(&threads_globals.exited.spinlock);
509
+ if(nti->list == ND_THREAD_LIST_EXITED) {
510
+ DOUBLE_LINKED_LIST_REMOVE_ITEM_UNSAFE(threads_globals.exited.list, nti, prev, next);
511
+ nti->list = ND_THREAD_LIST_NONE;
512
}
513
+ spinlock_unlock(&threads_globals.exited.spinlock);
514
+
515
+ freez(nti);
516
517
return ret;
518
}