Mirrors/overte-thingvellir
3
0
Fork 0
mirror of https://thingvellir.net/git/overte synced 2025-03-27 23:52:03 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

Merge pull request #14018 from birarda/bug/avatar-hash-map-deadlock-master

Browse source 
fix for deadlock in other avatar entity removal from tree
This commit is contained in:
John Conklin II 2018-09-26 09:58:26 -07:00 committed by GitHub
commit 0de433c7c7
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 79 additions and 37 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

44
interface/src/avatar/AvatarManager.cpp
View file

@ -456,31 +456,37 @@ void AvatarManager::handleRemovedAvatar(const AvatarSharedPointer& removedAvatar
}
void AvatarManager::clearOtherAvatars() {
// Remove other avatars from the world but don't actually remove them from _avatarHash
// each will either be removed on timeout or will re-added to the world on receipt of update.
const render::ScenePointer& scene = qApp->getMain3DScene();
render::Transaction transaction;
QReadLocker locker(&_hashLock);
AvatarHash::iterator avatarIterator = _avatarHash.begin();
while (avatarIterator != _avatarHash.end()) {
auto avatar = std::static_pointer_cast<Avatar>(avatarIterator.value());
if (avatar != _myAvatar) {
handleRemovedAvatar(avatar);
avatarIterator = _avatarHash.erase(avatarIterator);
} else {
++avatarIterator;
}
}
assert(scene);
scene->enqueueTransaction(transaction);
_myAvatar->clearLookAtTargetAvatar();
// setup a vector of removed avatars outside the scope of the hash lock
std::vector<AvatarSharedPointer> removedAvatars;
{
QWriteLocker locker(&_hashLock);
removedAvatars.reserve(_avatarHash.size());
auto avatarIterator = _avatarHash.begin();
while (avatarIterator != _avatarHash.end()) {
auto avatar = std::static_pointer_cast<Avatar>(avatarIterator.value());
if (avatar != _myAvatar) {
removedAvatars.push_back(avatar);
avatarIterator = _avatarHash.erase(avatarIterator);
} else {
++avatarIterator;
}
}
}
for (auto& av : removedAvatars) {
handleRemovedAvatar(av);
}
}
void AvatarManager::deleteAllAvatars() {
assert(_avatarsToChangeInPhysics.empty());
QReadLocker locker(&_hashLock);
QWriteLocker locker(&_hashLock);
AvatarHash::iterator avatarIterator = _avatarHash.begin();
while (avatarIterator != _avatarHash.end()) {
auto avatar = std::static_pointer_cast<OtherAvatar>(avatarIterator.value());

7
interface/src/avatar/AvatarManager.h
View file

@ -204,7 +204,12 @@ private:
void simulateAvatarFades(float deltaTime);
AvatarSharedPointer newSharedAvatar() override;
void handleRemovedAvatar(const AvatarSharedPointer& removedAvatar, KillAvatarReason removalReason = KillAvatarReason::NoReason) override;
// called only from the AvatarHashMap thread - cannot be called while this thread holds the
// hash lock, since handleRemovedAvatar needs a write lock on the entity tree and the entity tree
// frequently grabs a read lock on the hash to get a given avatar by ID
void handleRemovedAvatar(const AvatarSharedPointer& removedAvatar,
KillAvatarReason removalReason = KillAvatarReason::NoReason) override;
QVector<AvatarSharedPointer> _avatarsToFade;

62
libraries/avatars/src/AvatarHashMap.cpp
View file

@ -66,6 +66,22 @@ void AvatarReplicas::removeReplicas(const QUuid& parentID) {
}
}
std::vector<AvatarSharedPointer> AvatarReplicas::takeReplicas(const QUuid& parentID) {
std::vector<AvatarSharedPointer> replicas;
auto it = _replicasMap.find(parentID);
if (it != _replicasMap.end()) {
// take a copy of the replica shared pointers for this parent
replicas.swap(it->second);
// erase the replicas for this parent from our map
_replicasMap.erase(it);
}
return replicas;
}
void AvatarReplicas::processAvatarIdentity(const QUuid& parentID, const QByteArray& identityData, bool& identityChanged, bool& displayNameChanged) {
if (_replicasMap.find(parentID) != _replicasMap.end()) {
auto &replicas = _replicasMap[parentID];
@ -386,24 +402,31 @@ void AvatarHashMap::processKillAvatar(QSharedPointer<ReceivedMessage> message, S
}
void AvatarHashMap::removeAvatar(const QUuid& sessionUUID, KillAvatarReason removalReason) {
QWriteLocker locker(&_hashLock);
std::vector<AvatarSharedPointer> removedAvatars;
auto replicaIDs = _replicas.getReplicaIDs(sessionUUID);
_replicas.removeReplicas(sessionUUID);
for (auto id : replicaIDs) {
auto removedReplica = _avatarHash.take(id);
if (removedReplica) {
handleRemovedAvatar(removedReplica, removalReason);
{
QWriteLocker locker(&_hashLock);
auto replicas = _replicas.takeReplicas(sessionUUID);
for (auto& replica : replicas) {
auto removedReplica = _avatarHash.take(replica->getID());
if (removedReplica) {
removedAvatars.push_back(removedReplica);
}
}
_pendingAvatars.remove(sessionUUID);
auto removedAvatar = _avatarHash.take(sessionUUID);
if (removedAvatar) {
removedAvatars.push_back(removedAvatar);
}
}
_pendingAvatars.remove(sessionUUID);
auto removedAvatar = _avatarHash.take(sessionUUID);
if (removedAvatar) {
for (auto& removedAvatar: removedAvatars) {
handleRemovedAvatar(removedAvatar, removalReason);
}
}
void AvatarHashMap::handleRemovedAvatar(const AvatarSharedPointer& removedAvatar, KillAvatarReason removalReason) {
@ -421,11 +444,18 @@ void AvatarHashMap::sessionUUIDChanged(const QUuid& sessionUUID, const QUuid& ol
}
void AvatarHashMap::clearOtherAvatars() {
QWriteLocker locker(&_hashLock);
QList<AvatarSharedPointer> removedAvatars;
for (auto& av : _avatarHash) {
handleRemovedAvatar(av);
{
QWriteLocker locker(&_hashLock);
// grab a copy of the current avatars so we can call handleRemoveAvatar for them
removedAvatars = _avatarHash.values();
_avatarHash.clear();
}
_avatarHash.clear();
for (auto& av : removedAvatars) {
handleRemovedAvatar(av);
}
}

3
libraries/avatars/src/AvatarHashMap.h
View file

@ -49,6 +49,7 @@ public:
void parseDataFromBuffer(const QUuid& parentID, const QByteArray& buffer);
void processAvatarIdentity(const QUuid& parentID, const QByteArray& identityData, bool& identityChanged, bool& displayNameChanged);
void removeReplicas(const QUuid& parentID);
std::vector<AvatarSharedPointer> takeReplicas(const QUuid& parentID);
void processTrait(const QUuid& parentID, AvatarTraits::TraitType traitType, QByteArray traitBinaryData);
void processDeletedTraitInstance(const QUuid& parentID, AvatarTraits::TraitType traitType, AvatarTraits::TraitInstanceID instanceID);
void processTraitInstance(const QUuid& parentID, AvatarTraits::TraitType traitType,
@ -179,7 +180,7 @@ protected:
bool& isNew);
virtual AvatarSharedPointer findAvatar(const QUuid& sessionUUID) const; // uses a QReadLocker on the hashLock
virtual void removeAvatar(const QUuid& sessionUUID, KillAvatarReason removalReason = KillAvatarReason::NoReason);
virtual void handleRemovedAvatar(const AvatarSharedPointer& removedAvatar, KillAvatarReason removalReason = KillAvatarReason::NoReason);
AvatarHash _avatarHash;