From eeca8895fc7e585f497e908368b8cc1a509847d3 Mon Sep 17 00:00:00 2001 From: Andreas Hartmetz Date: Sun, 5 Nov 2017 11:16:00 +0100 Subject: Remove obsolete DBus dispatch lock In c2049f67e4cfe5f09e1b033b910cb37d043a287e, all DBus I/O was moved into a service thread. The dispatch lock used to protect DBus I/O in the threading setup before that commit. It is not needed anymore. No discernible difference in QtDBus benchmarks, roughly 500 bytes code size reduction on AMD64. The main point is to reduce confusion from unnecessary code. Change-Id: Idcbdd2b7e2b317cf6da0b5bfc5ec70afed1f1b48 Reviewed-by: Thiago Macieira --- src/dbus/qdbusconnection_p.h | 10 ++++------ src/dbus/qdbusintegrator.cpp | 21 +++------------------ src/dbus/qdbusthreaddebug_p.h | 40 ---------------------------------------- 3 files changed, 7 insertions(+), 64 deletions(-) diff --git a/src/dbus/qdbusconnection_p.h b/src/dbus/qdbusconnection_p.h index 6e58c5bd21..3043d7378c 100644 --- a/src/dbus/qdbusconnection_p.h +++ b/src/dbus/qdbusconnection_p.h @@ -59,7 +59,6 @@ #include #include -#include #include #include #include @@ -198,7 +197,6 @@ public: ~QDBusConnectionPrivate(); void createBusService(); - void setDispatchEnabled(bool enable); void setPeer(DBusConnection *connection, const QDBusErrorInternal &error); void setConnection(DBusConnection *connection, const QDBusErrorInternal &error); void setServer(QDBusServer *object, DBusServer *server, const QDBusErrorInternal &error); @@ -275,6 +273,7 @@ protected: public slots: // public slots + void setDispatchEnabled(bool enable); void doDispatch(); void socketRead(int); void socketWrite(int); @@ -312,9 +311,6 @@ public: QDBusServer *serverObject; }; - // the dispatch lock protects everything related to the DBusConnection or DBusServer - // including the timeouts and watches - QMutex dispatchLock; union { DBusConnection *connection; DBusServer *server; @@ -390,7 +386,9 @@ public: public slots: void execute() { - con->setDispatchEnabled(true); + // This call cannot race with something disabling dispatch only because dispatch is + // never re-disabled from Qt code on an in-use connection once it has been enabled. + QMetaObject::invokeMethod(con, "setDispatchEnabled", Qt::QueuedConnection, Q_ARG(bool, true)); if (!con->ref.deref()) con->deleteLater(); deleteLater(); diff --git a/src/dbus/qdbusintegrator.cpp b/src/dbus/qdbusintegrator.cpp index c74c63fdc6..03de5b0091 100644 --- a/src/dbus/qdbusintegrator.cpp +++ b/src/dbus/qdbusintegrator.cpp @@ -150,7 +150,6 @@ static dbus_bool_t qDBusAddTimeout(DBusTimeout *timeout, void *data) if (Q_UNLIKELY(!q_dbus_timeout_get_enabled(timeout))) return false; - QDBusDispatchLocker locker(AddTimeoutAction, d); Q_ASSERT(d->timeouts.key(timeout, 0) == 0); int timerId = d->startTimer(q_dbus_timeout_get_interval(timeout)); @@ -172,8 +171,6 @@ static void qDBusRemoveTimeout(DBusTimeout *timeout, void *data) QDBusConnectionPrivate *d = static_cast(data); Q_ASSERT(QThread::currentThread() == d->thread()); - QDBusDispatchLocker locker(RemoveTimeoutAction, d); - QDBusConnectionPrivate::TimeoutHash::iterator it = d->timeouts.begin(); while (it != d->timeouts.end()) { if (it.value() == timeout) { @@ -210,7 +207,6 @@ static dbus_bool_t qDBusAddWatch(DBusWatch *watch, void *data) QDBusConnectionPrivate::Watcher watcher; - QDBusDispatchLocker locker(AddWatchAction, d); if (flags & DBUS_WATCH_READABLE) { //qDebug("addReadWatch %d", fd); watcher.watch = watch; @@ -241,7 +237,6 @@ static void qDBusRemoveWatch(DBusWatch *watch, void *data) Q_ASSERT(QThread::currentThread() == d->thread()); int fd = q_dbus_watch_get_unix_fd(watch); - QDBusDispatchLocker locker(RemoveWatchAction, d); QDBusConnectionPrivate::WatcherHash::iterator i = d->watchers.find(fd); while (i != d->watchers.end() && i.key() == fd) { if (i.value().watch == watch) { @@ -263,8 +258,6 @@ static void qDBusToggleWatch(DBusWatch *watch, void *data) Q_ASSERT(QThread::currentThread() == d->thread()); int fd = q_dbus_watch_get_unix_fd(watch); - QDBusDispatchLocker locker(ToggleWatchAction, d); - QDBusConnectionPrivate::WatcherHash::iterator i = d->watchers.find(fd); while (i != d->watchers.end() && i.key() == fd) { if (i.value().watch == watch) { @@ -766,7 +759,7 @@ static int findSlot(const QMetaObject *mo, const QByteArray &name, int flags, */ void QDBusConnectionPrivate::setDispatchEnabled(bool enable) { - QDBusDispatchLocker locker(SetDispatchEnabledAction, this); + checkThread(); dispatchEnabled = enable; if (enable) emit dispatchStatusChanged(); @@ -1024,7 +1017,7 @@ extern bool qDBusInitThreads(); QDBusConnectionPrivate::QDBusConnectionPrivate(QObject *p) : QObject(p), ref(1), capabilities(0), mode(InvalidMode), busService(0), - dispatchLock(QMutex::Recursive), connection(0), + connection(0), rootNode(QString(QLatin1Char('/'))), anonymousAuthenticationAllowed(false), dispatchEnabled(true) @@ -1176,7 +1169,6 @@ bool QDBusConnectionPrivate::handleError(const QDBusErrorInternal &error) void QDBusConnectionPrivate::timerEvent(QTimerEvent *e) { { - QDBusDispatchLocker locker(TimerEventAction, this); DBusTimeout *timeout = timeouts.value(e->timerId(), 0); if (timeout) q_dbus_timeout_handle(timeout); @@ -1187,7 +1179,6 @@ void QDBusConnectionPrivate::timerEvent(QTimerEvent *e) void QDBusConnectionPrivate::doDispatch() { - QDBusDispatchLocker locker(DoDispatchAction, this); if (mode == ClientMode || mode == PeerMode) { while (q_dbus_connection_dispatch(connection) == DBUS_DISPATCH_DATA_REMAINS) ; if (dispatchEnabled && !pendingMessages.isEmpty()) { @@ -1205,7 +1196,6 @@ void QDBusConnectionPrivate::doDispatch() void QDBusConnectionPrivate::socketRead(int fd) { - QDBusDispatchLocker locker(SocketReadAction, this); WatcherHash::ConstIterator it = watchers.constFind(fd); while (it != watchers.constEnd() && it.key() == fd) { if (it->watch && it->read && it->read->isEnabled()) { @@ -1220,7 +1210,6 @@ void QDBusConnectionPrivate::socketRead(int fd) void QDBusConnectionPrivate::socketWrite(int fd) { - QDBusDispatchLocker locker(SocketWriteAction, this); WatcherHash::ConstIterator it = watchers.constFind(fd); while (it != watchers.constEnd() && it.key() == fd) { if (it->watch && it->write && it->write->isEnabled()) { @@ -1283,10 +1272,7 @@ void QDBusConnectionPrivate::relaySignal(QObject *obj, const QMetaObject *mo, in //qDBusDebug() << "Emitting signal" << message; //qDBusDebug() << "for paths:"; q_dbus_message_set_no_reply(msg, true); // the reply would not be delivered to anything - { - QDBusDispatchLocker locker(HuntAndEmitAction, this); - huntAndEmit(connection, msg, obj, rootNode, isScriptable, isAdaptor); - } + huntAndEmit(connection, msg, obj, rootNode, isScriptable, isAdaptor); q_dbus_message_unref(msg); } @@ -2164,7 +2150,6 @@ void QDBusConnectionPrivate::sendInternal(QDBusPendingCallPrivate *pcall, void * Q_ASSERT(isNoReply == !!q_dbus_message_get_no_reply(msg)); checkThread(); - QDBusDispatchLocker locker(SendMessageAction, this); if (isNoReply && q_dbus_connection_send(connection, msg, nullptr)) { // success diff --git a/src/dbus/qdbusthreaddebug_p.h b/src/dbus/qdbusthreaddebug_p.h index 96f389fa49..ad0984e26c 100644 --- a/src/dbus/qdbusthreaddebug_p.h +++ b/src/dbus/qdbusthreaddebug_p.h @@ -95,17 +95,6 @@ enum ThreadAction { PendingCallBlockAction = 28, SendMessageAction = 29, HuntAndEmitAction = 30, - - AddTimeoutAction = 50, - RealAddTimeoutAction = 51, - RemoveTimeoutAction = 52, - KillTimerAction = 58, - TimerEventAction = 59, - AddWatchAction = 60, - RemoveWatchAction = 61, - ToggleWatchAction = 62, - SocketReadAction = 63, - SocketWriteAction = 64 }; struct QDBusLockerBase @@ -176,35 +165,6 @@ struct QDBusWriteLocker: QDBusLockerBase } }; -struct QDBusMutexLocker: QDBusLockerBase -{ - QDBusConnectionPrivate *self; - QMutex *mutex; - ThreadAction action; - inline QDBusMutexLocker(ThreadAction a, QDBusConnectionPrivate *s, - QMutex *m) - : self(s), mutex(m), action(a) - { - reportThreadAction(action, BeforeLock, self); - mutex->lock(); - reportThreadAction(action, AfterLock, self); - } - - inline ~QDBusMutexLocker() - { - reportThreadAction(action, BeforeUnlock, self); - mutex->unlock(); - reportThreadAction(action, AfterUnlock, self); - } -}; - -struct QDBusDispatchLocker: QDBusMutexLocker -{ - inline QDBusDispatchLocker(ThreadAction a, QDBusConnectionPrivate *s) - : QDBusMutexLocker(a, s, &s->dispatchLock) - { } -}; - #if QDBUS_THREAD_DEBUG # define SEM_ACQUIRE(action, sem) \ do { \ -- cgit v1.2.3