Merge pull request #12547 from Atlante45/fix/session-uuid-lock

Protect session UUID from concurrent read/writes
This commit is contained in:
John Conklin II 2018-03-05 08:24:22 -08:00 committed by GitHub
commit 8cb7e0584f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 31 additions and 30 deletions

View file

@ -405,7 +405,7 @@ void DomainServer::restart() {
exit(DomainServer::EXIT_CODE_REBOOT); exit(DomainServer::EXIT_CODE_REBOOT);
} }
const QUuid& DomainServer::getID() { QUuid DomainServer::getID() {
return DependencyManager::get<LimitedNodeList>()->getSessionUUID(); return DependencyManager::get<LimitedNodeList>()->getSessionUUID();
} }

View file

@ -135,7 +135,7 @@ signals:
void userDisconnected(); void userDisconnected();
private: private:
const QUuid& getID(); QUuid getID();
void parseCommandLine(); void parseCommandLine();
QString getContentBackupDir(); QString getContentBackupDir();

View file

@ -49,19 +49,8 @@ const std::set<NodeType_t> SOLO_NODE_TYPES = {
}; };
LimitedNodeList::LimitedNodeList(int socketListenPort, int dtlsListenPort) : LimitedNodeList::LimitedNodeList(int socketListenPort, int dtlsListenPort) :
_sessionUUID(),
_nodeHash(),
_nodeMutex(QReadWriteLock::Recursive),
_nodeSocket(this), _nodeSocket(this),
_dtlsSocket(NULL), _packetReceiver(new PacketReceiver(this))
_localSockAddr(),
_publicSockAddr(),
_stunSockAddr(STUN_SERVER_HOSTNAME, STUN_SERVER_PORT),
_packetReceiver(new PacketReceiver(this)),
_numCollectedPackets(0),
_numCollectedBytes(0),
_packetStatTimer(),
_permissions(NodePermissions())
{ {
qRegisterMetaType<ConnectionStep>("ConnectionStep"); qRegisterMetaType<ConnectionStep>("ConnectionStep");
auto port = (socketListenPort != INVALID_PORT) ? socketListenPort : LIMITED_NODELIST_LOCAL_PORT.get(); auto port = (socketListenPort != INVALID_PORT) ? socketListenPort : LIMITED_NODELIST_LOCAL_PORT.get();
@ -122,13 +111,22 @@ LimitedNodeList::LimitedNodeList(int socketListenPort, int dtlsListenPort) :
} }
} }
QUuid LimitedNodeList::getSessionUUID() const {
QReadLocker lock { &_sessionUUIDLock };
return _sessionUUID;
}
void LimitedNodeList::setSessionUUID(const QUuid& sessionUUID) { void LimitedNodeList::setSessionUUID(const QUuid& sessionUUID) {
QUuid oldUUID = _sessionUUID; QUuid oldUUID;
_sessionUUID = sessionUUID; {
QWriteLocker lock { &_sessionUUIDLock };
oldUUID = _sessionUUID;
_sessionUUID = sessionUUID;
}
if (sessionUUID != oldUUID) { if (sessionUUID != oldUUID) {
qCDebug(networking) << "NodeList UUID changed from" << uuidStringWithoutCurlyBraces(oldUUID) qCDebug(networking) << "NodeList UUID changed from" << uuidStringWithoutCurlyBraces(oldUUID)
<< "to" << uuidStringWithoutCurlyBraces(_sessionUUID); << "to" << uuidStringWithoutCurlyBraces(sessionUUID);
emit uuidChanged(sessionUUID, oldUUID); emit uuidChanged(sessionUUID, oldUUID);
} }
} }

View file

@ -104,7 +104,7 @@ public:
}; };
Q_ENUM(ConnectionStep); Q_ENUM(ConnectionStep);
const QUuid& getSessionUUID() const { return _sessionUUID; } QUuid getSessionUUID() const;
void setSessionUUID(const QUuid& sessionUUID); void setSessionUUID(const QUuid& sessionUUID);
void setPermissions(const NodePermissions& newPermissions); void setPermissions(const NodePermissions& newPermissions);
@ -380,20 +380,19 @@ protected:
bool sockAddrBelongsToNode(const HifiSockAddr& sockAddr) { return findNodeWithAddr(sockAddr) != SharedNodePointer(); } bool sockAddrBelongsToNode(const HifiSockAddr& sockAddr) { return findNodeWithAddr(sockAddr) != SharedNodePointer(); }
QUuid _sessionUUID;
NodeHash _nodeHash; NodeHash _nodeHash;
mutable QReadWriteLock _nodeMutex; mutable QReadWriteLock _nodeMutex { QReadWriteLock::Recursive };
udt::Socket _nodeSocket; udt::Socket _nodeSocket;
QUdpSocket* _dtlsSocket; QUdpSocket* _dtlsSocket { nullptr };
HifiSockAddr _localSockAddr; HifiSockAddr _localSockAddr;
HifiSockAddr _publicSockAddr; HifiSockAddr _publicSockAddr;
HifiSockAddr _stunSockAddr; HifiSockAddr _stunSockAddr { STUN_SERVER_HOSTNAME, STUN_SERVER_PORT };
bool _hasTCPCheckedLocalSocket { false }; bool _hasTCPCheckedLocalSocket { false };
PacketReceiver* _packetReceiver; PacketReceiver* _packetReceiver;
std::atomic<int> _numCollectedPackets; std::atomic<int> _numCollectedPackets { 0 };
std::atomic<int> _numCollectedBytes; std::atomic<int> _numCollectedBytes { 0 };
QElapsedTimer _packetStatTimer; QElapsedTimer _packetStatTimer;
NodePermissions _permissions; NodePermissions _permissions;
@ -424,6 +423,10 @@ private slots:
void flagTimeForConnectionStep(ConnectionStep connectionStep, quint64 timestamp); void flagTimeForConnectionStep(ConnectionStep connectionStep, quint64 timestamp);
void possiblyTimeoutSTUNAddressLookup(); void possiblyTimeoutSTUNAddressLookup();
void addSTUNHandlerToUnfiltered(); // called once STUN socket known void addSTUNHandlerToUnfiltered(); // called once STUN socket known
private:
mutable QReadWriteLock _sessionUUIDLock;
QUuid _sessionUUID;
}; };
#endif // hifi_LimitedNodeList_h #endif // hifi_LimitedNodeList_h

View file

@ -798,7 +798,7 @@ void NodeList::sendIgnoreRadiusStateToNode(const SharedNodePointer& destinationN
void NodeList::ignoreNodeBySessionID(const QUuid& nodeID, bool ignoreEnabled) { void NodeList::ignoreNodeBySessionID(const QUuid& nodeID, bool ignoreEnabled) {
// enumerate the nodes to send a reliable ignore packet to each that can leverage it // enumerate the nodes to send a reliable ignore packet to each that can leverage it
if (!nodeID.isNull() && _sessionUUID != nodeID) { if (!nodeID.isNull() && getSessionUUID() != nodeID) {
eachMatchingNode([](const SharedNodePointer& node)->bool { eachMatchingNode([](const SharedNodePointer& node)->bool {
if (node->getType() == NodeType::AudioMixer || node->getType() == NodeType::AvatarMixer) { if (node->getType() == NodeType::AudioMixer || node->getType() == NodeType::AvatarMixer) {
return true; return true;
@ -851,7 +851,7 @@ void NodeList::ignoreNodeBySessionID(const QUuid& nodeID, bool ignoreEnabled) {
void NodeList::removeFromIgnoreMuteSets(const QUuid& nodeID) { void NodeList::removeFromIgnoreMuteSets(const QUuid& nodeID) {
// don't remove yourself, or nobody // don't remove yourself, or nobody
if (!nodeID.isNull() && _sessionUUID != nodeID) { if (!nodeID.isNull() && getSessionUUID() != nodeID) {
{ {
QWriteLocker ignoredSetLocker{ &_ignoredSetLock }; QWriteLocker ignoredSetLocker{ &_ignoredSetLock };
_ignoredNodeIDs.unsafe_erase(nodeID); _ignoredNodeIDs.unsafe_erase(nodeID);
@ -870,7 +870,7 @@ bool NodeList::isIgnoringNode(const QUuid& nodeID) const {
void NodeList::personalMuteNodeBySessionID(const QUuid& nodeID, bool muteEnabled) { void NodeList::personalMuteNodeBySessionID(const QUuid& nodeID, bool muteEnabled) {
// cannot personal mute yourself, or nobody // cannot personal mute yourself, or nobody
if (!nodeID.isNull() && _sessionUUID != nodeID) { if (!nodeID.isNull() && getSessionUUID() != nodeID) {
auto audioMixer = soloNodeOfType(NodeType::AudioMixer); auto audioMixer = soloNodeOfType(NodeType::AudioMixer);
if (audioMixer) { if (audioMixer) {
if (isIgnoringNode(nodeID)) { if (isIgnoringNode(nodeID)) {
@ -970,7 +970,7 @@ void NodeList::maybeSendIgnoreSetToNode(SharedNodePointer newNode) {
void NodeList::setAvatarGain(const QUuid& nodeID, float gain) { void NodeList::setAvatarGain(const QUuid& nodeID, float gain) {
// cannot set gain of yourself // cannot set gain of yourself
if (_sessionUUID != nodeID) { if (getSessionUUID() != nodeID) {
auto audioMixer = soloNodeOfType(NodeType::AudioMixer); auto audioMixer = soloNodeOfType(NodeType::AudioMixer);
if (audioMixer) { if (audioMixer) {
// setup the packet // setup the packet
@ -1013,7 +1013,7 @@ void NodeList::kickNodeBySessionID(const QUuid& nodeID) {
// send a request to domain-server to kick the node with the given session ID // send a request to domain-server to kick the node with the given session ID
// the domain-server will handle the persistence of the kick (via username or IP) // the domain-server will handle the persistence of the kick (via username or IP)
if (!nodeID.isNull() && _sessionUUID != nodeID ) { if (!nodeID.isNull() && getSessionUUID() != nodeID ) {
if (getThisNodeCanKick()) { if (getThisNodeCanKick()) {
// setup the packet // setup the packet
auto kickPacket = NLPacket::create(PacketType::NodeKickRequest, NUM_BYTES_RFC4122_UUID, true); auto kickPacket = NLPacket::create(PacketType::NodeKickRequest, NUM_BYTES_RFC4122_UUID, true);
@ -1036,7 +1036,7 @@ void NodeList::kickNodeBySessionID(const QUuid& nodeID) {
void NodeList::muteNodeBySessionID(const QUuid& nodeID) { void NodeList::muteNodeBySessionID(const QUuid& nodeID) {
// cannot mute yourself, or nobody // cannot mute yourself, or nobody
if (!nodeID.isNull() && _sessionUUID != nodeID ) { if (!nodeID.isNull() && getSessionUUID() != nodeID ) {
if (getThisNodeCanKick()) { if (getThisNodeCanKick()) {
auto audioMixer = soloNodeOfType(NodeType::AudioMixer); auto audioMixer = soloNodeOfType(NodeType::AudioMixer);
if (audioMixer) { if (audioMixer) {