1
0
Fork 0
mirror of https://github.com/juce-framework/JUCE.git synced 2026-01-10 23:44:24 +00:00

MessageManager: Improve thread safety of Lock type

Previously, the following sequence of events was possible:

Background thread                   Main thread
------------------------------------------------------------------------
Lock::tryAcquire()
    Run to blockingMessage->post()

                                    BlockingMessage::messageCallback()
                                        Run to abortWait.set (1)

Lock::tryAcquire()
    Exit through return true

Lock::~Lock()
    Destroy memory used for Lock

                                    BlockingMessage::messageCallback()
                                        Execute lockedEvent.signal()
                                        Memory already freed, crash
This commit is contained in:
reuk 2023-04-26 21:25:05 +01:00
parent 2d31153d99
commit 33e81616ad
No known key found for this signature in database
GPG key ID: FCB43929F012EE5C
2 changed files with 62 additions and 54 deletions

View file

@ -284,25 +284,41 @@ bool MessageManager::existsAndIsCurrentThread() noexcept
*/
struct MessageManager::Lock::BlockingMessage : public MessageManager::MessageBase
{
BlockingMessage (const MessageManager::Lock* parent) noexcept
: owner (parent)
{}
explicit BlockingMessage (const MessageManager::Lock* parent) noexcept
: owner (parent) {}
void messageCallback() override
{
{
ScopedLock lock (ownerCriticalSection);
std::unique_lock lock { mutex };
if (auto* o = owner.get())
o->messageCallback();
if (owner != nullptr)
{
owner->abort();
acquired = true;
}
releaseEvent.wait();
condvar.wait (lock, [&] { return owner == nullptr; });
}
CriticalSection ownerCriticalSection;
Atomic<const MessageManager::Lock*> owner;
WaitableEvent releaseEvent;
void stopWaiting()
{
const ScopeGuard scope { [&] { condvar.notify_one(); } };
const std::scoped_lock lock { mutex };
owner = nullptr;
}
bool wasAcquired()
{
const std::scoped_lock lock { mutex };
return acquired;
}
private:
std::mutex mutex;
std::condition_variable condvar;
const MessageManager::Lock* owner = nullptr;
bool acquired = false;
JUCE_DECLARE_NON_COPYABLE (BlockingMessage)
};
@ -323,9 +339,12 @@ bool MessageManager::Lock::tryAcquire (bool lockIsMandatory) const noexcept
return false;
}
if (! lockIsMandatory && (abortWait.get() != 0))
if (! lockIsMandatory && [&]
{
const std::scoped_lock lock { mutex };
return std::exchange (abortWait, false);
}())
{
abortWait.set (0);
return false;
}
@ -350,65 +369,54 @@ bool MessageManager::Lock::tryAcquire (bool lockIsMandatory) const noexcept
return false;
}
do
for (;;)
{
while (abortWait.get() == 0)
lockedEvent.wait (-1);
{
std::unique_lock lock { mutex };
condvar.wait (lock, [&] { return std::exchange (abortWait, false); });
}
abortWait.set (0);
if (lockGained.get() != 0)
if (blockingMessage->wasAcquired())
{
mm->threadWithLock = Thread::getCurrentThreadId();
return true;
}
} while (lockIsMandatory);
// we didn't get the lock
blockingMessage->releaseEvent.signal();
{
ScopedLock lock (blockingMessage->ownerCriticalSection);
lockGained.set (0);
blockingMessage->owner.set (nullptr);
if (! lockIsMandatory)
break;
}
// we didn't get the lock
blockingMessage->stopWaiting();
blockingMessage = nullptr;
return false;
}
void MessageManager::Lock::exit() const noexcept
{
if (lockGained.compareAndSetBool (false, true))
if (blockingMessage == nullptr)
return;
const ScopeGuard scope { [&] { blockingMessage = nullptr; } };
blockingMessage->stopWaiting();
if (! blockingMessage->wasAcquired())
return;
if (auto* mm = MessageManager::instance)
{
auto* mm = MessageManager::instance;
jassert (mm == nullptr || mm->currentThreadHasLockedMessageManager());
lockGained.set (0);
if (mm != nullptr)
mm->threadWithLock = {};
if (blockingMessage != nullptr)
{
blockingMessage->releaseEvent.signal();
blockingMessage = nullptr;
}
jassert (mm->currentThreadHasLockedMessageManager());
mm->threadWithLock = {};
}
}
void MessageManager::Lock::messageCallback() const
{
lockGained.set (1);
abort();
}
void MessageManager::Lock::abort() const noexcept
{
abortWait.set (1);
lockedEvent.signal();
const ScopeGuard scope { [&] { condvar.notify_one(); } };
const std::scoped_lock lock { mutex };
abortWait = true;
}
//==============================================================================

View file

@ -298,12 +298,12 @@ public:
friend class ReferenceCountedObjectPtr<BlockingMessage>;
bool tryAcquire (bool) const noexcept;
void messageCallback() const;
//==============================================================================
mutable ReferenceCountedObjectPtr<BlockingMessage> blockingMessage;
WaitableEvent lockedEvent;
mutable Atomic<int> abortWait, lockGained;
mutable std::mutex mutex;
mutable std::condition_variable condvar;
mutable bool abortWait = false, acquired = false;
};
//==============================================================================