Skip to content

Commit

Permalink
Drop all locks.
Browse files Browse the repository at this point in the history 
Exception: The map import lock is actually necessary, but we're not
doing any multithreading *or* scoped unlocking here. Thus a simple
boolean flag works just as well.
  • Loading branch information
@smurfix
smurfix committed Oct 20, 2020
1 parent 45caa89 commit 93e8ade
Show file tree
Hide file tree
Showing 23 changed files with 77 additions and 221 deletions.
4 changes: 0 additions & 4 deletions src/ActionUnit.h
View file
Expand Up @@ -25,7 +25,6 @@

#include "pre_guard.h"
#include <QMap>
#include <QMutex>
#include <QPointer>
#include <QString>
#include "post_guard.h"
Expand All @@ -49,13 +48,11 @@ class ActionUnit

std::list<TAction*> getActionRootNodeList()
{
QMutexLocker locker(&mActionUnitLock);
return mActionRootNodeList;
}

QMap<int, TAction*> getActionList()
{
QMutexLocker locker(&mActionUnitLock);
return mActionMap;
}

Expand All @@ -79,7 +76,6 @@ class ActionUnit
void showToolBar(const QString&);
void hideToolBar(const QString&);

QMutex mActionUnitLock;
QList<TAction*> uninstallList;

private:
Expand Down
1 change: 0 additions & 1 deletion src/AliasUnit.cpp
View file
Expand Up @@ -150,7 +150,6 @@ void AliasUnit::removeAllTempAliases()

TAlias* AliasUnit::getAlias(int id)
{
QMutexLocker locker(&mAliasUnitLock);
if (mAliasMap.find(id) != mAliasMap.end()) {
return mAliasMap.value(id);
} else {
Expand Down
2 changes: 0 additions & 2 deletions src/AliasUnit.h
View file
Expand Up @@ -24,7 +24,6 @@

#include "pre_guard.h"
#include <QMultiMap>
#include <QMutex>
#include <QPointer>
#include <QString>
#include "post_guard.h"
Expand Down Expand Up @@ -65,7 +64,6 @@ class AliasUnit

QMultiMap<QString, TAlias*> mLookupTable;
std::list<TAlias*> mCleanupList;
QMutex mAliasUnitLock;
int statsAliasTotal;
int statsTempAliases;
int statsActiveAliases;
Expand Down
25 changes: 0 additions & 25 deletions src/Host.cpp
View file
Expand Up @@ -1503,13 +1503,11 @@ void Host::connectToServer()

void Host::closingDown()
{
QMutexLocker locker(&mLock);
mIsClosingDown = true;
}

bool Host::isClosingDown()
{
QMutexLocker locker(&mLock);
return mIsClosingDown;
}

Expand Down Expand Up @@ -2001,7 +1999,6 @@ void Host::setWideAmbiguousEAsianGlyphs(const Qt::CheckState state)
bool needToEmit = false;
const QByteArray encoding(mTelnet.getEncoding());

QMutexLocker locker(& mLock);
if (state == Qt::PartiallyChecked) {
// Set things automatically
mAutoAmbigousWidthGlyphsSetting = true;
Expand Down Expand Up @@ -2043,9 +2040,6 @@ void Host::setWideAmbiguousEAsianGlyphs(const Qt::CheckState state)

}

locker.unlock();
// We do not need to keep the mutex any longer as we have a local copy to
// work with whilst the connected methods react to the signal:
if (needToEmit) {
emit signal_changeIsAmbigousWidthGlyphsToBeWide(localState);
}
Expand Down Expand Up @@ -2344,16 +2338,12 @@ void Host::processDiscordMSDP(const QString& variable, QString value)

void Host::setDiscordApplicationID(const QString& s)
{
QMutexLocker locker(& mLock);
mDiscordApplicationID = s;
locker.unlock();

writeProfileData(QStringLiteral("discordApplicationId"), s);
}

const QString& Host::getDiscordApplicationID()
{
QMutexLocker locker(&mLock);
return mDiscordApplicationID;
}

Expand All @@ -2374,13 +2364,11 @@ bool Host::discordUserIdMatch(const QString& userName, const QString& userDiscri

void Host::setSpellDic(const QString& newDict)
{
QMutexLocker locker(& mLock);
bool isChanged = false;
if (!newDict.isEmpty() && mSpellDic != newDict) {
mSpellDic = newDict;
isChanged = true;
}
locker.unlock();
if (isChanged && mpConsole) {
mpConsole->setSystemSpellDictionary(newDict);
}
Expand All @@ -2393,7 +2381,6 @@ void Host::setUserDictionaryOptions(const bool _useDictionary, const bool useSha
{
Q_UNUSED(_useDictionary);
bool useDictionary = true;
QMutexLocker locker(&mLock);
bool dictionaryChanged {};
// Copy the value while we have the lock:
bool isSpellCheckingEnabled = mEnableSpellCheck;
Expand All @@ -2406,7 +2393,6 @@ void Host::setUserDictionaryOptions(const bool _useDictionary, const bool useSha
mUseSharedDictionary = useShared;
dictionaryChanged = true;
}
locker.unlock();

if (!mpConsole) {
return;
Expand Down Expand Up @@ -2448,11 +2434,8 @@ void Host::setName(const QString& newName)
currentPlayerRoom = mpMap->mRoomIdHash.take(mHostName);
}

QMutexLocker locker(& mLock);
// Now we have the exclusive lock on this class's protected members
mHostName = newName;
// We have made the change to the protected aspects of this class so can unlock the mutex locker and proceed:
locker.unlock();

mTelnet.mProfileName = newName;
if (mpMap) {
Expand Down Expand Up @@ -2559,30 +2542,22 @@ void Host::updateAnsi16ColorsInTable()

void Host::setPlayerRoomStyleDetails(const quint8 styleCode, const quint8 outerDiameter, const quint8 innerDiameter, const QColor& outerColor, const QColor& innerColor)
{
QMutexLocker locker(& mLock);
// Now we have the exclusive lock on this class's protected members

mPlayerRoomStyle = styleCode;
mPlayerRoomOuterDiameterPercentage = outerDiameter;
mPlayerRoomInnerDiameterPercentage = innerDiameter;
mPlayerRoomOuterColor = outerColor;
mPlayerRoomInnerColor = innerColor;
// We have made the change to the protected aspects of this class so can unlock the mutex locker and proceed:
locker.unlock();
}

void Host::getPlayerRoomStyleDetails(quint8& styleCode, quint8& outerDiameter, quint8& innerDiameter, QColor& primaryColor, QColor& secondaryColor)
{
QMutexLocker locker(& mLock);
// Now we have the exclusive lock on this class's protected members

styleCode = mPlayerRoomStyle;
outerDiameter = mPlayerRoomOuterDiameterPercentage;
innerDiameter = mPlayerRoomInnerDiameterPercentage;
primaryColor = mPlayerRoomOuterColor;
secondaryColor = mPlayerRoomInnerColor;
// We have accessed the protected aspects of this class so can unlock the mutex locker and proceed:
locker.unlock();
}

// Used to set the searchOptions here and the one in the editor if present, for
Expand Down
83 changes: 41 additions & 42 deletions src/Host.h
View file
Expand Up @@ -158,67 +158,67 @@ class Host : public QObject
Q_DECLARE_FLAGS(DiscordOptionFlags, DiscordOptionFlag)


QString getName() { QMutexLocker locker(& mLock); return mHostName; }
QString getCommandSeparator() { QMutexLocker locker(& mLock); return mCommandSeparator; }
void setName(const QString& s);
QString getUrl() { QMutexLocker locker(& mLock); return mUrl; }
void setUrl(const QString& s) { QMutexLocker locker(& mLock); mUrl = s; }
QString getUserDefinedName() { QMutexLocker locker(& mLock); return mUserDefinedName; }
void setUserDefinedName(const QString& s) { QMutexLocker locker(& mLock); mUserDefinedName = s; }
int getPort() { QMutexLocker locker(& mLock); return mPort; }
void setPort(const int p) { QMutexLocker locker(& mLock); mPort = p; }
void setAutoReconnect(const bool b) { mTelnet.setAutoReconnect(b); }
QString & getLogin() { QMutexLocker locker(& mLock); return mLogin; }
void setLogin(const QString& s) { QMutexLocker locker(& mLock); mLogin = s; }
QString & getPass() { QMutexLocker locker(& mLock); return mPass; }
void setPass(const QString& s) { QMutexLocker locker(& mLock); mPass = s; }
int getRetries() { QMutexLocker locker(& mLock); return mRetries;}
void setRetries(const int c) { QMutexLocker locker(& mLock); mRetries = c; }
int getTimeout() { QMutexLocker locker(& mLock); return mTimeout; }
void setTimeout(const int seconds) { QMutexLocker locker(& mLock); mTimeout = seconds; }
bool wideAmbiguousEAsianGlyphs() { QMutexLocker locker(& mLock); return mWideAmbigousWidthGlyphs; }
QString getName() { return mHostName; }
QString getCommandSeparator() { return mCommandSeparator; }
void setName(const QString& s);
QString getUrl() { return mUrl; }
void setUrl(const QString& s) { mUrl = s; }
QString getUserDefinedName() { return mUserDefinedName; }
void setUserDefinedName(const QString& s) { mUserDefinedName = s; }
int getPort() { return mPort; }
void setPort(const int p) { mPort = p; }
void setAutoReconnect(const bool b) { mTelnet.setAutoReconnect(b); }
QString & getLogin() { return mLogin; }
void setLogin(const QString& s) { mLogin = s; }
QString & getPass() { return mPass; }
void setPass(const QString& s) { mPass = s; }
int getRetries() { return mRetries;}
void setRetries(const int c) { mRetries = c; }
int getTimeout() { return mTimeout; }
void setTimeout(const int seconds) { mTimeout = seconds; }
bool wideAmbiguousEAsianGlyphs() { return mWideAmbigousWidthGlyphs; }
// Uses PartiallyChecked to set the automatic mode, otherwise Checked/Unchecked means use wide/narrow ambiguous glyphs
void setWideAmbiguousEAsianGlyphs(Qt::CheckState state);
void setWideAmbiguousEAsianGlyphs(Qt::CheckState state);
// Is used to set preference dialog control directly:
Qt::CheckState getWideAmbiguousEAsianGlyphsControlState() { QMutexLocker locker(& mLock);
return mAutoAmbigousWidthGlyphsSetting
? Qt::PartiallyChecked
: (mWideAmbigousWidthGlyphs ? Qt::Checked : Qt::Unchecked); }
void setHaveColorSpaceId(const bool state) { QMutexLocker locker(& mLock); mSGRCodeHasColSpaceId = state; }
bool getHaveColorSpaceId() { QMutexLocker locker(& mLock); return mSGRCodeHasColSpaceId; }
void setMayRedefineColors(const bool state) { QMutexLocker locker(& mLock); mServerMayRedefineColors = state; }
bool getMayRedefineColors() { QMutexLocker locker(& mLock); return mServerMayRedefineColors; }
void setDiscordApplicationID(const QString& s);
const QString& getDiscordApplicationID();
void setSpellDic(const QString&);
const QString& getSpellDic() { QMutexLocker locker(& mLock); return mSpellDic; }
void setUserDictionaryOptions(const bool useDictionary, const bool useShared);
void getUserDictionaryOptions(bool& useDictionary, bool& useShared) { QMutexLocker locker(& mLock); useDictionary = mEnableUserDictionary; useShared = mUseSharedDictionary; }
Qt::CheckState getWideAmbiguousEAsianGlyphsControlState() {
return mAutoAmbigousWidthGlyphsSetting
? Qt::PartiallyChecked
: (mWideAmbigousWidthGlyphs ? Qt::Checked : Qt::Unchecked); }
void setHaveColorSpaceId(const bool state) { mSGRCodeHasColSpaceId = state; }
bool getHaveColorSpaceId() { return mSGRCodeHasColSpaceId; }
void setMayRedefineColors(const bool state) { mServerMayRedefineColors = state; }
bool getMayRedefineColors() { return mServerMayRedefineColors; }
void setDiscordApplicationID(const QString& s);
const QString& getDiscordApplicationID();
void setSpellDic(const QString&);
const QString& getSpellDic() { return mSpellDic; }
void setUserDictionaryOptions(const bool useDictionary, const bool useShared);
void getUserDictionaryOptions(bool& useDictionary, bool& useShared) {
useDictionary = mEnableUserDictionary;
useShared = mUseSharedDictionary; }

void closingDown();
bool isClosingDown();
unsigned int assemblePath();
bool checkForMappingScript();

TriggerUnit* getTriggerUnit() { return &mTriggerUnit; }
TimerUnit* getTimerUnit() { return &mTimerUnit; }
AliasUnit* getAliasUnit() { return &mAliasUnit; }
ActionUnit* getActionUnit() { return &mActionUnit; }
KeyUnit* getKeyUnit() { return &mKeyUnit; }
ScriptUnit* getScriptUnit() { return &mScriptUnit; }
TimerUnit* getTimerUnit() { return &mTimerUnit; }
AliasUnit* getAliasUnit() { return &mAliasUnit; }
ActionUnit* getActionUnit() { return &mActionUnit; }
KeyUnit* getKeyUnit() { return &mKeyUnit; }
ScriptUnit* getScriptUnit() { return &mScriptUnit; }

void connectToServer();
void send(QString cmd, bool wantPrint = true, bool dontExpandAliases = false);

int getHostID()
{
QMutexLocker locker(&mLock);
return mHostID;
}

void setHostID(int id)
{
QMutexLocker locker(&mLock);
mHostID = id;
}

Expand Down Expand Up @@ -651,7 +651,6 @@ private slots:
bool mIsClosingDown;

QString mLine;
QMutex mLock;
QString mLogin;
QString mPass;

Expand Down
33 changes: 1 addition & 32 deletions src/HostManager.cpp
View file
Expand Up @@ -19,32 +19,18 @@
* 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. *
***************************************************************************/

/*
* Revised to allow concurrent read access to mHostPool but as some methods
* were not used they have been commented out - if they are needed again they
* will need to be rewritten to use the (QReadWriteLock) mPoolReadWriteLock
* that is now used instead of the previous (QMutex) mPoolLock which was used
* with a QMutexLocker - the latter is not suitable for a QReadWriteLock I
* think - SlySven Dec 2016
*/

#include "HostManager.h"


#include "mudlet.h"

bool HostManager::deleteHost(const QString& hostname)
{
mPoolReadWriteLock.lockForWrite(); // Will block until gets lock

// make sure this is really a new host
if (!mHostPool.contains(hostname)) {
mPoolReadWriteLock.unlock();
qDebug() << "HostManager::deleteHost(" << hostname.toUtf8().constData() << ") ERROR: it is not a member of host pool... releasing lock and aborting, returning false!";
return false;
} else {
int ret = mHostPool.remove(hostname);
mPoolReadWriteLock.unlock();
return ret;
}
}
Expand All @@ -61,10 +47,8 @@ bool HostManager::addHost(const QString& hostname, const QString& port, const QS
portnumber = port.toInt();
}

mPoolReadWriteLock.lockForWrite(); // Will block until gets lock
// make sure this is really a new host
if (mHostPool.contains(hostname)) {
mPoolReadWriteLock.unlock();
return false;
}

Expand All @@ -77,31 +61,21 @@ bool HostManager::addHost(const QString& hostname, const QString& port, const QS

if (Q_UNLIKELY(!pNewHost)) {
qDebug() << "HostManager::addHost(" << hostname.toUtf8().constData() << ") ERROR: failed to create new Host for the host pool... releasing lock and aborting, returning false!";
mPoolReadWriteLock.unlock();
return false;
}

mHostPool.insert(hostname, pNewHost);
mPoolReadWriteLock.unlock();
return true;
}

int HostManager::getHostCount()
{
mPoolReadWriteLock.lockForRead(); // Will block if a write lock is in place
// This assumes that there will not be nullptr values for destroyed Host
// instances:
const unsigned int total = mHostPool.count();
mPoolReadWriteLock.unlock();
return total;
return mHostPool.count();
}

void HostManager::postIrcMessage(const QString& a, const QString& b, const QString& c)
{
mPoolReadWriteLock.lockForRead(); // Will block if a write lock is in place

const QList<QSharedPointer<Host>> hostList = mHostPool.values();
mPoolReadWriteLock.unlock();
for (const auto& i : hostList) {
if (i) {
i->postIrcMessage(a, b, c);
Expand All @@ -121,9 +95,7 @@ void HostManager::postInterHostEvent(const Host* pHost, const TEvent& event, con
return;
}

mPoolReadWriteLock.lockForRead(); // Will block if a write lock is in place
const QList<QSharedPointer<Host>> hostList = mHostPool.values();
mPoolReadWriteLock.unlock();

int i = 0;
QList<int> beforeSendingHost;
Expand Down Expand Up @@ -157,10 +129,7 @@ void HostManager::postInterHostEvent(const Host* pHost, const TEvent& event, con

Host* HostManager::getHost(const QString& hostname)
{
mPoolReadWriteLock.lockForRead(); // Will block if a write lock is in place
Host* pHost = mHostPool.value(hostname).data();
mPoolReadWriteLock.unlock();

return pHost;
}

Expand Down

0 comments on commit 93e8ade

Please sign in to comment.