From 331889d067f9a4c6984a8e34e55e73915e711754 Mon Sep 17 00:00:00 2001 From: Niels Sascha Reedijk Date: Fri, 28 Aug 2020 21:26:45 +0100 Subject: [PATCH] Kernel/Threads: remove limit on number of dead threads in a team When a thread is created, it is expected that some other thread (usually the creating thread) will want to make sure it completes. This is done using the pthread_join() or wait_for_thread() calls. It is possible that threads end before another thread waits for its completion. That's why there is a dead thread list for each team, which holds thread ids and their exit status so that a call to pthread_join() or wait_for_thread() in the future can complete succesfully. The dead thread list was limited to 32 threads per team. If there would be more, the oldest thread would be kicked off. This could cause issues in situations where a team would create more than 32 threads, and would start waiting for their result after they have finished. Some of the calls would fail because the threads would no longer be in the dead list. This specifically caused problems for cargo (the Rust package manager), which could depending on the number of dependencies, could create more than 32 threads. See: https://github.com/nielx/rust/issues/3 This change removes the limit of dead threads within a team. Note that there is a risk that a badly written program that does not detach or joins its threads can make this an endless list, but the impact is relatively small (dead threads only occupy a bit of kernel memory). Change-Id: I0135dd54e10ee48a529f23228d21237d4f1a74e2 Reviewed-on: https://review.haiku-os.org/c/haiku/+/3178 Reviewed-by: Adrien Destugues --- headers/private/kernel/thread_types.h | 3 --- src/system/kernel/team.cpp | 1 - src/system/kernel/thread.cpp | 14 +------------- 3 files changed, 1 insertion(+), 17 deletions(-) diff --git a/headers/private/kernel/thread_types.h b/headers/private/kernel/thread_types.h index 1da7cd5c5d..2fa6b0090e 100644 --- a/headers/private/kernel/thread_types.h +++ b/headers/private/kernel/thread_types.h @@ -96,8 +96,6 @@ struct team_watcher { #define MAX_DEAD_CHILDREN 32 // this is a soft limit for the number of child death entries in a team -#define MAX_DEAD_THREADS 32 - // this is a soft limit for the number of thread death entries in a team struct job_control_entry : DoublyLinkedListLinkImpl { @@ -238,7 +236,6 @@ struct Team : TeamThreadIteratorEntry, KernelReferenceable, struct xsi_sem_context *xsi_sem_context; struct team_death_entry *death_entry; // protected by fLock struct list dead_threads; - int dead_threads_count; // protected by the team's fLock team_dead_children dead_children; diff --git a/src/system/kernel/team.cpp b/src/system/kernel/team.cpp index 8fc10a8087..e791a27c30 100644 --- a/src/system/kernel/team.cpp +++ b/src/system/kernel/team.cpp @@ -470,7 +470,6 @@ Team::Team(team_id id, bool kernel) // dead threads list_init(&dead_threads); - dead_threads_count = 0; // dead children dead_children.count = 0; diff --git a/src/system/kernel/thread.cpp b/src/system/kernel/thread.cpp index b8e6d2ac96..1f10548ff9 100644 --- a/src/system/kernel/thread.cpp +++ b/src/system/kernel/thread.cpp @@ -2139,17 +2139,8 @@ thread_exit(void) threadDeathEntry->thread = thread->id; threadDeathEntry->status = thread->exit.status; - // add entry -- remove an old one, if we hit the limit + // add entry to dead thread list list_add_item(&team->dead_threads, threadDeathEntry); - team->dead_threads_count++; - threadDeathEntry = NULL; - - if (team->dead_threads_count > MAX_DEAD_THREADS) { - threadDeathEntry - = (thread_death_entry*)list_remove_head_item( - &team->dead_threads); - team->dead_threads_count--; - } } threadCreationLocker.Unlock(); @@ -2164,8 +2155,6 @@ thread_exit(void) thread->id)); } - free(threadDeathEntry); - // delete the team if we're its main thread if (deleteTeam) { team_delete_team(team, debuggerPort); @@ -2505,7 +2494,6 @@ wait_for_thread_etc(thread_id id, uint32 flags, bigtime_t timeout, &team->dead_threads, threadDeathEntry)) != NULL) { if (threadDeathEntry->thread == id) { list_remove_item(&team->dead_threads, threadDeathEntry); - team->dead_threads_count--; death.status = threadDeathEntry->status; free(threadDeathEntry); break;