Disambiguate null interface case

In some cases, the interface being queried may be persistently nullptr
(e.g. a gem isn't active, or a feature was disabled by a cvar). In this
case, querying the interface would always enter the writer-lock branch
with existing code because nullptr was synonmous with "we haven't
queried for this variable yet."

This PR adds an additional state variable that is set when s_instance is
assigned to, so that after the variable is queried and found to be null,
future queries can enter the read-lock branch.

Signed-off-by: Jeremy Ong <jcong@amazon.com>
monroegm-disable-blank-issue-2
Jeremy Ong 4 years ago
parent a96387ee35
commit 6660c365b9

@ -109,6 +109,7 @@ namespace AZ
*/ */
static EnvironmentVariable<T*> s_instance; static EnvironmentVariable<T*> s_instance;
static AZStd::shared_mutex s_mutex; static AZStd::shared_mutex s_mutex;
static bool s_instanceAssigned;
}; };
template <typename T> template <typename T>
@ -117,6 +118,9 @@ namespace AZ
template <typename T> template <typename T>
AZStd::shared_mutex Interface<T>::s_mutex; AZStd::shared_mutex Interface<T>::s_mutex;
template <typename T>
bool Interface<T>::s_instanceAssigned;
template <typename T> template <typename T>
void Interface<T>::Register(T* type) void Interface<T>::Register(T* type)
{ {
@ -135,18 +139,19 @@ namespace AZ
AZStd::unique_lock<AZStd::shared_mutex> lock(s_mutex); AZStd::unique_lock<AZStd::shared_mutex> lock(s_mutex);
s_instance = Environment::CreateVariable<T*>(GetVariableName()); s_instance = Environment::CreateVariable<T*>(GetVariableName());
s_instance.Get() = type; s_instance.Get() = type;
s_instanceAssigned = true;
} }
template <typename T> template <typename T>
void Interface<T>::Unregister(T* type) void Interface<T>::Unregister(T* type)
{ {
if (!s_instance || !s_instance.Get()) if (!s_instanceAssigned)
{ {
AZ_Assert(false, "Interface '%s' not registered on this module!", AzTypeInfo<T>::Name()); AZ_Assert(false, "Interface '%s' not registered on this module!", AzTypeInfo<T>::Name());
return; return;
} }
if (s_instance.Get() != type) if (s_instance && s_instance.Get() != type)
{ {
AZ_Assert(false, "Interface '%s' is not the same instance that was registered! [Expected '%p', Found '%p']", AzTypeInfo<T>::Name(), type, s_instance.Get()); AZ_Assert(false, "Interface '%s' is not the same instance that was registered! [Expected '%p', Found '%p']", AzTypeInfo<T>::Name(), type, s_instance.Get());
return; return;
@ -156,6 +161,7 @@ namespace AZ
AZStd::unique_lock<AZStd::shared_mutex> lock(s_mutex); AZStd::unique_lock<AZStd::shared_mutex> lock(s_mutex);
*s_instance = nullptr; *s_instance = nullptr;
s_instance.Reset(); s_instance.Reset();
s_instanceAssigned = false;
} }
template <typename T> template <typename T>
@ -165,9 +171,9 @@ namespace AZ
// This is the fast path which won't block. // This is the fast path which won't block.
{ {
AZStd::shared_lock<AZStd::shared_mutex> lock(s_mutex); AZStd::shared_lock<AZStd::shared_mutex> lock(s_mutex);
if (s_instance) if (s_instanceAssigned)
{ {
return s_instance.Get(); return s_instance ? s_instance.Get() : nullptr;
} }
} }
@ -175,6 +181,7 @@ namespace AZ
// take the full lock and request it. // take the full lock and request it.
AZStd::unique_lock<AZStd::shared_mutex> lock(s_mutex); AZStd::unique_lock<AZStd::shared_mutex> lock(s_mutex);
s_instance = Environment::FindVariable<T*>(GetVariableName()); s_instance = Environment::FindVariable<T*>(GetVariableName());
s_instanceAssigned = true;
return s_instance ? s_instance.Get() : nullptr; return s_instance ? s_instance.Get() : nullptr;
} }

Loading…
Cancel
Save