From 6f2009f33db8c75bd475556221d9ab5e56195a26 Mon Sep 17 00:00:00 2001
From: Kitty Barnett <develop@catznip.com>
Date: Wed, 17 Jun 2020 15:19:29 +0200
Subject: [PATCH] [FIXED] hasBehaviourExcept() with an empty option returns
false even though a RLV_OPTION_MODIFIER active restriction exists ->
RlvCommand::markRefCounted() is called from
RlvBehaviourGenericProcessor<RLV_OPTION_MODIFIER> as expected -> However,
the RlvCommand reference we pass throughout the entire command processing
line is a reference to the originally parsed command and not a reference to
the copy that was stored in the RlvObject's command list -> The fact that
we're mutating a constant is excusable in this case since only the command
processor will know whether an option command should end up getting reference
counted -> This bug only applies to hasBehaviourExcept() since the
behaviour reference counter was still incremented/decremented successfully
and hasBehaviourExcept() does a deep check on the individual commands instead
-> Fixes FIRE-24215
---
indra/newview/rlvhandler.cpp | 63 +++++++++++++++++++-----------------
indra/newview/rlvhandler.h | 6 ++--
indra/newview/rlvhelper.cpp | 14 ++++----
indra/newview/rlvhelper.h | 4 +--
4 files changed, 47 insertions(+), 40 deletions(-)
diff --git a/indra/newview/rlvhandler.cpp b/indra/newview/rlvhandler.cpp
index 09143d10dfb..68f0c364ebf 100644
--- a/indra/newview/rlvhandler.cpp
+++ b/indra/newview/rlvhandler.cpp
@@ -438,42 +438,46 @@ bool RlvHandler::notifyCommandHandlers(rlvExtCommandHandler f, const RlvCommand&
}
// Checked: 2009-11-25 (RLVa-1.1.0f) | Modified: RLVa-1.1.0f
-ERlvCmdRet RlvHandler::processCommand(const RlvCommand& rlvCmd, bool fFromObj)
+ERlvCmdRet RlvHandler::processCommand(std::reference_wrapper<const RlvCommand> rlvCmd, bool fFromObj)
{
- RLV_DEBUGS << "[" << rlvCmd.getObjectID() << "]: " << rlvCmd.asString() << RLV_ENDL;
-
- if ( (isBlockedObject(rlvCmd.getObjectID())) && (RLV_TYPE_REMOVE != rlvCmd.getParamType()) && (RLV_TYPE_CLEAR != rlvCmd.getParamType()) )
- {
- RLV_DEBUGS << "\t-> blocked object" << RLV_ENDL;
- return RLV_RET_FAILED_BLOCKED;
- }
- if (!rlvCmd.isValid())
{
- RLV_DEBUGS << "\t-> invalid syntax" << RLV_ENDL;
- return RLV_RET_FAILED_SYNTAX;
- }
- if (rlvCmd.isBlocked())
- {
- RLV_DEBUGS << "\t-> blocked command" << RLV_ENDL;
- return RLV_RET_FAILED_DISABLED;
+ const RlvCommand& rlvCmdTmp = rlvCmd; // Reference to the temporary with limited variable scope since we don't want it to leak below
+
+ RLV_DEBUGS << "[" << rlvCmdTmp.getObjectID() << "]: " << rlvCmdTmp.asString() << RLV_ENDL;
+
+ if ( (isBlockedObject(rlvCmdTmp.getObjectID())) && (RLV_TYPE_REMOVE != rlvCmdTmp.getParamType()) && (RLV_TYPE_CLEAR != rlvCmdTmp.getParamType()) )
+ {
+ RLV_DEBUGS << "\t-> blocked object" << RLV_ENDL;
+ return RLV_RET_FAILED_BLOCKED;
+ }
+ if (!rlvCmdTmp.isValid())
+ {
+ RLV_DEBUGS << "\t-> invalid syntax" << RLV_ENDL;
+ return RLV_RET_FAILED_SYNTAX;
+ }
+ if (rlvCmdTmp.isBlocked())
+ {
+ RLV_DEBUGS << "\t-> blocked command" << RLV_ENDL;
+ return RLV_RET_FAILED_DISABLED;
+ }
}
// Using a stack for executing commands solves a few problems:
// - if we passed RlvObject::m_idObj for idObj somewhere and process a @clear then idObj points to invalid/cleared memory at the end
// - if command X triggers command Y along the way then getCurrentCommand()/getCurrentObject() still return Y even when finished
- m_CurCommandStack.push(&rlvCmd); m_CurObjectStack.push(rlvCmd.getObjectID());
+ m_CurCommandStack.push(rlvCmd); m_CurObjectStack.push(rlvCmd.get().getObjectID());
const LLUUID& idCurObj = m_CurObjectStack.top();
ERlvCmdRet eRet = RLV_RET_UNKNOWN;
- switch (rlvCmd.getParamType())
+ switch (rlvCmd.get().getParamType())
{
case RLV_TYPE_ADD: // Checked: 2009-11-26 (RLVa-1.1.0f) | Modified: RLVa-1.1.0f
{
- if ( (m_Behaviours[rlvCmd.getBehaviourType()]) &&
- ( (RLV_BHVR_SETCAM == rlvCmd.getBehaviourType()) || (RLV_BHVR_SETDEBUG == rlvCmd.getBehaviourType()) || (RLV_BHVR_SETENV == rlvCmd.getBehaviourType()) ) )
+ ERlvBehaviour eBhvr = rlvCmd.get().getBehaviourType();
+ if ( (m_Behaviours[eBhvr]) && ( (RLV_BHVR_SETCAM == eBhvr) || (RLV_BHVR_SETDEBUG == eBhvr) || (RLV_BHVR_SETENV == eBhvr) ) )
{
// Some restrictions can only be held by one single object to avoid deadlocks
- RLV_DEBUGS << "\t- " << rlvCmd.getBehaviour() << " is already set by another object => discarding" << RLV_ENDL;
+ RLV_DEBUGS << "\t- " << rlvCmd.get().getBehaviour() << " is already set by another object => discarding" << RLV_ENDL;
eRet = RLV_RET_FAILED_LOCK;
break;
}
@@ -481,14 +485,14 @@ ERlvCmdRet RlvHandler::processCommand(const RlvCommand& rlvCmd, bool fFromObj)
rlv_object_map_t::iterator itObj = m_Objects.find(idCurObj); bool fAdded = false;
if (itObj != m_Objects.end())
{
- RlvObject& rlvObj = itObj->second;
- fAdded = rlvObj.addCommand(rlvCmd);
+ // Add the command to an existing object
+ rlvCmd = itObj->second.addCommand(rlvCmd, fAdded);
}
else
{
- RlvObject rlvObj(idCurObj);
- fAdded = rlvObj.addCommand(rlvCmd);
- itObj = m_Objects.insert(std::pair<LLUUID, RlvObject>(idCurObj, rlvObj)).first;
+ // Create a new RLV object and then add the command to it (and grab its reference)
+ itObj = m_Objects.insert(std::pair<LLUUID, RlvObject>(idCurObj, RlvObject(idCurObj))).first;
+ rlvCmd = itObj->second.addCommand(rlvCmd, fAdded);
}
RLV_DEBUGS << "\t- " << ( (fAdded) ? "adding behaviour" : "skipping duplicate" ) << RLV_ENDL;
@@ -563,12 +567,13 @@ ERlvCmdRet RlvHandler::processCommand(const RlvCommand& rlvCmd, bool fFromObj)
// Checked: 2009-11-25 (RLVa-1.1.0f) | Modified: RLVa-1.1.0f
ERlvCmdRet RlvHandler::processCommand(const LLUUID& idObj, const std::string& strCommand, bool fFromObj)
{
+ const RlvCommand rlvCmd(idObj, strCommand);
if (STATE_STARTED != LLStartUp::getStartupState())
{
- m_Retained.push_back(RlvCommand(idObj, strCommand));
+ m_Retained.push_back(rlvCmd);
return RLV_RET_RETAINED;
}
- return processCommand(RlvCommand(idObj, strCommand), fFromObj);
+ return processCommand(std::ref(rlvCmd), fFromObj);
}
// Checked: 2010-02-27 (RLVa-1.2.0a) | Modified: RLVa-1.1.0f
@@ -583,7 +588,7 @@ void RlvHandler::processRetainedCommands(ERlvBehaviour eBhvrFilter /*=RLV_BHVR_U
if ( ((RLV_BHVR_UNKNOWN == eBhvrFilter) || (rlvCmd.getBehaviourType() == eBhvrFilter)) &&
((RLV_TYPE_UNKNOWN == eTypeFilter) || (rlvCmd.getParamType() == eTypeFilter)) )
{
- processCommand(rlvCmd, true);
+ processCommand(std::ref(rlvCmd), true);
m_Retained.erase(itCurCmd);
}
}
diff --git a/indra/newview/rlvhandler.h b/indra/newview/rlvhandler.h
index 5a6e5508eb5..06c18fdc539 100644
--- a/indra/newview/rlvhandler.h
+++ b/indra/newview/rlvhandler.h
@@ -133,7 +133,7 @@ class RlvHandler : public LLOldEvents::LLSimpleListener, public LLParticularGrou
bool processIMQuery(const LLUUID& idSender, const std::string& strCommand);
// Returns a pointer to the currently executing command (do *not* save this pointer)
- const RlvCommand* getCurrentCommand() const { return (!m_CurCommandStack.empty()) ? m_CurCommandStack.top() : NULL; }
+ const RlvCommand* getCurrentCommand() const { return (!m_CurCommandStack.empty()) ? &m_CurCommandStack.top().get() : nullptr; }
// Returns the UUID of the object we're currently executing a command for
const LLUUID& getCurrentObject() const { return (!m_CurObjectStack.empty()) ? m_CurObjectStack.top() : LLUUID::null; }
@@ -203,7 +203,7 @@ class RlvHandler : public LLOldEvents::LLSimpleListener, public LLParticularGrou
* Command processing
*/
protected:
- ERlvCmdRet processCommand(const RlvCommand& rlvCmd, bool fFromObj);
+ ERlvCmdRet processCommand(std::reference_wrapper<const RlvCommand> rlvCmdRef, bool fFromObj);
ERlvCmdRet processClearCommand(const RlvCommand& rlvCmd);
// Command handlers (RLV_TYPE_ADD and RLV_TYPE_CLEAR)
@@ -244,7 +244,7 @@ class RlvHandler : public LLOldEvents::LLSimpleListener, public LLParticularGrou
rlv_command_list_t m_Retained;
RlvGCTimer* m_pGCTimer;
- std::stack<const RlvCommand*> m_CurCommandStack;// Convenience (see @tpto)
+ std::stack<std::reference_wrapper<const RlvCommand>> m_CurCommandStack; // Convenience (see @tpto)
std::stack<LLUUID> m_CurObjectStack; // Convenience (see @tpto)
rlv_behaviour_signal_t m_OnBehaviour;
diff --git a/indra/newview/rlvhelper.cpp b/indra/newview/rlvhelper.cpp
index 4a4ab3c7c0b..3658f2daafa 100644
--- a/indra/newview/rlvhelper.cpp
+++ b/indra/newview/rlvhelper.cpp
@@ -642,8 +642,9 @@ RlvCommand::RlvCommand(const LLUUID& idObj, const std::string& strCommand)
}
RlvCommand::RlvCommand(const RlvCommand& rlvCmd, ERlvParamType eParamType)
- : m_fValid(rlvCmd.m_fValid), m_idObj(rlvCmd.m_idObj), m_strBehaviour(rlvCmd.m_strBehaviour), m_pBhvrInfo(rlvCmd.m_pBhvrInfo),
- m_eParamType( (RLV_TYPE_UNKNOWN == eParamType) ? rlvCmd.m_eParamType : eParamType),m_fStrict(rlvCmd.m_fStrict), m_strOption(rlvCmd.m_strOption), m_strParam(rlvCmd.m_strParam), m_fRefCounted(false)
+ : m_fValid(rlvCmd.m_fValid), m_idObj(rlvCmd.m_idObj), m_strBehaviour(rlvCmd.m_strBehaviour), m_pBhvrInfo(rlvCmd.m_pBhvrInfo)
+ , m_eParamType( (RLV_TYPE_UNKNOWN == eParamType) ? rlvCmd.m_eParamType : eParamType),m_fStrict(rlvCmd.m_fStrict), m_strOption(rlvCmd.m_strOption)
+ , m_strParam(rlvCmd.m_strParam), m_fRefCounted(rlvCmd.m_fRefCounted)
{
}
@@ -989,7 +990,7 @@ RlvObject::RlvObject(const LLUUID& idObj) : m_idObj(idObj), m_nLookupMisses(0)
m_idRoot = (pObj) ? pObj->getRootEdit()->getID() : LLUUID::null;
}
-bool RlvObject::addCommand(const RlvCommand& rlvCmd)
+const RlvCommand& RlvObject::addCommand(const RlvCommand& rlvCmd, bool& fAdded)
{
RLV_ASSERT(RLV_TYPE_ADD == rlvCmd.getParamType());
@@ -999,14 +1000,15 @@ bool RlvObject::addCommand(const RlvCommand& rlvCmd)
if ( (itCmd->getBehaviour() == rlvCmd.getBehaviour()) && (itCmd->getOption() == rlvCmd.getOption()) &&
(itCmd->isStrict() == rlvCmd.isStrict() ) )
{
- return false;
+ fAdded = false;
+ return *itCmd;
}
}
// Now that we know it's not a duplicate, add it to the end of the list
m_Commands.push_back(rlvCmd);
-
- return true;
+ fAdded = true;
+ return m_Commands.back();
}
bool RlvObject::removeCommand(const RlvCommand& rlvCmd)
diff --git a/indra/newview/rlvhelper.h b/indra/newview/rlvhelper.h
index 6cf2c5c62eb..02d682596a1 100644
--- a/indra/newview/rlvhelper.h
+++ b/indra/newview/rlvhelper.h
@@ -434,8 +434,8 @@ class RlvObject
* Member functions
*/
public:
- bool addCommand(const RlvCommand& rlvCmd);
- bool removeCommand(const RlvCommand& rlvCmd);
+ const RlvCommand& addCommand(const RlvCommand& rlvCmd, bool& fAdded);
+ bool removeCommand(const RlvCommand& rlvCmd);
std::string getStatusString(const std::string& strFilter, const std::string& strSeparator) const;
bool hasBehaviour(ERlvBehaviour eBehaviour, bool fStrictOnly) const;
--
GitLab