From 2dc4922da35f9d5674caf15169c498bfae4fdce1 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Thu, 9 Jul 2015 15:23:05 -0700 Subject: [PATCH] bring over code from entity-level-locking branch --- .../src/AssignmentActionFactory.cpp | 7 +- .../src/AssignmentActionFactory.h | 7 +- interface/src/InterfaceActionFactory.cpp | 7 +- interface/src/InterfaceActionFactory.h | 6 +- interface/src/avatar/AvatarActionHold.cpp | 16 +- .../src/EntityActionFactoryInterface.h | 6 +- libraries/entities/src/EntityItem.cpp | 272 ++++++++++++------ libraries/entities/src/EntityItem.h | 39 ++- .../entities/src/EntityScriptingInterface.cpp | 2 +- libraries/entities/src/EntitySimulation.cpp | 1 + 10 files changed, 243 insertions(+), 120 deletions(-) diff --git a/assignment-client/src/AssignmentActionFactory.cpp b/assignment-client/src/AssignmentActionFactory.cpp index e1c5d3adff..7c404cbd97 100644 --- a/assignment-client/src/AssignmentActionFactory.cpp +++ b/assignment-client/src/AssignmentActionFactory.cpp @@ -17,8 +17,7 @@ EntityActionPointer assignmentActionFactory(EntityActionType type, const QUuid& } -EntityActionPointer AssignmentActionFactory::factory(EntitySimulation* simulation, - EntityActionType type, +EntityActionPointer AssignmentActionFactory::factory(EntityActionType type, const QUuid& id, EntityItemPointer ownerEntity, QVariantMap arguments) { @@ -33,9 +32,7 @@ EntityActionPointer AssignmentActionFactory::factory(EntitySimulation* simulatio } -EntityActionPointer AssignmentActionFactory::factoryBA(EntitySimulation* simulation, - EntityItemPointer ownerEntity, - QByteArray data) { +EntityActionPointer AssignmentActionFactory::factoryBA(EntityItemPointer ownerEntity, QByteArray data) { QDataStream serializedActionDataStream(data); EntityActionType type; QUuid id; diff --git a/assignment-client/src/AssignmentActionFactory.h b/assignment-client/src/AssignmentActionFactory.h index 41245dac68..e2d58f3e6a 100644 --- a/assignment-client/src/AssignmentActionFactory.h +++ b/assignment-client/src/AssignmentActionFactory.h @@ -19,14 +19,11 @@ class AssignmentActionFactory : public EntityActionFactoryInterface { public: AssignmentActionFactory() : EntityActionFactoryInterface() { } virtual ~AssignmentActionFactory() { } - virtual EntityActionPointer factory(EntitySimulation* simulation, - EntityActionType type, + virtual EntityActionPointer factory(EntityActionType type, const QUuid& id, EntityItemPointer ownerEntity, QVariantMap arguments); - virtual EntityActionPointer factoryBA(EntitySimulation* simulation, - EntityItemPointer ownerEntity, - QByteArray data); + virtual EntityActionPointer factoryBA(EntityItemPointer ownerEntity, QByteArray data); }; #endif // hifi_AssignmentActionFactory_h diff --git a/interface/src/InterfaceActionFactory.cpp b/interface/src/InterfaceActionFactory.cpp index 363fb66e76..dca1015ecc 100644 --- a/interface/src/InterfaceActionFactory.cpp +++ b/interface/src/InterfaceActionFactory.cpp @@ -35,8 +35,7 @@ EntityActionPointer interfaceActionFactory(EntityActionType type, const QUuid& i } -EntityActionPointer InterfaceActionFactory::factory(EntitySimulation* simulation, - EntityActionType type, +EntityActionPointer InterfaceActionFactory::factory(EntityActionType type, const QUuid& id, EntityItemPointer ownerEntity, QVariantMap arguments) { @@ -51,9 +50,7 @@ EntityActionPointer InterfaceActionFactory::factory(EntitySimulation* simulation } -EntityActionPointer InterfaceActionFactory::factoryBA(EntitySimulation* simulation, - EntityItemPointer ownerEntity, - QByteArray data) { +EntityActionPointer InterfaceActionFactory::factoryBA(EntityItemPointer ownerEntity, QByteArray data) { QDataStream serializedArgumentStream(data); EntityActionType type; QUuid id; diff --git a/interface/src/InterfaceActionFactory.h b/interface/src/InterfaceActionFactory.h index 004c24163f..2031f5c57a 100644 --- a/interface/src/InterfaceActionFactory.h +++ b/interface/src/InterfaceActionFactory.h @@ -18,13 +18,11 @@ class InterfaceActionFactory : public EntityActionFactoryInterface { public: InterfaceActionFactory() : EntityActionFactoryInterface() { } virtual ~InterfaceActionFactory() { } - virtual EntityActionPointer factory(EntitySimulation* simulation, - EntityActionType type, + virtual EntityActionPointer factory(EntityActionType type, const QUuid& id, EntityItemPointer ownerEntity, QVariantMap arguments); - virtual EntityActionPointer factoryBA(EntitySimulation* simulation, - EntityItemPointer ownerEntity, + virtual EntityActionPointer factoryBA(EntityItemPointer ownerEntity, QByteArray data); }; diff --git a/interface/src/avatar/AvatarActionHold.cpp b/interface/src/avatar/AvatarActionHold.cpp index 72019ecf28..ba37112fe1 100644 --- a/interface/src/avatar/AvatarActionHold.cpp +++ b/interface/src/avatar/AvatarActionHold.cpp @@ -85,15 +85,17 @@ void AvatarActionHold::updateActionWorker(float deltaTimeStep) { return; } + if (_positionalTarget != position || _rotationalTarget != rotation) { + auto ownerEntity = _ownerEntity.lock(); + if (ownerEntity) { + ownerEntity->setActionDataDirty(true); + } + } + _positionalTarget = position; _rotationalTarget = rotation; unlock(); - auto ownerEntity = _ownerEntity.lock(); - if (ownerEntity) { - ownerEntity->setActionDataDirty(true); - } - ObjectActionSpring::updateActionWorker(deltaTimeStep); } @@ -152,10 +154,6 @@ bool AvatarActionHold::updateArguments(QVariantMap arguments) { _rotationalTargetSet = true; _active = true; unlock(); - auto ownerEntity = _ownerEntity.lock(); - if (ownerEntity) { - ownerEntity->setActionDataDirty(true); - } return true; } diff --git a/libraries/entities/src/EntityActionFactoryInterface.h b/libraries/entities/src/EntityActionFactoryInterface.h index 9f4056cdff..adff1a53ba 100644 --- a/libraries/entities/src/EntityActionFactoryInterface.h +++ b/libraries/entities/src/EntityActionFactoryInterface.h @@ -23,13 +23,11 @@ class EntityActionFactoryInterface : public QObject, public Dependency { public: EntityActionFactoryInterface() { } virtual ~EntityActionFactoryInterface() { } - virtual EntityActionPointer factory(EntitySimulation* simulation, - EntityActionType type, + virtual EntityActionPointer factory(EntityActionType type, const QUuid& id, EntityItemPointer ownerEntity, QVariantMap arguments) { assert(false); return nullptr; } - virtual EntityActionPointer factoryBA(EntitySimulation* simulation, - EntityItemPointer ownerEntity, + virtual EntityActionPointer factoryBA(EntityItemPointer ownerEntity, QByteArray data) { assert(false); return nullptr; } }; diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 3aeaf4829f..e408ce3afa 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -1489,20 +1489,22 @@ void EntityItem::clearSimulationOwnership() { } + bool EntityItem::addAction(EntitySimulation* simulation, EntityActionPointer action) { + lockForWrite(); checkWaitingToRemove(simulation); - if (!checkWaitingActionData(simulation)) { - return false; - } bool result = addActionInternal(simulation, action); if (!result) { removeAction(simulation, action->getID()); } + + unlock(); return result; } bool EntityItem::addActionInternal(EntitySimulation* simulation, EntityActionPointer action) { + assertLocked(); assert(action); assert(simulation); auto actionOwnerEntity = action->getOwnerEntity().lock(); @@ -1523,36 +1525,37 @@ bool EntityItem::addActionInternal(EntitySimulation* simulation, EntityActionPoi } bool EntityItem::updateAction(EntitySimulation* simulation, const QUuid& actionID, const QVariantMap& arguments) { + lockForWrite(); checkWaitingToRemove(simulation); - if (!checkWaitingActionData(simulation)) { - return false; - } if (!_objectActions.contains(actionID)) { + unlock(); return false; } EntityActionPointer action = _objectActions[actionID]; - bool success = action->updateArguments(arguments); + bool success = action->updateArguments(arguments); if (success) { _allActionsDataCache = serializeActions(success); } else { qDebug() << "EntityItem::updateAction failed"; } + unlock(); return success; } bool EntityItem::removeAction(EntitySimulation* simulation, const QUuid& actionID) { + lockForWrite(); checkWaitingToRemove(simulation); - if (!checkWaitingActionData(simulation)) { - return false;; - } - return removeActionInternal(actionID); + bool success = removeActionInternal(actionID); + unlock(); + return success; } bool EntityItem::removeActionInternal(const QUuid& actionID, EntitySimulation* simulation) { + assertWriteLocked(); if (_objectActions.contains(actionID)) { if (!simulation) { EntityTree* entityTree = _element ? _element->getTree() : nullptr; @@ -1575,7 +1578,7 @@ bool EntityItem::removeActionInternal(const QUuid& actionID, EntitySimulation* s } bool EntityItem::clearActions(EntitySimulation* simulation) { - _waitingActionData.clear(); + lockForWrite(); QHash::iterator i = _objectActions.begin(); while (i != _objectActions.end()) { const QUuid id = i.key(); @@ -1584,85 +1587,84 @@ bool EntityItem::clearActions(EntitySimulation* simulation) { action->setOwnerEntity(nullptr); action->removeFromSimulation(simulation); } + // empty _serializedActions means no actions for the EntityItem _actionsToRemove.clear(); _allActionsDataCache.clear(); + unlock(); return true; } -bool EntityItem::deserializeActions(QByteArray allActionsData, EntitySimulation* simulation) const { - bool success = true; - QVector serializedActions; - if (allActionsData.size() > 0) { - QDataStream serializedActionsStream(allActionsData); - serializedActionsStream >> serializedActions; + +void EntityItem::deserializeActions() { + assertUnlocked(); + lockForWrite(); + deserializeActionsInternal(); + unlock(); +} + + +void EntityItem::deserializeActionsInternal() { + assertWriteLocked(); + + if (!_element) { + return; } // Keep track of which actions got added or updated by the new actionData - QSet updated; EntityTree* entityTree = _element ? _element->getTree() : nullptr; - if (!simulation) { - simulation = entityTree ? entityTree->getSimulation() : nullptr; + assert(entityTree); + EntitySimulation* simulation = entityTree ? entityTree->getSimulation() : nullptr; + assert(simulation); + + QVector serializedActions; + if (_allActionsDataCache.size() > 0) { + QDataStream serializedActionsStream(_allActionsDataCache); + serializedActionsStream >> serializedActions; } - if (simulation && entityTree) { - foreach(QByteArray serializedAction, serializedActions) { - QDataStream serializedActionStream(serializedAction); - EntityActionType actionType; - QUuid actionID; - serializedActionStream >> actionType; - serializedActionStream >> actionID; - updated << actionID; + QSet updated; - if (_objectActions.contains(actionID)) { - EntityActionPointer action = _objectActions[actionID]; - // TODO: make sure types match? there isn't currently a way to - // change the type of an existing action. - action->deserialize(serializedAction); - } else { - auto actionFactory = DependencyManager::get(); - if (simulation) { - EntityItemPointer entity = entityTree->findEntityByEntityItemID(_id); - EntityActionPointer action = actionFactory->factoryBA(simulation, entity, serializedAction); - if (action) { - entity->addActionInternal(simulation, action); - } - } else { - // we can't yet add the action. This method will be called later. - success = false; - } + foreach(QByteArray serializedAction, serializedActions) { + QDataStream serializedActionStream(serializedAction); + EntityActionType actionType; + QUuid actionID; + serializedActionStream >> actionType; + serializedActionStream >> actionID; + updated << actionID; + + if (_objectActions.contains(actionID)) { + EntityActionPointer action = _objectActions[actionID]; + // TODO: make sure types match? there isn't currently a way to + // change the type of an existing action. + action->deserialize(serializedAction); + } else { + auto actionFactory = DependencyManager::get(); + + // EntityItemPointer entity = entityTree->findEntityByEntityItemID(_id, false); + EntityItemPointer entity = shared_from_this(); + EntityActionPointer action = actionFactory->factoryBA(entity, serializedAction); + if (action) { + entity->addActionInternal(simulation, action); } } + } - // remove any actions that weren't included in the new data. - QHash::const_iterator i = _objectActions.begin(); - while (i != _objectActions.end()) { - const QUuid id = i.key(); - if (!updated.contains(id)) { - _actionsToRemove << id; - } - i++; + // remove any actions that weren't included in the new data. + QHash::const_iterator i = _objectActions.begin(); + while (i != _objectActions.end()) { + const QUuid id = i.key(); + if (!updated.contains(id)) { + _actionsToRemove << id; } - } else { - // no simulation - success = false; + i++; } - return success; -} - -bool EntityItem::checkWaitingActionData(EntitySimulation* simulation) const { - if (_waitingActionData.size() == 0) { - return true; - } - bool success = deserializeActions(_waitingActionData, simulation); - if (success) { - _waitingActionData.clear(); - } - return success; + return; } void EntityItem::checkWaitingToRemove(EntitySimulation* simulation) { + assertLocked(); foreach(QUuid actionID, _actionsToRemove) { removeActionInternal(actionID, simulation); } @@ -1670,21 +1672,22 @@ void EntityItem::checkWaitingToRemove(EntitySimulation* simulation) { } void EntityItem::setActionData(QByteArray actionData) { + assertUnlocked(); + lockForWrite(); + setActionDataInternal(actionData); + unlock(); +} + +void EntityItem::setActionDataInternal(QByteArray actionData) { + assertWriteLocked(); checkWaitingToRemove(); - bool success = deserializeActions(actionData); _allActionsDataCache = actionData; - if (success) { - _waitingActionData.clear(); - } else { - _waitingActionData = actionData; - } + deserializeActionsInternal(); } QByteArray EntityItem::serializeActions(bool& success) const { + assertLocked(); QByteArray result; - if (!checkWaitingActionData()) { - return _waitingActionData; - } if (_objectActions.size() == 0) { success = true; @@ -1713,7 +1716,7 @@ QByteArray EntityItem::serializeActions(bool& success) const { return result; } -const QByteArray EntityItem::getActionData() const { +const QByteArray EntityItem::getActionDataInternal() const { if (_actionDataDirty) { bool success; QByteArray newDataCache = serializeActions(success); @@ -1725,17 +1728,120 @@ const QByteArray EntityItem::getActionData() const { return _allActionsDataCache; } +const QByteArray EntityItem::getActionData() const { + assertUnlocked(); + lockForRead(); + auto result = getActionDataInternal(); + unlock(); + return result; +} + QVariantMap EntityItem::getActionArguments(const QUuid& actionID) const { QVariantMap result; - - if (!checkWaitingActionData()) { - return result; - } + lockForRead(); if (_objectActions.contains(actionID)) { EntityActionPointer action = _objectActions[actionID]; result = action->getArguments(); result["type"] = EntityActionInterface::actionTypeToString(action->getType()); } + unlock(); return result; } + + + +#define ENABLE_LOCKING 1 + +#ifdef ENABLE_LOCKING +void EntityItem::lockForRead() const { + _lock.lockForRead(); +} + +bool EntityItem::tryLockForRead() const { + return _lock.tryLockForRead(); +} + +void EntityItem::lockForWrite() const { + _lock.lockForWrite(); +} + +bool EntityItem::tryLockForWrite() const { + return _lock.tryLockForWrite(); +} + +void EntityItem::unlock() const { + _lock.unlock(); +} + +bool EntityItem::isLocked() const { + bool readSuccess = tryLockForRead(); + if (readSuccess) { + unlock(); + } + bool writeSuccess = tryLockForWrite(); + if (writeSuccess) { + unlock(); + } + if (readSuccess && writeSuccess) { + return false; // if we can take both kinds of lock, there was no previous lock + } + return true; // either read or write failed, so there is some lock in place. +} + + +bool EntityItem::isWriteLocked() const { + bool readSuccess = tryLockForRead(); + if (readSuccess) { + unlock(); + return false; + } + bool writeSuccess = tryLockForWrite(); + if (writeSuccess) { + unlock(); + return false; + } + return true; // either read or write failed, so there is some lock in place. +} + + +bool EntityItem::isUnlocked() const { + // this can't be sure -- this may get unlucky and hit locks from other threads. what we're actually trying + // to discover is if *this* thread hasn't locked the EntityItem. Try repeatedly to take both kinds of lock. + bool readSuccess = false; + for (int i=0; i<80; i++) { + readSuccess = tryLockForRead(); + if (readSuccess) { + unlock(); + break; + } + QThread::usleep(200); + } + + bool writeSuccess = false; + if (readSuccess) { + for (int i=0; i<80; i++) { + writeSuccess = tryLockForWrite(); + if (writeSuccess) { + unlock(); + break; + } + QThread::usleep(300); + } + } + + if (readSuccess && writeSuccess) { + return true; // if we can take both kinds of lock, there was no previous lock + } + return false; +} +#else +void EntityItem::lockForRead() const { } +bool EntityItem::tryLockForRead() const { return true; } +void EntityItem::lockForWrite() const { } +bool EntityItem::tryLockForWrite() const { return true; } +void EntityItem::unlock() const { } +bool EntityItem::isLocked() const { return true; } +bool EntityItem::isWriteLocked() const { return true; } +bool EntityItem::isUnlocked() const { return true; } +#endif diff --git a/libraries/entities/src/EntityItem.h b/libraries/entities/src/EntityItem.h index 7893818267..de431446e8 100644 --- a/libraries/entities/src/EntityItem.h +++ b/libraries/entities/src/EntityItem.h @@ -68,10 +68,28 @@ const float ACTIVATION_ANGULAR_VELOCITY_DELTA = 0.03f; #define debugTimeOnly(T) qPrintable(QString("%1").arg(T, 16, 10)) #define debugTreeVector(V) V << "[" << V << " in meters ]" +#if DEBUG + #define assertLocked() assert(isLocked()) +#else + #define assertLocked() +#endif + +#if DEBUG + #define assertWriteLocked() assert(isWriteLocked()) +#else + #define assertWriteLocked() +#endif + +#if DEBUG + #define assertUnlocked() assert(isUnlocked()) +#else + #define assertUnlocked() +#endif + /// EntityItem class this is the base class for all entity types. It handles the basic properties and functionality available /// to all other entity types. In particular: postion, size, rotation, age, lifetime, velocity, gravity. You can not instantiate /// one directly, instead you must only construct one of it's derived classes with additional features. -class EntityItem { +class EntityItem : public std::enable_shared_from_this { // These two classes manage lists of EntityItem pointers and must be able to cleanup pointers when an EntityItem is deleted. // To make the cleanup robust each EntityItem has backpointers to its manager classes (which are only ever set/cleared by // the managers themselves, hence they are fiends) whose NULL status can be used to determine which managers still need to @@ -395,10 +413,14 @@ public: bool hasActions() { return !_objectActions.empty(); } QList getActionIDs() { return _objectActions.keys(); } QVariantMap getActionArguments(const QUuid& actionID) const; + void deserializeActions(); void setActionDataDirty(bool value) const { _actionDataDirty = value; } protected: + const QByteArray getActionDataInternal() const; + void setActionDataInternal(QByteArray actionData); + static bool _sendPhysicsUpdates; EntityTypes::EntityType _type; QUuid _id; @@ -471,19 +493,28 @@ protected: bool addActionInternal(EntitySimulation* simulation, EntityActionPointer action); bool removeActionInternal(const QUuid& actionID, EntitySimulation* simulation = nullptr); - bool deserializeActions(QByteArray allActionsData, EntitySimulation* simulation = nullptr) const; + void deserializeActionsInternal(); QByteArray serializeActions(bool& success) const; QHash _objectActions; + static int _maxActionsDataSize; mutable QByteArray _allActionsDataCache; // when an entity-server starts up, EntityItem::setActionData is called before the entity-tree is // ready. This means we can't find our EntityItemPointer or add the action to the simulation. These // are used to keep track of and work around this situation. - bool checkWaitingActionData(EntitySimulation* simulation = nullptr) const; void checkWaitingToRemove(EntitySimulation* simulation = nullptr); - mutable QByteArray _waitingActionData; mutable QSet _actionsToRemove; mutable bool _actionDataDirty = false; + + mutable QReadWriteLock _lock; + void lockForRead() const; + bool tryLockForRead() const; + void lockForWrite() const; + bool tryLockForWrite() const; + void unlock() const; + bool isLocked() const; + bool isWriteLocked() const; + bool isUnlocked() const; }; #endif // hifi_EntityItem_h diff --git a/libraries/entities/src/EntityScriptingInterface.cpp b/libraries/entities/src/EntityScriptingInterface.cpp index 7cc2c03dfc..f1c6157694 100644 --- a/libraries/entities/src/EntityScriptingInterface.cpp +++ b/libraries/entities/src/EntityScriptingInterface.cpp @@ -574,7 +574,7 @@ QUuid EntityScriptingInterface::addAction(const QString& actionTypeString, if (actionType == ACTION_TYPE_NONE) { return false; } - EntityActionPointer action = actionFactory->factory(simulation, actionType, actionID, entity, arguments); + EntityActionPointer action = actionFactory->factory(actionType, actionID, entity, arguments); if (action) { entity->addAction(simulation, action); auto nodeList = DependencyManager::get(); diff --git a/libraries/entities/src/EntitySimulation.cpp b/libraries/entities/src/EntitySimulation.cpp index a2d20fe5d5..f2bd1e873e 100644 --- a/libraries/entities/src/EntitySimulation.cpp +++ b/libraries/entities/src/EntitySimulation.cpp @@ -146,6 +146,7 @@ void EntitySimulation::sortEntitiesThatMoved() { void EntitySimulation::addEntity(EntityItemPointer entity) { assert(entity); + entity->deserializeActions(); if (entity->isMortal()) { _mortalEntities.insert(entity); quint64 expiry = entity->getExpiry();