Fix more segfaults on NetPlay quit

Basically everything here was race conditions in Qt callbacks, so I changed the client/server instances to std::shared_ptr and added null checks. It checks that the object exists in the callback, and the shared_ptr ensures it doesn't get destroyed until we're done with it.

MD5 check would also cause a segfault if you quit without cancelling it first, which was pretty silly.

7 files changed, 48 insertions, 29 deletions

diff --git a/Source/Core/Core/NetPlayClient.cpp b/Source/Core/Core/NetPlayClient.cpp
index d29b931ae0..00f2915951 100644
--- a/Source/Core/Core/NetPlayClient.cpp
+++ b/Source/Core/Core/NetPlayClient.cpp
@@ -70,6 +70,10 @@ NetPlayClient::~NetPlayClient()
 
   if (m_is_connected)
   {
+    m_should_compute_MD5 = false;
+    m_dialog->AbortMD5();
+    if (m_MD5_thread.joinable())
+      m_MD5_thread.join();
     m_do_loop.Clear();
     m_thread.join();
   }
@@ -1636,12 +1640,14 @@ void NetPlayClient::ComputeMD5(const std::string& file_identifier)
     return;
   }
 
+  if (m_MD5_thread.joinable())
+    m_MD5_thread.join();
   m_MD5_thread = std::thread([this, file]() {
     std::string sum = MD5::MD5Sum(file, [&](int progress) {
       sf::Packet packet;
       packet << static_cast<MessageId>(NP_MSG_MD5_PROGRESS);
       packet << progress;
-      Send(packet);
+      SendAsync(std::move(packet));
 
       return m_should_compute_MD5;
     });
@@ -1649,9 +1655,8 @@ void NetPlayClient::ComputeMD5(const std::string& file_identifier)
     sf::Packet packet;
     packet << static_cast<MessageId>(NP_MSG_MD5_RESULT);
     packet << sum;
-    Send(packet);
+    SendAsync(std::move(packet));
   });
-  m_MD5_thread.detach();
 }
 
 const PadMappingArray& NetPlayClient::GetPadMapping() const
diff --git a/Source/Core/DolphinQt/MainWindow.cpp b/Source/Core/DolphinQt/MainWindow.cpp
index 30fdaef58f..f6f7ce1f9e 100644
--- a/Source/Core/DolphinQt/MainWindow.cpp
+++ b/Source/Core/DolphinQt/MainWindow.cpp
@@ -668,7 +668,7 @@ bool MainWindow::RequestStop()
     const Core::State state = Core::GetState();
 
     // Only pause the game, if NetPlay is not running
-    bool pause = Settings::Instance().GetNetPlayClient() == nullptr;
+    bool pause = !Settings::Instance().GetNetPlayClient();
 
     if (pause)
       Core::SetState(Core::State::Paused);
@@ -1071,7 +1071,7 @@ bool MainWindow::NetPlayJoin()
 
   std::string host_ip;
   u16 host_port;
-  if (Settings::Instance().GetNetPlayServer() != nullptr)
+  if (Settings::Instance().GetNetPlayServer())
   {
     host_ip = "127.0.0.1";
     host_port = Settings::Instance().GetNetPlayServer()->GetPort();
diff --git a/Source/Core/DolphinQt/NetPlay/MD5Dialog.cpp b/Source/Core/DolphinQt/NetPlay/MD5Dialog.cpp
index b86b1e782e..ec8db64fc1 100644
--- a/Source/Core/DolphinQt/NetPlay/MD5Dialog.cpp
+++ b/Source/Core/DolphinQt/NetPlay/MD5Dialog.cpp
@@ -21,7 +21,11 @@
 static QString GetPlayerNameFromPID(int pid)
 {
   QString player_name = QObject::tr("Invalid Player ID");
-  for (const auto* player : Settings::Instance().GetNetPlayClient()->GetPlayers())
+  auto client = Settings::Instance().GetNetPlayClient();
+  if (!client)
+    return player_name;
+
+  for (const auto* player : client->GetPlayers())
   {
     if (player->pid == pid)
     {
@@ -82,7 +86,11 @@ void MD5Dialog::show(const QString& title)
   m_results.clear();
   m_check_label->setText(QString::fromStdString(""));
 
-  for (const auto* player : Settings::Instance().GetNetPlayClient()->GetPlayers())
+  auto client = Settings::Instance().GetNetPlayClient();
+  if (!client)
+    return;
+
+  for (const auto* player : client->GetPlayers())
   {
     m_progress_bars[player->pid] = new QProgressBar;
     m_status_labels[player->pid] = new QLabel;
@@ -118,7 +126,8 @@ void MD5Dialog::SetResult(int pid, const std::string& result)
 
   m_results.push_back(result);
 
-  if (m_results.size() >= Settings::Instance().GetNetPlayClient()->GetPlayers().size())
+  auto client = Settings::Instance().GetNetPlayClient();
+  if (client && m_results.size() >= client->GetPlayers().size())
   {
     if (std::adjacent_find(m_results.begin(), m_results.end(), std::not_equal_to<>()) ==
         m_results.end())
@@ -134,7 +143,7 @@ void MD5Dialog::SetResult(int pid, const std::string& result)
 
 void MD5Dialog::reject()
 {
-  auto* server = Settings::Instance().GetNetPlayServer();
+  auto server = Settings::Instance().GetNetPlayServer();
 
   if (server)
     server->AbortMD5();
diff --git a/Source/Core/DolphinQt/NetPlay/NetPlayDialog.cpp b/Source/Core/DolphinQt/NetPlay/NetPlayDialog.cpp
index ce296acef8..5e3d757eeb 100644
--- a/Source/Core/DolphinQt/NetPlay/NetPlayDialog.cpp
+++ b/Source/Core/DolphinQt/NetPlay/NetPlayDialog.cpp
@@ -240,8 +240,9 @@ void NetPlayDialog::ConnectWidgets()
             if (value == m_buffer_size)
               return;
 
-            if (Settings::Instance().GetNetPlayServer() != nullptr)
-              Settings::Instance().GetNetPlayServer()->AdjustPadBufferSize(value);
+            auto server = Settings::Instance().GetNetPlayServer();
+            if (server)
+              server->AdjustPadBufferSize(value);
           });
 
   connect(m_start_button, &QPushButton::clicked, this, &NetPlayDialog::OnStart);
@@ -371,7 +372,10 @@ void NetPlayDialog::show(std::string nickname, bool use_traversal)
 
 void NetPlayDialog::UpdateGUI()
 {
-  auto* client = Settings::Instance().GetNetPlayClient();
+  auto client = Settings::Instance().GetNetPlayClient();
+  auto server = Settings::Instance().GetNetPlayServer();
+  if (!client)
+    return;
 
   // Update Player List
   const auto players = client->GetPlayers();
@@ -466,11 +470,10 @@ void NetPlayDialog::UpdateGUI()
       break;
     }
   }
-  else if (Settings::Instance().GetNetPlayServer())
+  else if (server)
   {
-    m_hostcode_label->setText(
-        QString::fromStdString(Settings::Instance().GetNetPlayServer()->GetInterfaceHost(
-            m_room_box->currentData().toString().toStdString())));
+    m_hostcode_label->setText(QString::fromStdString(
+        server->GetInterfaceHost(m_room_box->currentData().toString().toStdString())));
     m_hostcode_action_button->setText(tr("Copy"));
     m_hostcode_action_button->setEnabled(true);
   }
@@ -555,7 +558,7 @@ void NetPlayDialog::GameStatusChanged(bool running)
 
 void NetPlayDialog::SetOptionsEnabled(bool enabled)
 {
-  if (Settings::Instance().GetNetPlayServer() != nullptr)
+  if (Settings::Instance().GetNetPlayServer())
   {
     m_start_button->setEnabled(enabled);
     m_game_button->setEnabled(enabled);
@@ -574,7 +577,9 @@ void NetPlayDialog::OnMsgStartGame()
   DisplayMessage(tr("Started game"), "green");
 
   QueueOnObject(this, [this] {
-    Settings::Instance().GetNetPlayClient()->StartGame(FindGame(m_current_game));
+    auto client = Settings::Instance().GetNetPlayClient();
+    if (client)
+      client->StartGame(FindGame(m_current_game));
   });
 }
 
diff --git a/Source/Core/DolphinQt/NetPlay/PadMappingDialog.cpp b/Source/Core/DolphinQt/NetPlay/PadMappingDialog.cpp
index fbfe64e538..d1b7c97ee4 100644
--- a/Source/Core/DolphinQt/NetPlay/PadMappingDialog.cpp
+++ b/Source/Core/DolphinQt/NetPlay/PadMappingDialog.cpp
@@ -59,8 +59,8 @@ void PadMappingDialog::ConnectWidgets()
 
 int PadMappingDialog::exec()
 {
-  auto* client = Settings::Instance().GetNetPlayClient();
-  auto* server = Settings::Instance().GetNetPlayServer();
+  auto client = Settings::Instance().GetNetPlayClient();
+  auto server = Settings::Instance().GetNetPlayServer();
   // Load Settings
   m_players = client->GetPlayers();
   m_pad_mapping = server->GetPadMapping();
diff --git a/Source/Core/DolphinQt/Settings.cpp b/Source/Core/DolphinQt/Settings.cpp
index c1dce6b29d..56183daa86 100644
--- a/Source/Core/DolphinQt/Settings.cpp
+++ b/Source/Core/DolphinQt/Settings.cpp
@@ -276,9 +276,9 @@ GameListModel* Settings::GetGameListModel() const
   return model;
 }
 
-NetPlay::NetPlayClient* Settings::GetNetPlayClient()
+std::shared_ptr<NetPlay::NetPlayClient> Settings::GetNetPlayClient()
 {
-  return m_client.get();
+  return m_client;
 }
 
 void Settings::ResetNetPlayClient(NetPlay::NetPlayClient* client)
@@ -286,9 +286,9 @@ void Settings::ResetNetPlayClient(NetPlay::NetPlayClient* client)
   m_client.reset(client);
 }
 
-NetPlay::NetPlayServer* Settings::GetNetPlayServer()
+std::shared_ptr<NetPlay::NetPlayServer> Settings::GetNetPlayServer()
 {
-  return m_server.get();
+  return m_server;
 }
 
 void Settings::ResetNetPlayServer(NetPlay::NetPlayServer* server)
diff --git a/Source/Core/DolphinQt/Settings.h b/Source/Core/DolphinQt/Settings.h
index 68303530fa..377ff11586 100644
--- a/Source/Core/DolphinQt/Settings.h
+++ b/Source/Core/DolphinQt/Settings.h
@@ -25,7 +25,7 @@ namespace NetPlay
 {
 class NetPlayClient;
 class NetPlayServer;
-}
+}  // namespace NetPlay
 
 class GameListModel;
 class InputConfig;
@@ -98,9 +98,9 @@ public:
   void DecreaseVolume(int volume);
 
   // NetPlay
-  NetPlay::NetPlayClient* GetNetPlayClient();
+  std::shared_ptr<NetPlay::NetPlayClient> GetNetPlayClient();
   void ResetNetPlayClient(NetPlay::NetPlayClient* client = nullptr);
-  NetPlay::NetPlayServer* GetNetPlayServer();
+  std::shared_ptr<NetPlay::NetPlayServer> GetNetPlayServer();
   void ResetNetPlayServer(NetPlay::NetPlayServer* server = nullptr);
 
   // Cheats
@@ -169,8 +169,8 @@ signals:
 private:
   bool m_batch = false;
   bool m_controller_state_needed = false;
-  std::unique_ptr<NetPlay::NetPlayClient> m_client;
-  std::unique_ptr<NetPlay::NetPlayServer> m_server;
+  std::shared_ptr<NetPlay::NetPlayClient> m_client;
+  std::shared_ptr<NetPlay::NetPlayServer> m_server;
   Settings();
 };