Skip to content

Commit

Permalink
invert mDataMutex locking logic (lock from the main thread only!)
Browse files Browse the repository at this point in the history 
Seems to work. Please test it well so we don't introduce anything harmful!
  • Loading branch information
@antis81
antis81 committed Dec 30, 2021
1 parent f60f4eb commit a19fd41
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 18 deletions.
64 changes: 46 additions & 18 deletions daemon/core.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1059,6 +1059,7 @@ void Core::runEventLoop(Window rootWindow)
{
log(LOG_DEBUG, "Core::run -> XPeekEvent [waiting…]");
XPeekEvent(mDisplay, &event);
//QMutexLocker lock(&mDataMutex);
log(LOG_DEBUG, "Core::run -> XPeekEvent [event]");
if (!mX11EventLoopActive)
{
Expand All @@ -1072,10 +1073,9 @@ void Core::runEventLoop(Window rootWindow)
continue;
}
keyReleaseExpected = false; // Close time window for accepting meta keys.

if (((event.type == KeyPress) || (event.type == KeyRelease)) && mDataMutex.tryLock(0))
if (((event.type == KeyPress) || (event.type == KeyRelease)))
{
mDataMutex.unlock();
//mDataMutex.unlock();

// pop event from the x11 queue and process it
XNextEvent(mDisplay, &event);
Expand Down Expand Up @@ -1589,9 +1589,9 @@ void Core::runEventLoop(Window rootWindow)
mX11EventLoopActive = false;
break;
}
mDataMutex.lock();
//mDataMutex.lock();
mGrabbingShortcut = true;
mDataMutex.unlock();
//mDataMutex.unlock();
}
break;

Expand All @@ -1610,9 +1610,9 @@ void Core::runEventLoop(Window rootWindow)
break;
}

mDataMutex.lock();
//mDataMutex.lock();
mGrabbingShortcut = false;
mDataMutex.unlock();
//mDataMutex.unlock();
}
break;
} // end of switch-case
Expand Down Expand Up @@ -2036,7 +2036,6 @@ QString Core::checkShortcut(const QString &shortcut, X11Shortcut &X11shortcut)
QPair<QString, qulonglong> Core::addOrRegisterClientAction(const QString &shortcut, const QDBusObjectPath &path, const QString &description, const QString &sender)
{
X11Shortcut X11shortcut;

QString newShortcut = checkShortcut(shortcut, X11shortcut);
// if (newShortcut.isEmpty())
// {
Expand Down Expand Up @@ -2083,9 +2082,13 @@ QPair<QString, qulonglong> Core::addOrRegisterClientAction(const QString &shortc

void Core::addClientAction(QPair<QString, qulonglong> &result, const QString &shortcut, const QDBusObjectPath &path, const QString &description, const QString &sender)
{
log(LOG_INFO, "addClientAction shortcut:'%s' path:'%s' description:'%s' sender:'%s'", qPrintable(shortcut), qPrintable(path.path()), qPrintable(description), qPrintable(sender));

QMutexLocker lock(&mDataMutex);
addClientActionInternal(result, shortcut, path, description, sender);
}

void Core::addClientActionInternal(QPair<QString, qulonglong> &result, const QString &shortcut, const QDBusObjectPath &path, const QString &description, const QString &sender)
{
log(LOG_INFO, "addClientAction shortcut:'%s' path:'%s' description:'%s' sender:'%s'", qPrintable(shortcut), qPrintable(path.path()), qPrintable(description), qPrintable(sender));

SenderByClientPath::iterator senderByClientPath = mSenderByClientPath.find(path);
if (senderByClientPath != mSenderByClientPath.end())
Expand Down Expand Up @@ -2126,18 +2129,25 @@ void Core::addClientAction(QPair<QString, qulonglong> &result, const QString &sh

qulonglong Core::registerClientAction(const QString &shortcut, const QDBusObjectPath &path, const QString &description)
{
log(LOG_INFO, "registerClientAction shortcut:'%s' path:'%s' description:'%s'", qPrintable(shortcut), qPrintable(path.path()), qPrintable(description));

QMutexLocker lock(&mDataMutex);
return registerClientActionInternal(shortcut, path, description);
}

qulonglong Core::registerClientActionInternal(const QString &shortcut, const QDBusObjectPath &path, const QString &description)
{
log(LOG_INFO, "registerClientAction shortcut:'%s' path:'%s' description:'%s'", qPrintable(shortcut), qPrintable(path.path()), qPrintable(description));
return addOrRegisterClientAction(shortcut, path, description, QString()).second;
}

void Core::addMethodAction(QPair<QString, qulonglong> &result, const QString &shortcut, const QString &service, const QDBusObjectPath &path, const QString &interface, const QString &method, const QString &description)
{
log(LOG_INFO, "addMethodAction shortcut:'%s' service:'%s' path:'%s' interface:'%s' method:'%s' description:'%s'", qPrintable(shortcut), qPrintable(service), qPrintable(path.path()), qPrintable(interface), qPrintable(method), qPrintable(description));

QMutexLocker lock(&mDataMutex);
addMethodActionInternal(result, shortcut, service, path, interface, method, description);
}

void Core::addMethodActionInternal(QPair<QString, qulonglong> &result, const QString &shortcut, const QString &service, const QDBusObjectPath &path, const QString &interface, const QString &method, const QString &description)
{
log(LOG_INFO, "addMethodAction shortcut:'%s' service:'%s' path:'%s' interface:'%s' method:'%s' description:'%s'", qPrintable(shortcut), qPrintable(service), qPrintable(path.path()), qPrintable(interface), qPrintable(method), qPrintable(description));

X11Shortcut X11shortcut;
QString newShortcut = checkShortcut(shortcut, X11shortcut);
Expand Down Expand Up @@ -2169,16 +2179,27 @@ void Core::addMethodAction(QPair<QString, qulonglong> &result, const QString &sh

qulonglong Core::registerMethodAction(const QString &shortcut, const QString &service, const QDBusObjectPath &path, const QString &interface, const QString &method, const QString &description)
{
QMutexLocker lock(&mDataMutex);
return registerMethodActionInternal(shortcut, service, path, interface, method, description);
}

qulonglong Core::registerMethodActionInternal(const QString &shortcut, const QString &service, const QDBusObjectPath &path, const QString &interface, const QString &method, const QString &description)
{
log(LOG_INFO, "registerMethodAction shortcut:'%s' path:'%s' description:'%s'", qPrintable(shortcut), qPrintable(path.path()), qPrintable(description));
QPair<QString, qulonglong> result;
addMethodAction(result, shortcut, service, path, interface, method, description);
addMethodActionInternal(result, shortcut, service, path, interface, method, description);
return result.second;
}

void Core::addCommandAction(QPair<QString, qulonglong> &result, const QString &shortcut, const QString &command, const QStringList &arguments, const QString &description)
{
log(LOG_INFO, "addCommandAction shortcut:'%s' command:'%s' arguments:'%s' description:'%s'", qPrintable(shortcut), qPrintable(command), qPrintable(joinToString(arguments, QString(), QLatin1String("' '"), QString())), qPrintable(description));

QMutexLocker lock(&mDataMutex);
addCommandActionInternal(result, shortcut, command, arguments, description);
}

void Core::addCommandActionInternal(QPair<QString, qulonglong> &result, const QString &shortcut, const QString &command, const QStringList &arguments, const QString &description)
{
log(LOG_INFO, "addCommandAction shortcut:'%s' command:'%s' arguments:'%s' description:'%s'", qPrintable(shortcut), qPrintable(command), qPrintable(joinToString(arguments, QString(), QLatin1String("' '"), QString())), qPrintable(description));

X11Shortcut X11shortcut;
QString newShortcut = checkShortcut(shortcut, X11shortcut);
Expand Down Expand Up @@ -2210,8 +2231,15 @@ void Core::addCommandAction(QPair<QString, qulonglong> &result, const QString &s

qulonglong Core::registerCommandAction(const QString &shortcut, const QString &command, const QStringList &arguments, const QString &description)
{
QMutexLocker lock(&mDataMutex);
return registerCommandActionInternal(shortcut, command, arguments, description);
}

qulonglong Core::registerCommandActionInternal(const QString &shortcut, const QString &command, const QStringList &arguments, const QString &description)
{
log(LOG_INFO, "registerCommandAction shortcut:'%s' arguments:'%s' description:'%s'", qPrintable(shortcut), qPrintable(arguments.join(QLatin1Char(' '))), qPrintable(description));
QPair<QString, qulonglong> result;
addCommandAction(result, shortcut, command, arguments, description);
addCommandActionInternal(result, shortcut, command, arguments, description);
return result.second;
}

Expand Down
6 changes: 6 additions & 0 deletions daemon/core.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,11 @@ class Core : public QThread, public LogTarget
void serviceDisappeared(const QString &sender);

void addClientAction(QPair<QString, qulonglong> &result, const QString &shortcut, const QDBusObjectPath &path, const QString &description, const QString &sender);
void addClientActionInternal(QPair<QString, qulonglong> &result, const QString &shortcut, const QDBusObjectPath &path, const QString &description, const QString &sender);
void addMethodAction(QPair<QString, qulonglong> &result, const QString &shortcut, const QString &service, const QDBusObjectPath &path, const QString &interface, const QString &method, const QString &description);
void addMethodActionInternal(QPair<QString, qulonglong> &result, const QString &shortcut, const QString &service, const QDBusObjectPath &path, const QString &interface, const QString &method, const QString &description);
void addCommandAction(QPair<QString, qulonglong> &result, const QString &shortcut, const QString &command, const QStringList &arguments, const QString &description);
void addCommandActionInternal(QPair<QString, qulonglong> &result, const QString &shortcut, const QString &command, const QStringList &arguments, const QString &description);

void modifyClientAction(qulonglong &result, const QDBusObjectPath &path, const QString &description, const QString &sender);
void modifyActionDescription(bool &result, const qulonglong &id, const QString &description);
Expand Down Expand Up @@ -162,8 +165,11 @@ class Core : public QThread, public LogTarget
bool enableActionNonGuarded(qulonglong id, bool enabled);
QPair<QString, qulonglong> addOrRegisterClientAction(const QString &shortcut, const QDBusObjectPath &path, const QString &description, const QString &sender);
qulonglong registerClientAction(const QString &shortcut, const QDBusObjectPath &path, const QString &description);
qulonglong registerClientActionInternal(const QString &shortcut, const QDBusObjectPath &path, const QString &description);
qulonglong registerMethodAction(const QString &shortcut, const QString &service, const QDBusObjectPath &path, const QString &interface, const QString &method, const QString &description);
qulonglong registerMethodActionInternal(const QString &shortcut, const QString &service, const QDBusObjectPath &path, const QString &interface, const QString &method, const QString &description);
qulonglong registerCommandAction(const QString &shortcut, const QString &command, const QStringList &arguments, const QString &description);
qulonglong registerCommandActionInternal(const QString &shortcut, const QString &command, const QStringList &arguments, const QString &description);

GeneralActionInfo actionInfo(const ShortcutAndAction &shortcutAndAction) const;

Expand Down

0 comments on commit a19fd41

Please sign in to comment.