mirror/JUCE
1
0
Fork 0
mirror of https://github.com/juce-framework/JUCE.git synced 2026-02-04 03:40:07 +00:00
Code Activity

Singleton: Fix thread sanitizer warning about race on "instance" data member

Browse source
This commit is contained in:
reuk 2021-10-04 15:35:20 +01:00
parent eb3c3ed27c
commit 4b57909fb0
No known key found for this signature in database
GPG key ID: 9ADCD339CFC98A11
2 changed files with 43 additions and 49 deletions
Download patch file Download diff file Expand all files Collapse all files

2
modules/juce_core/juce_core.h
View file

@ -244,6 +244,7 @@ JUCE_END_IGNORE_WARNINGS_MSVC
#include "memory/juce_ReferenceCountedObject.h"
#include "memory/juce_ScopedPointer.h"
#include "memory/juce_OptionalScopedPointer.h"
#include "containers/juce_ScopedValueSetter.h"
#include "memory/juce_Singleton.h"
#include "memory/juce_WeakReference.h"
#include "threads/juce_ScopedLock.h"
@ -259,7 +260,6 @@ JUCE_END_IGNORE_WARNINGS_MSVC
#include "containers/juce_ListenerList.h"
#include "containers/juce_OwnedArray.h"
#include "containers/juce_ReferenceCountedArray.h"
#include "containers/juce_ScopedValueSetter.h"
#include "containers/juce_SortedSet.h"
#include "containers/juce_SparseSet.h"
#include "containers/juce_AbstractFifo.h"

90
modules/juce_core/memory/juce_Singleton.h
View file

@ -47,53 +47,49 @@ struct SingletonHolder : private MutexType // (inherited so we can use the empt
If you're having trouble cleaning up your singletons, perhaps consider using the
SharedResourcePointer class instead.
*/
jassert (instance == nullptr);
jassert (instance.load() == nullptr);
}
/** Returns the current instance, or creates a new instance if there isn't one. */
Type* get()
{
if (instance == nullptr)
if (auto* ptr = instance.load())
return ptr;
typename MutexType::ScopedLockType sl (*this);
if (auto* ptr = instance.load())
return ptr;
auto once = onlyCreateOncePerRun; // (local copy avoids VS compiler warning about this being constant)
if (once)
{
typename MutexType::ScopedLockType sl (*this);
static bool createdOnceAlready = false;
if (instance == nullptr)
if (createdOnceAlready)
{
auto once = onlyCreateOncePerRun; // (local copy avoids VS compiler warning about this being constant)
if (once)
{
static bool createdOnceAlready = false;
if (createdOnceAlready)
{
// This means that the doNotRecreateAfterDeletion flag was set
// and you tried to create the singleton more than once.
jassertfalse;
return nullptr;
}
createdOnceAlready = true;
}
static bool alreadyInside = false;
if (alreadyInside)
{
// This means that your object's constructor has done something which has
// ended up causing a recursive loop of singleton creation..
jassertfalse;
}
else
{
alreadyInside = true;
getWithoutChecking();
alreadyInside = false;
}
// This means that the doNotRecreateAfterDeletion flag was set
// and you tried to create the singleton more than once.
jassertfalse;
return nullptr;
}
createdOnceAlready = true;
}
return instance;
static bool alreadyInside = false;
if (alreadyInside)
{
// This means that your object's constructor has done something which has
// ended up causing a recursive loop of singleton creation.
jassertfalse;
return nullptr;
}
const ScopedValueSetter<bool> scope (alreadyInside, true);
return getWithoutChecking();
}
/** Returns the current instance, or creates a new instance if there isn't one, but doesn't do
@ -101,32 +97,30 @@ struct SingletonHolder : private MutexType // (inherited so we can use the empt
*/
Type* getWithoutChecking()
{
if (instance == nullptr)
{
auto newObject = new Type(); // (create into a local so that instance is still null during construction)
instance = newObject;
}
if (auto* p = instance.load())
return p;
return instance;
auto* newObject = new Type(); // (create into a local so that instance is still null during construction)
instance.store (newObject);
return newObject;
}
/** Deletes and resets the current instance, if there is one. */
void deleteInstance()
{
typename MutexType::ScopedLockType sl (*this);
auto old = instance;
instance = nullptr;
delete old;
delete instance.exchange (nullptr);
}
/** Called by the class's destructor to clear the pointer if it is currently set to the given object. */
void clear (Type* expectedObject) noexcept
{
if (instance == expectedObject)
instance = nullptr;
instance.compare_exchange_strong (expectedObject, nullptr);
}
Type* instance = nullptr;
// This must be atomic, otherwise a late call to get() may attempt to read instance while it is
// being modified by the very first call to get().
std::atomic<Type*> instance { nullptr };
};