Ticket #46496: QDBusConnection-Merge-the-dispatch-and-the-watch-and.patch

File QDBusConnection-Merge-the-dispatch-and-the-watch-and.patch, 7.2 KB (added by RJVB (René Bertin), 10 years ago)

from Ubuntu

  • qtbase/src/dbus/qdbusconnection_p.h 

    From eb99c28861f5e841f306cfe8689627fe0e9bf2e8 Mon Sep 17 00:00:00 2001
From: Thiago Macieira <thiago.macieira@intel.com>
Date: Tue, 28 Oct 2014 19:26:17 -0700
Subject: [PATCH 2/3] QDBusConnection: Merge the dispatch and the
 watch-and-timeout locks
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We don't need two anymore because they now protect the same thing: the
state of the DBusConnection. The difference existed when it was possible
for two threads to access the DBusConnection at the same time: one doing
dispatching and one doing something else. Unfortunately, even though
DBusConnection supports this, QtDBus doesn't.

From d47c05b1889bb4f06203bbc65f4660b8d0128954 (2008-10-08):
   Details:  if we're removing a timer or a watcher from our list,
   there's a race condition: one thread (not the QDBusConnection thread)
   could be asking for the removal (which causes an event to be sent),
   then deletes the pointer. In the meantime, QDBusConnection will
   process the timers and socket notifiers and could end up calling
   lidbus-1 with deleted pointers.

That commit fixed the race condition but introduced a deadlock.

Task-number: QTBUG-42189
Change-Id: I034038f763cbad3a67398909defd31a23c27c965
Reviewed-by: Jędrzej Nowacki <jedrzej.nowacki@digia.com>
Reviewed-by: Albert Astals Cid <albert.astals@canonical.com>
Reviewed-by: Frederik Gladhorn <frederik.gladhorn@theqtcompany.com>
---
 src/dbus/qdbusconnection_p.h  | 16 +++++-----------
 src/dbus/qdbusintegrator.cpp  | 22 +++++++++++-----------
 src/dbus/qdbusthreaddebug_p.h |  7 -------
 3 files changed, 16 insertions(+), 29 deletions(-)
     
    290290    QStringList serverConnectionNames;
    291291
    292292    ConnectionMode mode;
     293    QDBusConnectionInterface *busService;
    293294
    294     // members accessed in unlocked mode (except for deletion)
    295     // connection and server provide their own locking mechanisms
    296     // busService doesn't have state to be changed
     295    // the dispatch lock protects everything related to the DBusConnection or DBusServer
     296    // including the timeouts and watches
     297    QMutex dispatchLock;
    297298    DBusConnection *connection;
    298299    DBusServer *server;
    299     QDBusConnectionInterface *busService;
    300 
    301     // watchers and timeouts are accessed from any thread
    302     // but the corresponding timer and QSocketNotifier must be handled
    303     // only in the object's thread
    304     QMutex watchAndTimeoutLock;
    305300    WatcherHash watchers;
    306301    TimeoutHash timeouts;
    307302    PendingTimeoutList timeoutsPendingAdd;
    308303
    309     // members accessed through a lock
    310     QMutex dispatchLock;
     304    // the master lock protects our own internal state
    311305    QReadWriteLock lock;
    312306    QDBusError lastError;
    313307

  • qtbase/src/dbus/qdbusintegrator.cpp

     
    155155    if (!q_dbus_timeout_get_enabled(timeout))
    156156        return true;
    157157
    158     QDBusWatchAndTimeoutLocker locker(AddTimeoutAction, d);
     158    QDBusDispatchLocker locker(AddTimeoutAction, d);
    159159    if (QCoreApplication::instance() && QThread::currentThread() == d->thread()) {
    160160        // correct thread
    161161        return qDBusRealAddTimeout(d, timeout, q_dbus_timeout_get_interval(timeout));
    … …  
    190190
    191191    QDBusConnectionPrivate *d = static_cast<QDBusConnectionPrivate *>(data);
    192192
    193     QDBusWatchAndTimeoutLocker locker(RemoveTimeoutAction, d);
     193    QDBusDispatchLocker locker(RemoveTimeoutAction, d);
    194194
    195195    // is it pending addition?
    196196    QDBusConnectionPrivate::PendingTimeoutList::iterator pit = d->timeoutsPendingAdd.begin();
    … …  
    263263{
    264264    QDBusConnectionPrivate::Watcher watcher;
    265265
    266     QDBusWatchAndTimeoutLocker locker(AddWatchAction, d);
     266    QDBusDispatchLocker locker(AddWatchAction, d);
    267267    if (flags & DBUS_WATCH_READABLE) {
    268268        //qDebug("addReadWatch %d", fd);
    269269        watcher.watch = watch;
    … …  
    297297    QDBusConnectionPrivate *d = static_cast<QDBusConnectionPrivate *>(data);
    298298    int fd = q_dbus_watch_get_unix_fd(watch);
    299299
    300     QDBusWatchAndTimeoutLocker locker(RemoveWatchAction, d);
     300    QDBusDispatchLocker locker(RemoveWatchAction, d);
    301301    QDBusConnectionPrivate::WatcherHash::iterator i = d->watchers.find(fd);
    302302    while (i != d->watchers.end() && i.key() == fd) {
    303303        if (i.value().watch == watch) {
    … …  
    341341
    342342static void qDBusRealToggleWatch(QDBusConnectionPrivate *d, DBusWatch *watch, int fd)
    343343{
    344     QDBusWatchAndTimeoutLocker locker(ToggleWatchAction, d);
     344    QDBusDispatchLocker locker(ToggleWatchAction, d);
    345345
    346346    QDBusConnectionPrivate::WatcherHash::iterator i = d->watchers.find(fd);
    347347    while (i != d->watchers.end() && i.key() == fd) {
    … …  
    10161016extern bool qDBusInitThreads();
    10171017
    10181018QDBusConnectionPrivate::QDBusConnectionPrivate(QObject *p)
    1019     : QObject(p), ref(1), capabilities(0), mode(InvalidMode), connection(0), server(0), busService(0),
    1020       watchAndTimeoutLock(QMutex::Recursive), dispatchLock(QMutex::Recursive),
     1019    : QObject(p), ref(1), capabilities(0), mode(InvalidMode), busService(0),
     1020      dispatchLock(QMutex::Recursive), connection(0), server(0),
    10211021      rootNode(QString(QLatin1Char('/'))),
    10221022      anonymousAuthenticationAllowed(false)
    10231023{
    … …  
    11271127void QDBusConnectionPrivate::timerEvent(QTimerEvent *e)
    11281128{
    11291129    {
    1130         QDBusWatchAndTimeoutLocker locker(TimerEventAction, this);
     1130        QDBusDispatchLocker locker(TimerEventAction, this);
    11311131        DBusTimeout *timeout = timeouts.value(e->timerId(), 0);
    11321132        if (timeout)
    11331133            q_dbus_timeout_handle(timeout);
    … …  
    11461146    switch (ev->subtype)
    11471147    {
    11481148    case QDBusConnectionCallbackEvent::AddTimeout: {
    1149         QDBusWatchAndTimeoutLocker locker(RealAddTimeoutAction, this);
     1149        QDBusDispatchLocker locker(RealAddTimeoutAction, this);
    11501150        while (!timeoutsPendingAdd.isEmpty()) {
    11511151            QPair<DBusTimeout *, int> entry = timeoutsPendingAdd.takeFirst();
    11521152            qDBusRealAddTimeout(this, entry.first, entry.second);
    … …  
    11821182    QVarLengthArray<DBusWatch *, 2> pendingWatches;
    11831183
    11841184    {
    1185         QDBusWatchAndTimeoutLocker locker(SocketReadAction, this);
     1185        QDBusDispatchLocker locker(SocketReadAction, this);
    11861186        WatcherHash::ConstIterator it = watchers.constFind(fd);
    11871187        while (it != watchers.constEnd() && it.key() == fd) {
    11881188            if (it->watch && it->read && it->read->isEnabled())
    … …  
    12021202    QVarLengthArray<DBusWatch *, 2> pendingWatches;
    12031203
    12041204    {
    1205         QDBusWatchAndTimeoutLocker locker(SocketWriteAction, this);
     1205        QDBusDispatchLocker locker(SocketWriteAction, this);
    12061206        WatcherHash::ConstIterator it = watchers.constFind(fd);
    12071207        while (it != watchers.constEnd() && it.key() == fd) {
    12081208            if (it->watch && it->write && it->write->isEnabled())

  • qtbase/src/dbus/qdbusthreaddebug_p.h

     
    207207    { }
    208208};
    209209
    210 struct QDBusWatchAndTimeoutLocker: QDBusMutexLocker
    211 {
    212     inline QDBusWatchAndTimeoutLocker(ThreadAction a, QDBusConnectionPrivate *s)
    213         : QDBusMutexLocker(a, s, &s->watchAndTimeoutLock)
    214     { }
    215 };
    216 
    217210#if QDBUS_THREAD_DEBUG
    218211# define SEM_ACQUIRE(action, sem)                                       \
    219212    do {                                                                \