Fix crashes in wintun and windivert stopping

Fixes windows shutdown crashes:

- windivert wasn't handling an ERROR_NO_DATA, which it gets when
  finished handling everything after a shutdown.
- wintun ReadPacket still gets invoked after end_session is called, but
  shouldn't be.  This adds an atomic<bool> to early return.
- fixes up some settings we send for windows service manager notify

daemon/lokinet.cpp

@@ -592,17 +592,19 @@ SvcCtrlHandler(DWORD dwCtrl)
   {
     case SERVICE_CONTROL_STOP:
       // tell service we are stopping
-      llarp::log::info(logcat, "Windows service controller gave SERVICE_CONTROL_STOP");
+      llarp::log::debug(logcat, "Windows service controller gave SERVICE_CONTROL_STOP");
       llarp::sys::service_manager->system_changed_our_state(llarp::sys::ServiceState::Stopping);
       return;
 
     case SERVICE_CONTROL_INTERROGATE:
+      llarp::log::debug(logcat, "Got win32 service interrogate signal");
       // report status
       llarp::log::debug(logcat, "Got win32 service interrogate signal");
       llarp::sys::service_manager->report_changed_state();
       return;
 
     default:
+      llarp::log::debug(logcat, "Got win32 unhandled signal {}", dwCtrl);
       break;
   }
 }

llarp/handlers/tun.cpp

@@ -35,6 +35,8 @@ namespace llarp
 {
   namespace handlers
   {
+    static auto logcat = log::Cat("tun");
+
     bool
     TunEndpoint::MaybeHookDNS(
         std::shared_ptr<dns::PacketSource_Base> source,

llarp/service/context.cpp

@@ -11,6 +11,7 @@ namespace llarp
 {
   namespace service
   {
+    static auto logcat = log::Cat("service");
     namespace
     {
       using EndpointConstructor =

llarp/service/endpoint.cpp

@@ -49,6 +49,9 @@ namespace llarp
 {
   namespace service
   {
+
+    static auto logcat = log::Cat("endpoint");
+
     Endpoint::Endpoint(AbstractRouter* r, Context* parent)
         : path::Builder{r, 3, path::default_len}
         , context{parent}

llarp/win32/service_manager.cpp

@@ -1,5 +1,6 @@
 #include <windows.h>
 #include <llarp.hpp>
+#include <llarp/util/logging.hpp>
 #include "service_manager.hpp"
 #include <dbghelp.h>
 #include <cassert>
@@ -9,6 +10,8 @@
 namespace llarp::sys
 {
 
+  static auto logcat = log::Cat("svc");
+
   namespace
   {
 
@@ -53,6 +56,21 @@ namespace llarp::sys
   {
     if (m_disable)
       return;
+
+    log::debug(
+        logcat,
+        "Reporting Windows service status '{}', exit code {}, wait hint {}, dwCP {}, dwCA {}",
+        _status.dwCurrentState == SERVICE_START_PENDING ? "start pending"
+            : _status.dwCurrentState == SERVICE_RUNNING ? "running"
+            : _status.dwCurrentState == SERVICE_STOPPED ? "stopped"
+            : _status.dwCurrentState == SERVICE_STOP_PENDING
+            ? "stop pending"
+            : fmt::format("unknown: {}", _status.dwCurrentState),
+        _status.dwWin32ExitCode,
+        _status.dwWaitHint,
+        _status.dwCheckPoint,
+        _status.dwControlsAccepted);
+
     SetServiceStatus(handle, &_status);
   }
 
@@ -69,7 +87,9 @@ namespace llarp::sys
     {
       auto new_state = *maybe_state;
       assert(_status.dwCurrentState != new_state);
+      _status.dwWin32ExitCode = NO_ERROR;
       _status.dwCurrentState = new_state;
+      _status.dwControlsAccepted = st == ServiceState::Running ? SERVICE_ACCEPT_STOP : 0;
       // tell windows it takes 5s at most to start or stop
       if (st == ServiceState::Starting or st == ServiceState::Stopping)
         _status.dwCheckPoint++;

llarp/win32/windivert.cpp

@@ -11,14 +11,11 @@ extern "C"
 {
 #include <windivert.h>
 }
-namespace L = llarp::log;
 
 namespace llarp::win32
 {
-  namespace
-  {
-    auto cat = L::Cat("windivert");
-  }
+  static auto logcat = log::Cat("windivert");
+
   namespace wd
   {
     namespace
@@ -73,7 +70,7 @@ namespace llarp::win32
           : m_Wake{wake}, m_RecvQueue{recv_queue_size}
       {
         wd::Initialize();
-        L::info(cat, "load windivert with filterspec: '{}'", filter_spec);
+        log::info(logcat, "load windivert with filterspec: '{}'", filter_spec);
 
         m_Handle = wd::open(filter_spec.c_str(), WINDIVERT_LAYER_NETWORK, 0, 0);
         if (auto err = GetLastError())
@@ -95,14 +92,19 @@ namespace llarp::win32
         if (not wd::recv(m_Handle, pkt.data(), pkt.size(), &sz, &addr))
         {
           auto err = GetLastError();
-          if (err and err != ERROR_BROKEN_PIPE)
-            throw win32::error{
-                err, fmt::format("failed to receive packet from windivert (code={})", err)};
-          else if (err)
+          if (err == ERROR_NO_DATA)
+            return std::nullopt;
+          if (err == ERROR_BROKEN_PIPE)
+          {
             SetLastError(0);
-          return std::nullopt;
+            return std::nullopt;
+          }
+
+          log::critical(logcat, "error receiving packet: {}", err);
+          throw win32::error{
+              err, fmt::format("failed to receive packet from windivert (code={})", err)};
         }
-        L::trace(cat, "got packet of size {}B", sz);
+        log::trace(logcat, "got packet of size {}B", sz);
         pkt.resize(sz);
         return Packet{std::move(pkt), std::move(addr)};
       }
@@ -112,7 +114,7 @@ namespace llarp::win32
       {
         const auto& pkt = w_pkt.pkt;
         const auto* addr = &w_pkt.addr;
-        L::trace(cat, "send dns packet of size {}B", pkt.size());
+        log::trace(logcat, "send dns packet of size {}B", pkt.size());
         UINT sz{};
         if (wd::send(m_Handle, pkt.data(), pkt.size(), &sz, addr))
           return;
@@ -147,12 +149,12 @@ namespace llarp::win32
       void
       Start() override
       {
-        L::info(cat, "starting windivert");
+        log::info(logcat, "starting windivert");
         if (m_Runner.joinable())
           throw std::runtime_error{"windivert thread is already running"};
 
         auto read_loop = [this]() {
-          log::debug(cat, "windivert read loop start");
+          log::debug(logcat, "windivert read loop start");
           while (true)
           {
             // in the read loop, read packets until they stop coming in
@@ -166,7 +168,7 @@ namespace llarp::win32
             else  // leave loop on read fail
               break;
           }
-          log::debug(cat, "windivert read loop end");
+          log::debug(logcat, "windivert read loop end");
         };
 
         m_Runner = std::thread{std::move(read_loop)};
@@ -175,7 +177,7 @@ namespace llarp::win32
       void
       Stop() override
       {
-        L::info(cat, "stopping windivert");
+        log::info(logcat, "stopping windivert");
         wd::shutdown(m_Handle, WINDIVERT_SHUTDOWN_BOTH);
         m_Runner.join();
       }

llarp/win32/wintun.cpp

@@ -199,6 +199,8 @@ namespace llarp::win32
     {
       WINTUN_SESSION_HANDLE _impl;
       HANDLE _handle;
+      std::atomic<bool> ended{false};
+      static_assert(std::atomic<bool>::is_always_lock_free);
 
      public:
       WintunSession() : _impl{nullptr}, _handle{nullptr}
@@ -217,8 +219,9 @@ namespace llarp::win32
       }
 
       void
-      Stop() const
+      Stop()
       {
+        ended = true;
         end_session(_impl);
       }
 
@@ -233,6 +236,8 @@ namespace llarp::win32
       [[nodiscard]] std::pair<std::unique_ptr<PacketWrapper>, bool>
       ReadPacket() const
       {
+        if (ended)
+          return {nullptr, true};
         DWORD sz;
         if (auto* ptr = read_packet(_impl, &sz))
           return {std::unique_ptr<PacketWrapper>{new PacketWrapper{ptr, sz, _impl}}, false};