From addece2001c2683709aec70ec48f91fa142c0752 Mon Sep 17 00:00:00 2001 From: Ingo Weinhold Date: Sun, 2 Sep 2007 22:21:26 +0000 Subject: [PATCH] * Fixed incorrect check in condition_variable_interrupt_thread(). Condition variables would never be interrupted. * ConditionVariableEntry::Add() did not correctly insert the entry into the per-thread list of entries (the next link of the previous entry was not adjusted), which could leave the entry unnotified when the previous entry was notified, thus leaving it in the respective condition variable's list after the end of its life time. This should fix a crashing bug I rarely encountered. * Added debug checks in the PrivateConditionVariableEntry constructor/destructor that should have helped me to find forementioned bug hours earlier, had I been bright enough to realize that I didn't include and those KDEBUG guarded checks were never executed. :-/ git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@22151 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- headers/private/kernel/condition_variable.h | 19 +++++++++++++++++++ src/system/kernel/condition_variable.cpp | 5 ++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/headers/private/kernel/condition_variable.h b/headers/private/kernel/condition_variable.h index 1b733b910b..7943ca7fed 100644 --- a/headers/private/kernel/condition_variable.h +++ b/headers/private/kernel/condition_variable.h @@ -8,6 +8,8 @@ #include +#include + #ifdef __cplusplus #include @@ -18,11 +20,28 @@ class PrivateConditionVariable; struct PrivateConditionVariableEntry { public: +#if KDEBUG + inline PrivateConditionVariableEntry() + : fVariable(NULL) + { + } + + inline ~PrivateConditionVariableEntry() + { + if (fVariable != NULL) { + panic("Destroying condition variable entry %p, but it's still " + "attached to variable %p\n", this, fVariable); + } + } +#endif + inline PrivateConditionVariable* Variable() const { return fVariable; } inline PrivateConditionVariableEntry* ThreadNext() const { return fThreadNext; } + inline PrivateConditionVariableEntry* ThreadPrevious() const + { return fThreadPrevious; } class Private; diff --git a/src/system/kernel/condition_variable.cpp b/src/system/kernel/condition_variable.cpp index 43be0e2e95..3a5ab79fc0 100644 --- a/src/system/kernel/condition_variable.cpp +++ b/src/system/kernel/condition_variable.cpp @@ -107,6 +107,8 @@ PrivateConditionVariableEntry::Add(const void* object, if (threadNext) { fThreadPrevious = threadNext->fThreadPrevious; threadNext->fThreadPrevious = this; + if (fThreadPrevious) + fThreadPrevious->fThreadNext = this; } else fThreadPrevious = NULL; @@ -399,7 +401,7 @@ condition_variable_interrupt_thread(struct thread* thread) // re-get the thread and do the checks again thread = thread_get_thread_struct_locked(threadID); - if (thread != NULL || thread->state != B_THREAD_WAITING + if (thread == NULL || thread->state != B_THREAD_WAITING || thread->condition_variable_entry == NULL) { return B_BAD_VALUE; } @@ -417,6 +419,7 @@ condition_variable_interrupt_thread(struct thread* thread) PrivateConditionVariableEntry::Private(*entry).SetResult(B_INTERRUPTED); // remove all of the thread's entries from their variables + ASSERT(entry->ThreadPrevious() == NULL); while (entry) { PrivateConditionVariableEntry::Private(*entry).Remove(); entry = entry->ThreadNext();