livie
/
phoenix-firestorm
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

DRTVWR-493: Permit LLParamSingleton::initSingleton() circularity. 

This was forbidden, but AndreyK points out cases in which LLParamSingleton::
initSingleton() should in fact be allowed to circle back to its own instance()
method. Use a recursive_mutex instead of plain mutex to permit that; remove
LL_ERRS preventing it.

Add LLParamSingleton::instance() method that calls
LLParamSingleton::getInstance(). Inheriting LLSingleton::instance() called
LLSingleton::getInstance() -- not at all what we want.

Add LLParamSingleton unit tests.
master
Nat Goodspeed 2019-08-12 17:35:45 -04:00
parent 5a72d34b76
commit 4fce6dc435
2 changed files with 144 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
indra/llcommon/llsingleton.h
View File

@ -491,9 +491,6 @@ typename LLSingleton<T>::SingletonData LLSingleton<T>::sData;
* * However, distinct initParamSingleton() calls can be used to engage
* different constructors, as long as only one such call is executed at
* runtime.
* * Circularity is not permitted. No LLSingleton referenced by an
* LLParamSingleton's constructor or initSingleton() method may call this
* LLParamSingleton's instance() or getInstance() methods.
* * Unlike LLSingleton, an LLParamSingleton cannot be "revived" by an
* instance() or getInstance() call after deleteSingleton().
*
@ -508,7 +505,6 @@ private:
public:
using super::deleteSingleton;
using super::instance;
using super::instanceExists;
using super::wasDeleted;
@ -519,7 +515,7 @@ public:
// In case racing threads both call initParamSingleton() at the same
// time, serialize them. One should initialize; the other should see
// mInitState already set.
std::unique_lock<std::mutex> lk(mMutex);
std::unique_lock<decltype(mMutex)> lk(mMutex);
// For organizational purposes this function shouldn't be called twice
if (super::sData.mInitState != super::UNINITIALIZED)
{
@ -538,7 +534,7 @@ public:
{
// In case racing threads call getInstance() at the same moment as
// initParamSingleton(), serialize the calls.
std::unique_lock<std::mutex> lk(mMutex);
std::unique_lock<decltype(mMutex)> lk(mMutex);
switch (super::sData.mInitState)
{
@ -550,15 +546,10 @@ public:
case super::CONSTRUCTING:
super::logerrs("Tried to access param singleton ",
super::demangle(typeid(DERIVED_TYPE).name()).c_str(),
" from singleton constructor!");
" from singleton constructor!");
break;
case super::INITIALIZING:
super::logerrs("Tried to access param singleton ",
super::demangle(typeid(DERIVED_TYPE).name()).c_str(),
" from initSingleton() method!");
break;
case super::INITIALIZED:
return super::sData.mInstance;
@ -573,12 +564,23 @@ public:
return nullptr;
}
// instance() is replicated here so it calls
// LLParamSingleton::getInstance() rather than LLSingleton::getInstance()
// -- avoid making getInstance() virtual
static DERIVED_TYPE& instance()
{
return *getInstance();
}
private:
static std::mutex mMutex;
// Use a recursive_mutex in case of constructor circularity. With a
// non-recursive mutex, that would result in deadlock rather than the
// logerrs() call coded above.
static std::recursive_mutex mMutex;
};
template<typename T>
typename std::mutex LLParamSingleton<T>::mMutex;
typename std::recursive_mutex LLParamSingleton<T>::mMutex;
/**
* Initialization locked singleton, only derived class can decide when to initialize.

129
indra/llcommon/tests/llsingleton_test.cpp
View File

@ -29,7 +29,8 @@
#include "llsingleton.h"
#include "../test/lltut.h"
#include "wrapllerrs.h"
#include "llsd.h"
// Capture execution sequence by appending to log string.
std::string sLog;
@ -198,4 +199,130 @@ namespace tut
TESTS(A, B, 4, 5, 6, 7)
TESTS(B, A, 8, 9, 10, 11)
#define PARAMSINGLETON(cls) \
class cls: public LLParamSingleton<cls> \
{ \
LLSINGLETON(cls, const LLSD::String& str): mDesc(str) {} \
cls(LLSD::Integer i): mDesc(i) {} \
\
public: \
std::string desc() const { return mDesc.asString(); } \
\
private: \
LLSD mDesc; \
}
// Declare two otherwise-identical LLParamSingleton classes so we can
// validly initialize each using two different constructors. If we tried
// to test that with a single LLParamSingleton class within the same test
// program, we'd get 'trying to use deleted LLParamSingleton' errors.
PARAMSINGLETON(PSing1);
PARAMSINGLETON(PSing2);
template<> template<>
void singleton_object_t::test<12>()
{
set_test_name("LLParamSingleton");
WrapLLErrs catcherr;
// query methods
ensure("false positive on instanceExists()", ! PSing1::instanceExists());
ensure("false positive on wasDeleted()", ! PSing1::wasDeleted());
// try to reference before initializing
std::string threw = catcherr.catch_llerrs([](){
(void)PSing1::instance();
});
ensure_contains("too-early instance() didn't throw", threw, "Uninitialized");
// getInstance() behaves the same as instance()
threw = catcherr.catch_llerrs([](){
(void)PSing1::getInstance();
});
ensure_contains("too-early getInstance() didn't throw", threw, "Uninitialized");
// initialize using LLSD::String constructor
PSing1::initParamSingleton("string");
ensure_equals(PSing1::instance().desc(), "string");
ensure("false negative on instanceExists()", PSing1::instanceExists());
// try to initialize again
threw = catcherr.catch_llerrs([](){
PSing1::initParamSingleton("again");
});
ensure_contains("second ctor(string) didn't throw", threw, "twice");
// try to initialize using the other constructor -- should be
// well-formed, but illegal at runtime
threw = catcherr.catch_llerrs([](){
PSing1::initParamSingleton(17);
});
ensure_contains("other ctor(int) didn't throw", threw, "twice");
PSing1::deleteSingleton();
ensure("false negative on wasDeleted()", PSing1::wasDeleted());
threw = catcherr.catch_llerrs([](){
(void)PSing1::instance();
});
ensure_contains("accessed deleted LLParamSingleton", threw, "deleted");
}
template<> template<>
void singleton_object_t::test<13>()
{
set_test_name("LLParamSingleton alternate ctor");
WrapLLErrs catcherr;
// We don't have to restate all the tests for PSing1. Only test validly
// using the other constructor.
PSing2::initParamSingleton(17);
ensure_equals(PSing2::instance().desc(), "17");
// can't do it twice
std::string threw = catcherr.catch_llerrs([](){
PSing2::initParamSingleton(34);
});
ensure_contains("second ctor(int) didn't throw", threw, "twice");
// can't use the other constructor either
threw = catcherr.catch_llerrs([](){
PSing2::initParamSingleton("string");
});
ensure_contains("other ctor(string) didn't throw", threw, "twice");
}
class CircularPCtor: public LLParamSingleton<CircularPCtor>
{
LLSINGLETON(CircularPCtor)
{
// never mind indirection, just go straight for the circularity
(void)instance();
}
};
template<> template<>
void singleton_object_t::test<14>()
{
set_test_name("Circular LLParamSingleton constructor");
WrapLLErrs catcherr;
std::string threw = catcherr.catch_llerrs([](){
CircularPCtor::initParamSingleton();
});
ensure_contains("constructor circularity didn't throw", threw, "constructor");
}
class CircularPInit: public LLParamSingleton<CircularPInit>
{
LLSINGLETON_EMPTY_CTOR(CircularPInit);
public:
virtual void initSingleton()
{
// never mind indirection, just go straight for the circularity
(void)instance();
}
};
template<> template<>
void singleton_object_t::test<15>()
{
set_test_name("Circular LLParamSingleton initSingleton()");
WrapLLErrs catcherr;
std::string threw = catcherr.catch_llerrs([](){
CircularPInit::initParamSingleton();
});
ensure("initSingleton() circularity threw", threw.empty());
}
}