Simplify clean-up-on-network-disconnect handling
authorManuel Nickschas <sputnick@quassel-irc.org>
Sun, 2 Mar 2014 20:19:34 +0000 (21:19 +0100)
committerManuel Nickschas <sputnick@quassel-irc.org>
Sun, 2 Mar 2014 20:19:34 +0000 (21:19 +0100)
Previously, disconnecting from a network would trigger lots of unnecessary
stuff: because the NetworkModel needs to remove affected IrcChannels and
IrcUsers from its model items, we would trigger (on the core side!) an
IrcUser::quit() for every known user in that network, which would then remove
itself from all channels it's in, triggering the corresponding updates in the
related SyncableObjects, which would send lots of signals to the client which
would then perform its own cleanups per IrcUser, followed by throwing away
all IrcChannels and IrcUsers in that network anyway. By the time we reached
Network::removeChansAndUsers(), everything would actually be already all
gone triggered through syncobject updates.

Except in some rare cases when there was still something left behind, triggering
the dreaded "!_ircChannel && ircChannel" assert, that users have been reporting
for years. I still haven't figured out how that could possibly happen.

In any case, the only side effect that explicit call to IrcUser::quit() was
supposed to trigger was the removal of the relevant references in NetworkModel's
items. So now we just brutally delete all IrcUsers and IrcChannels on disconnect,
and have the NetworkModel items listen to the relevant destroyed() signals so
they can do their cleanup. This saves us from sending lots of stuff over the
network, and also should fix the assert (which we've replaced by a warning now,
just in case).

Fixes #1151 and a bunch of duplicates.

src/client/networkmodel.cpp
src/client/networkmodel.h
src/common/network.cpp

index 10e7575..dc985bd 100644 (file)
@@ -153,7 +153,7 @@ void NetworkItem::attachNetwork(Network *network)
     connect(network, SIGNAL(connectedSet(bool)),
         this, SIGNAL(networkDataChanged()));
     connect(network, SIGNAL(destroyed()),
-        this, SIGNAL(networkDataChanged()));
+        this, SLOT(onNetworkDestroyed()));
 
     emit networkDataChanged();
 }
@@ -233,6 +233,14 @@ void NetworkItem::onBeginRemoveChilds(int start, int end)
 }
 
 
+void NetworkItem::onNetworkDestroyed()
+{
+    _network = 0;
+    emit networkDataChanged();
+    removeAllChilds();
+}
+
+
 /*****************************************
 *  Fancy Buffer Items
 *****************************************/
@@ -520,6 +528,7 @@ void QueryBufferItem::setIrcUser(IrcUser *ircUser)
     }
 
     if (ircUser) {
+        connect(ircUser, SIGNAL(destroyed(QObject*)), SLOT(removeIrcUser()));
         connect(ircUser, SIGNAL(quited()), this, SLOT(removeIrcUser()));
         connect(ircUser, SIGNAL(awaySet(bool)), this, SIGNAL(dataChanged()));
         connect(ircUser, SIGNAL(encryptedSet(bool)), this, SLOT(setEncrypted(bool)));
@@ -595,10 +604,15 @@ QString ChannelBufferItem::toolTip(int column) const
 
 void ChannelBufferItem::attachIrcChannel(IrcChannel *ircChannel)
 {
-    Q_ASSERT(!_ircChannel && ircChannel);
+    if (_ircChannel) {
+        qWarning() << Q_FUNC_INFO << "IrcChannel already set; cleanup failed!?";
+        disconnect(_ircChannel, 0, this, 0);
+    }
 
     _ircChannel = ircChannel;
 
+    connect(ircChannel, SIGNAL(destroyed(QObject*)),
+        this, SLOT(ircChannelDestroyed()));
     connect(ircChannel, SIGNAL(topicSet(QString)),
         this, SLOT(setTopic(QString)));
     connect(ircChannel, SIGNAL(encryptedSet(bool)),
@@ -633,6 +647,16 @@ void ChannelBufferItem::ircChannelParted()
 }
 
 
+void ChannelBufferItem::ircChannelDestroyed()
+{
+    if (_ircChannel) {
+        _ircChannel = 0;
+        emit dataChanged();
+        removeAllChilds();
+    }
+}
+
+
 void ChannelBufferItem::join(const QList<IrcUser *> &ircUsers)
 {
     addUsersToCategory(ircUsers);
index dff7fa5..8c4597d 100644 (file)
@@ -72,6 +72,7 @@ signals:
 
 private slots:
     void onBeginRemoveChilds(int start, int end);
+    void onNetworkDestroyed();
 
 private:
     NetworkId _networkId;
@@ -211,6 +212,7 @@ public slots:
 
 private slots:
     void ircChannelParted();
+    void ircChannelDestroyed();
 
 private:
     IrcChannel *_ircChannel;
index d18f713..09c33ff 100644 (file)
@@ -288,20 +288,6 @@ void Network::removeChansAndUsers()
     QList<IrcChannel *> channels = ircChannels();
     _ircChannels.clear();
 
-    foreach(IrcChannel *channel, channels) {
-        proxy()->detachObject(channel);
-        disconnect(channel, 0, this, 0);
-    }
-    foreach(IrcUser *user, users) {
-        proxy()->detachObject(user);
-        disconnect(user, 0, this, 0);
-    }
-
-    // the second loop is needed because quit can have sideffects
-    foreach(IrcUser *user, users) {
-        user->quit();
-    }
-
     qDeleteAll(users);
     qDeleteAll(channels);
 }