fix segfault on disconnect (#5447)

* add new param to closeRequest

* don't emit signals in dtors

* send closeRequest

* fix build failure

* fix build failure

* see if we can get away with the overloaded triggered

* fix build failure

cockatrice/src/client/tabs/tab.h

@@ -50,7 +50,13 @@ public:
     }
     virtual QString getTabText() const = 0;
     virtual void retranslateUi() = 0;
-    virtual void closeRequest()
+    /**
+     * Sends a request to close the tab.
+     * Signals for cleanup should be emitted from this method instead of the destructor.
+     *
+     * @param forced whether this close request was initiated by the user or forced by the server.
+     */
+    virtual void closeRequest(bool /*forced*/ = false)
     {
     }
     virtual void tabActivated()

cockatrice/src/client/tabs/tab_deck_editor.cpp

@@ -358,7 +358,7 @@ void TabDeckEditor::createMenus()
     analyzeDeckMenu->addAction(aAnalyzeDeckTappedout);
 
     aClose = new QAction(QString(), this);
-    connect(aClose, SIGNAL(triggered()), this, SLOT(closeRequest()));
+    connect(aClose, &QAction::triggered, this, [this] { closeRequest(); });
 
     aClearFilterAll = new QAction(QString(), this);
     aClearFilterAll->setIcon(QPixmap("theme:icons/clearsearch"));
@@ -721,11 +721,6 @@ TabDeckEditor::TabDeckEditor(TabSupervisor *_tabSupervisor, QWidget *parent)
     loadLayout();
 }
 
-TabDeckEditor::~TabDeckEditor()
-{
-    emit deckEditorClosing(this);
-}
-
 void TabDeckEditor::retranslateUi()
 {
     cardInfo->retranslateUi();
@@ -980,10 +975,14 @@ bool TabDeckEditor::confirmClose()
     return true;
 }
 
-void TabDeckEditor::closeRequest()
+void TabDeckEditor::closeRequest(bool forced)
 {
-    if (confirmClose())
-        deleteLater();
+    if (!forced && !confirmClose()) {
+        return;
+    }
+
+    emit deckEditorClosing(this);
+    deleteLater();
 }
 
 void TabDeckEditor::actNewDeck()

cockatrice/src/client/tabs/tab_deck_editor.h

@@ -164,7 +164,6 @@ private:
 
 public:
     explicit TabDeckEditor(TabSupervisor *_tabSupervisor, QWidget *parent = nullptr);
-    ~TabDeckEditor() override;
     void retranslateUi() override;
     QString getTabText() const override;
     void setDeck(DeckLoader *_deckLoader);
@@ -179,7 +178,7 @@ public:
     void updateCardInfo(CardInfoPtr _card);
 
 public slots:
-    void closeRequest() override;
+    void closeRequest(bool forced = false) override;
     void showPrintingSelector();
 signals:
     void openDeckEditor(const DeckLoader *deckLoader);

cockatrice/src/client/tabs/tab_game.cpp

@@ -581,16 +581,14 @@ void TabGame::emitUserEvent()
 
 TabGame::~TabGame()
 {
+    scene->clearViews();
+
     delete replay;
 
     QMapIterator<int, Player *> i(players);
     while (i.hasNext()) {
         delete i.next().value();
     }
-
-    players.clear();
-
-    emit gameClosing(this);
 }
 
 void TabGame::updatePlayerListDockTitle()
@@ -701,9 +699,15 @@ void TabGame::retranslateUi()
     scene->retranslateUi();
 }
 
-void TabGame::closeRequest()
+void TabGame::closeRequest(bool forced)
 {
-    actLeaveGame();
+    if (!forced && !leaveGame()) {
+        return;
+    }
+
+    emit gameClosing(this);
+
+    deleteLater();
 }
 
 void TabGame::replayNextEvent(Player::EventProcessingOptions options)
@@ -794,19 +798,23 @@ void TabGame::actConcede()
     }
 }
 
-void TabGame::actLeaveGame()
+/**
+ * Confirms the leave game and sends the leave game command, if applicable.
+ *
+ * @return True if the leave game is confirmed
+ */
+bool TabGame::leaveGame()
 {
     if (!gameClosed) {
         if (!spectator)
             if (QMessageBox::question(this, tr("Leave game"), tr("Are you sure you want to leave this game?"),
                                       QMessageBox::Yes | QMessageBox::No, QMessageBox::No) != QMessageBox::Yes)
-                return;
+                return false;
 
         if (!replay)
             sendGameCommand(Command_LeaveGame());
     }
-    scene->clearViews();
-    deleteLater();
+    return true;
 }
 
 void TabGame::actSay()
@@ -1598,7 +1606,7 @@ void TabGame::createMenuItems()
     aConcede = new QAction(this);
     connect(aConcede, SIGNAL(triggered()), this, SLOT(actConcede()));
     aLeaveGame = new QAction(this);
-    connect(aLeaveGame, SIGNAL(triggered()), this, SLOT(actLeaveGame()));
+    connect(aLeaveGame, &QAction::triggered, this, [this] { closeRequest(); });
     aFocusChat = new QAction(this);
     connect(aFocusChat, SIGNAL(triggered()), sayEdit, SLOT(setFocus()));
     aCloseReplay = nullptr;
@@ -1648,7 +1656,7 @@ void TabGame::createReplayMenuItems()
     aFocusChat = nullptr;
     aLeaveGame = nullptr;
     aCloseReplay = new QAction(this);
-    connect(aCloseReplay, SIGNAL(triggered()), this, SLOT(actLeaveGame()));
+    connect(aCloseReplay, &QAction::triggered, this, [this] { closeRequest(); });
 
     phasesMenu = nullptr;
     gameMenu = new QMenu(this);

cockatrice/src/client/tabs/tab_game.h

@@ -189,6 +189,7 @@ private:
     void startGame(bool resuming);
     void stopGame();
     void closeGame();
+    bool leaveGame();
 
     void eventSpectatorSay(const Event_GameSay &event, int eventPlayerId, const GameEventContext &context);
     void eventSpectatorLeave(const Event_Leave &event, int eventPlayerId, const GameEventContext &context);
@@ -242,7 +243,6 @@ private slots:
 
     void actGameInfo();
     void actConcede();
-    void actLeaveGame();
     void actRemoveLocalArrows();
     void actRotateViewCW();
     void actRotateViewCCW();
@@ -278,7 +278,7 @@ public:
     ~TabGame() override;
     void retranslateUi() override;
     void updatePlayerListDockTitle();
-    void closeRequest() override;
+    void closeRequest(bool forced = false) override;
     const QMap<int, Player *> &getPlayers() const
     {
         return players;

cockatrice/src/client/tabs/tab_message.cpp

@@ -38,7 +38,7 @@ TabMessage::TabMessage(TabSupervisor *_tabSupervisor,
     vbox->addWidget(sayEdit);
 
     aLeave = new QAction(this);
-    connect(aLeave, SIGNAL(triggered()), this, SLOT(actLeave()));
+    connect(aLeave, &QAction::triggered, this, [this] { closeRequest(); });
 
     messageMenu = new QMenu(this);
     messageMenu->addAction(aLeave);
@@ -53,7 +53,6 @@ TabMessage::TabMessage(TabSupervisor *_tabSupervisor,
 
 TabMessage::~TabMessage()
 {
-    emit talkClosing(this);
     delete ownUserInfo;
     delete otherUserInfo;
 }
@@ -86,9 +85,10 @@ QString TabMessage::getTabText() const
     return tr("%1 - Private chat").arg(QString::fromStdString(otherUserInfo->name()));
 }
 
-void TabMessage::closeRequest()
+void TabMessage::closeRequest(bool /*forced*/)
 {
-    actLeave();
+    emit talkClosing(this);
+    deleteLater();
 }
 
 void TabMessage::sendMessage()
@@ -114,11 +114,6 @@ void TabMessage::messageSent(const Response &response)
             "This user is ignoring you, they cannot see your messages in main chat and you cannot join their games."));
 }
 
-void TabMessage::actLeave()
-{
-    deleteLater();
-}
-
 void TabMessage::processUserMessageEvent(const Event_UserMessage &event)
 {
     auto userInfo = event.sender_name() == otherUserInfo->name() ? otherUserInfo : ownUserInfo;

cockatrice/src/client/tabs/tab_message.h

@@ -29,7 +29,6 @@ signals:
     void maximizeClient();
 private slots:
     void sendMessage();
-    void actLeave();
     void messageSent(const Response &response);
     void addMentionTag(QString mentionTag);
     void messageClicked();
@@ -41,7 +40,7 @@ public:
                const ServerInfo_User &_otherUserInfo);
     ~TabMessage() override;
     void retranslateUi() override;
-    void closeRequest() override;
+    void closeRequest(bool forced = false) override;
     void tabActivated() override;
     QString getUserName() const;
     QString getTabText() const override;

cockatrice/src/client/tabs/tab_room.cpp

@@ -101,7 +101,7 @@ TabRoom::TabRoom(TabSupervisor *_tabSupervisor,
     hbox->addWidget(userList, 1);
 
     aLeaveRoom = new QAction(this);
-    connect(aLeaveRoom, SIGNAL(triggered()), this, SLOT(actLeaveRoom()));
+    connect(aLeaveRoom, &QAction::triggered, this, [this] { closeRequest(); });
 
     roomMenu = new QMenu(this);
     roomMenu->addAction(aLeaveRoom);
@@ -135,11 +135,6 @@ TabRoom::TabRoom(TabSupervisor *_tabSupervisor,
     setCentralWidget(mainWidget);
 }
 
-TabRoom::~TabRoom()
-{
-    emit roomClosing(this);
-}
-
 void TabRoom::retranslateUi()
 {
     gameSelector->retranslateUi();
@@ -175,9 +170,11 @@ void TabRoom::actShowPopup(const QString &message)
     }
 }
 
-void TabRoom::closeRequest()
+void TabRoom::closeRequest(bool /*forced*/)
 {
-    actLeaveRoom();
+    sendRoomCommand(prepareRoomCommand(Command_LeaveRoom()));
+    emit roomClosing(this);
+    deleteLater();
 }
 
 void TabRoom::tabActivated()
@@ -216,12 +213,6 @@ void TabRoom::sayFinished(const Response &response)
         chatView->appendMessage(tr("You are flooding the chat. Please wait a couple of seconds."));
 }
 
-void TabRoom::actLeaveRoom()
-{
-    sendRoomCommand(prepareRoomCommand(Command_LeaveRoom()));
-    deleteLater();
-}
-
 void TabRoom::actClearChat()
 {
     chatView->clearChat();

cockatrice/src/client/tabs/tab_room.h

@@ -70,7 +70,6 @@ signals:
 private slots:
     void sendMessage();
     void sayFinished(const Response &response);
-    void actLeaveRoom();
     void actClearChat();
     void actOpenChatSettings();
     void addMentionTag(QString mentionTag);
@@ -91,9 +90,8 @@ public:
             AbstractClient *_client,
             ServerInfo_User *_ownUser,
             const ServerInfo_Room &info);
-    ~TabRoom() override;
     void retranslateUi() override;
-    void closeRequest() override;
+    void closeRequest(bool forced = false) override;
     void tabActivated() override;
     void processRoomEvent(const RoomEvent &event);
     int getRoomId() const

cockatrice/src/client/tabs/tab_supervisor.cpp

@@ -304,30 +304,27 @@ void TabSupervisor::stop()
     tabAdmin = 0;
     tabLog = 0;
 
-    for (const auto tab : deckEditorTabs) {
-        disconnect(tab, nullptr, this, nullptr);
-        tab->deleteLater();
-    }
-    deckEditorTabs.clear();
+    QList<Tab *> tabsToDelete;
 
     for (auto i = roomTabs.cbegin(), end = roomTabs.cend(); i != end; ++i) {
-        disconnect(i.value(), nullptr, this, nullptr);
-        i.value()->deleteLater();
+        tabsToDelete << i.value();
     }
     roomTabs.clear();
 
     for (auto i = gameTabs.cbegin(), end = gameTabs.cend(); i != end; ++i) {
-        disconnect(i.value(), nullptr, this, nullptr);
-        i.value()->deleteLater();
+        tabsToDelete << i.value();
     }
     gameTabs.clear();
 
     for (const auto tab : replayTabs) {
-        disconnect(tab, nullptr, this, nullptr);
-        tab->deleteLater();
+        tabsToDelete << tab;
     }
     replayTabs.clear();
 
+    for (const auto tab : tabsToDelete) {
+        tab->closeRequest(true);
+    }
+
     delete userInfo;
     userInfo = 0;
 }

cockatrice/src/client/tabs/visual_deck_storage/tab_deck_storage_visual.cpp

@@ -58,7 +58,7 @@ TabDeckStorageVisual::TabDeckStorageVisual(TabSupervisor *_tabSupervisor, Abstra
     retranslateUi();
 }
 
-void TabDeckStorageVisual::closeRequest()
+void TabDeckStorageVisual::closeRequest(bool /*forced*/)
 {
     this->close();
 }

cockatrice/src/client/tabs/visual_deck_storage/tab_deck_storage_visual.h

@@ -33,7 +33,7 @@ public:
     }
 public slots:
     void cardUpdateFinished(int exitCode, QProcess::ExitStatus exitStatus);
-    void closeRequest() override;
+    void closeRequest(bool forced = false) override;
     void actOpenLocalDeck(QMouseEvent *event, DeckPreviewCardPictureWidget *instance);
     void actDeleteLocalDeck();
 signals: