Aren/haiku
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

kernel/condition_variable: Remove a confusing requirement for published variables.

Browse Source 
Part of the point of published variables is to make them "shareable",
and not require external synchronization. Requiring the callers
to ensure unpublishing does not occur is thus unreasonable, as e.g.
a variable could be unpublished immediately after being notified.

That is the case for some usages of these variables in the FreeBSD
compatibility layer, which under heavy usage, can and did trigger
use-after-unpublishes and then KDLs, at least in local testing.

Instead, only unlock the hash after we have locked the variable.
This is already done in some other functions, so it's safe to do
it here, too. This way, the variable won't be unpublished
while Notify() is running.
This commit is contained in:
Augustin Cavalier 2022-03-09 18:55:55 -05:00
parent d38d90de25
commit bb09a3ed07
2 changed files with 14 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
headers/private/kernel/condition_variable.h
View File

@ -63,9 +63,6 @@ public:
static void NotifyOne(const void* object, status_t result);
static void NotifyAll(const void* object, status_t result);
// (both methods) caller must ensure that
// the variable is not unpublished
// concurrently
void Add(ConditionVariableEntry* entry);
@ -81,6 +78,7 @@ public:
void Dump() const;
private:
static void _Notify(const void* object, bool all, status_t result);
void _Notify(bool all, status_t result);
void _NotifyLocked(bool all, status_t result);

23
src/system/kernel/condition_variable.cpp
View File

@ -362,26 +362,29 @@ ConditionVariable::Wait(recursive_lock* lock, uint32 flags, bigtime_t timeout)
/*static*/ void
ConditionVariable::NotifyOne(const void* object, status_t result)
{
InterruptsReadSpinLocker locker(sConditionVariableHashLock);
ConditionVariable* variable = sConditionVariableHash.Lookup(object);
locker.Unlock();
if (variable == NULL)
return;
variable->NotifyOne(result);
_Notify(object, false, result);
}
/*static*/ void
ConditionVariable::NotifyAll(const void* object, status_t result)
{
InterruptsReadSpinLocker locker(sConditionVariableHashLock);
_Notify(object, true, result);
}
/*static*/ void
ConditionVariable::_Notify(const void* object, bool all, status_t result)
{
InterruptsLocker ints;
ReadSpinLocker hashLocker(sConditionVariableHashLock);
ConditionVariable* variable = sConditionVariableHash.Lookup(object);
locker.Unlock();
if (variable == NULL)
return;
SpinLocker variableLocker(variable->fLock);
hashLocker.Unlock();
variable->NotifyAll(result);
variable->_NotifyLocked(all, result);
}