From 6cfe5de00dcd300cc2f10cd40f028f92ed8b7b04 Mon Sep 17 00:00:00 2001
From: Peter Johnson <johnson.peter@gmail.com>
Date: Tue, 27 Dec 2022 10:25:48 -0800
Subject: [PATCH] [ntcore] Don't deadlock server on early destroy (#4863)

It was possible to deadlock on instance destroy if the server had started
but had not yet fully initialized its handles.
---
 ntcore/src/main/native/cpp/InstanceImpl.cpp  | 13 ++++++++-----
 ntcore/src/main/native/cpp/NetworkServer.cpp |  9 +++++++++
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/ntcore/src/main/native/cpp/InstanceImpl.cpp b/ntcore/src/main/native/cpp/InstanceImpl.cpp
index 3fef3563bd..5ea1cad479 100644
--- a/ntcore/src/main/native/cpp/InstanceImpl.cpp
+++ b/ntcore/src/main/native/cpp/InstanceImpl.cpp
@@ -113,12 +113,15 @@ void InstanceImpl::StartServer(std::string_view persistFilename,
 }
 
 void InstanceImpl::StopServer() {
-  std::scoped_lock lock{m_mutex};
-  if ((networkMode & NT_NET_MODE_SERVER) == 0) {
-    return;
+  std::shared_ptr<NetworkServer> server;
+  {
+    std::scoped_lock lock{m_mutex};
+    if ((networkMode & NT_NET_MODE_SERVER) == 0) {
+      return;
+    }
+    server = std::move(m_networkServer);
+    networkMode = NT_NET_MODE_NONE;
   }
-  m_networkServer.reset();
-  networkMode = NT_NET_MODE_NONE;
 }
 
 void InstanceImpl::StartClient3(std::string_view identity) {
diff --git a/ntcore/src/main/native/cpp/NetworkServer.cpp b/ntcore/src/main/native/cpp/NetworkServer.cpp
index 971a1fdc78..0086d24834 100644
--- a/ntcore/src/main/native/cpp/NetworkServer.cpp
+++ b/ntcore/src/main/native/cpp/NetworkServer.cpp
@@ -108,6 +108,7 @@ class NSImpl {
          unsigned int port3, unsigned int port4,
          net::ILocalStorage& localStorage, IConnectionList& connList,
          wpi::Logger& logger, std::function<void()> initDone);
+  ~NSImpl();
 
   void HandleLocal();
   void LoadPersistent();
@@ -131,6 +132,7 @@ class NSImpl {
   std::shared_ptr<uv::Timer> m_savePersistentTimer;
   std::shared_ptr<uv::Async<>> m_flushLocal;
   std::shared_ptr<uv::Async<>> m_flush;
+  bool m_shutdown = false;
 
   std::vector<net::ClientMessage> m_localMsgs;
 
@@ -344,6 +346,10 @@ NSImpl::NSImpl(std::string_view persistentFilename,
   });
 }
 
+NSImpl::~NSImpl() {
+  m_loopRunner.ExecAsync([this](uv::Loop&) { m_shutdown = true; });
+}
+
 void NSImpl::HandleLocal() {
   m_localQueue.ReadQueue(&m_localMsgs);
   m_serverImpl.HandleLocal(m_localMsgs);
@@ -395,6 +401,9 @@ void NSImpl::SavePersistent(std::string_view filename, std::string_view data) {
 }
 
 void NSImpl::Init() {
+  if (m_shutdown) {
+    return;
+  }
   auto errs = m_serverImpl.LoadPersistent(m_persistentData);
   if (!errs.empty()) {
     WARNING("error reading persistent file: {}", errs);