Skip to content

Commit

Permalink
#5107: Block the changed signal until parsing is done. When the signa…
Browse files Browse the repository at this point in the history
…l is fired, we must be ready to serve queries to entity classes or model defs without deadlocking in our own ongoing parsing thread.

The change signal is buffered, and should be fired once parsing is done.
  • Loading branch information
codereader committed Jun 25, 2021
1 parent 8a5542c commit fd30a03
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 9 deletions.
2 changes: 1 addition & 1 deletion libs/ThreadedDefLoader.h
Expand Up @@ -49,7 +49,7 @@ class ThreadedDefLoader
ensureLoaderStarted();
}

// Ensrues that the worker thread has been started and is done processing
// Ensures that the worker thread has been started and is done processing
// This will block and wait for the worker execution before returning to the caller.
void ensureFinished()
{
Expand Down
7 changes: 7 additions & 0 deletions radiantcore/eclass/EClassManager.cpp
Expand Up @@ -441,11 +441,18 @@ void EClassManager::parse(TextInputStream& inStr, const vfs::FileInfo& fileInfo,

i->second->setParseStamp(_curParseStamp);

// Hold back the changed signal for the moment being
i->second->blockChangedSignal(true);
// Add this eclass to the queue (we'll fire the signal when we're done parsing)
_changeNotificationQueue.emplace_back(*i->second);

// Parse the contents of the eclass (excluding name)
i->second->parseFromTokens(tokeniser);

// Set the mod directory
i->second->setModName(modDir);

i->second->blockChangedSignal(false);
}
else if (blockType == "model")
{
Expand Down
5 changes: 5 additions & 0 deletions radiantcore/eclass/EClassManager.h
Expand Up @@ -55,6 +55,11 @@ class EClassManager :

sigc::connection _eclassColoursChanged;

// During reparsing, we block all eclass changed signals to avoid deadlocks
// This queue will contain all the eclasses that need to send out their updates
// which will be done immediately after parsing is done
std::vector<std::reference_wrapper<EntityClass>> _changeNotificationQueue;

public:
// Constructor
EClassManager();
Expand Down
10 changes: 4 additions & 6 deletions radiantcore/eclass/EntityClass.cpp
Expand Up @@ -31,10 +31,8 @@ EntityClass::EntityClass(const std::string& name, const vfs::FileInfo& fileInfo,
_inheritanceResolved(false),
_modName("base"),
_emptyAttribute("", "", ""),
_parseStamp(0)
{}

EntityClass::~EntityClass()
_parseStamp(0),
_blockChangeSignal(false)
{}

std::string EntityClass::getName() const
Expand Down Expand Up @@ -109,7 +107,7 @@ void EntityClass::setColour(const Vector3& colour)

_wireShader = fmt::format("<{0:f} {1:f} {2:f}>", _colour[0], _colour[1], _colour[2]);

_changedSignal.emit();
emitChangedSignal();
}

void EntityClass::resetColour()
Expand Down Expand Up @@ -456,7 +454,7 @@ void EntityClass::parseFromTokens(parser::DefTokeniser& tokeniser)
} // while true

// Notify the observers
_changedSignal.emit();
emitChangedSignal();
}

} // namespace eclass
16 changes: 14 additions & 2 deletions radiantcore/eclass/EntityClass.h
Expand Up @@ -86,6 +86,7 @@ class EntityClass

// Emitted when contents are reloaded
sigc::signal<void> _changedSignal;
bool _blockChangeSignal;

private:
// Clear all contents (done before parsing from tokens)
Expand Down Expand Up @@ -131,8 +132,6 @@ class EntityClass
*/
EntityClass(const std::string& name, const vfs::FileInfo& fileInfo, bool fixedSize);

virtual ~EntityClass();

/// Add a new attribute
void addAttribute(const EntityClassAttribute& attribute);

Expand Down Expand Up @@ -209,6 +208,19 @@ class EntityClass
{
return _parseStamp;
}

void emitChangedSignal()
{
if (!_blockChangeSignal)
{
_changedSignal.emit();
}
}

void blockChangedSignal(bool block)
{
_blockChangeSignal = block;
}
};

}

0 comments on commit fd30a03

Please sign in to comment.