Merge pull request #2045 from majestrate/windows-service-issues

windows platform bug fixes

.drone.jsonnet

@@ -163,7 +163,7 @@ local windows_cross_pipeline(name,
         'echo "man-db man-db/auto-update boolean false" | debconf-set-selections',
         apt_get_quiet + ' update',
         apt_get_quiet + ' install -y eatmydata',
-        'eatmydata ' + apt_get_quiet + ' install --no-install-recommends -y build-essential cmake git pkg-config ccache g++-mingw-w64-x86-64-posix nsis zip icoutils automake libtool librsvg2-bin',
+        'eatmydata ' + apt_get_quiet + ' install --no-install-recommends -y build-essential cmake git pkg-config ccache g++-mingw-w64-x86-64-posix nsis zip icoutils automake libtool librsvg2-bin bison',
         'update-alternatives --set x86_64-w64-mingw32-gcc /usr/bin/x86_64-w64-mingw32-gcc-posix',
         'update-alternatives --set x86_64-w64-mingw32-g++ /usr/bin/x86_64-w64-mingw32-g++-posix',
         'JOBS=' + jobs + ' VERBOSE=1 ./contrib/windows.sh -DSTRIP_SYMBOLS=ON -DGUI_EXE=$${DRONE_WORKSPACE}/gui/release/Lokinet-GUI_portable.exe' +

cmake/StaticBuild.cmake

@@ -306,8 +306,15 @@ build_external(expat
 )
 add_static_target(expat expat_external libexpat.a)
 
+
+if(WIN32)
+  set(unbound_patch
+    PATCH_COMMAND ${PROJECT_SOURCE_DIR}/contrib/apply-patches.sh
+        ${PROJECT_SOURCE_DIR}/contrib/patches/unbound-delete-crash-fix.patch)
+endif()
 build_external(unbound
   DEPENDS openssl_external expat_external
+  ${unbound_patch}
   CONFIGURE_COMMAND ./configure ${cross_host} ${cross_rc} --prefix=${DEPS_DESTDIR} --disable-shared
   --enable-static --with-libunbound-only --with-pic
   --$<IF:$<BOOL:${WITH_LTO}>,enable,disable>-flto --with-ssl=${DEPS_DESTDIR}

contrib/patches/unbound-delete-crash-fix.patch

@@ -0,0 +1,33 @@
+commit 56d816014d5e8a7eb055169c7e13a303dad5e50f
+Author: Jason Rhinelander <[email protected]>
+Date:   Mon Oct 31 22:07:03 2022 -0300
+
+    Set tube->ev_listen to NULL to prevent double unregister
+
+    On windows when using threaded mode (i.e. `ub_ctx_async(ctx, 1)`)
+    tube_remove_bg_listen gets called twice: once when the thread does its
+    own cleanup, then again in `tube_delete()`.  Because `ev_listen` doesn't
+    get cleared, however, we end we calling ub_winsock_unregister_wsaevent
+    with a freed pointer.
+
+    This doesn't always manifest because, apparently, for various compilers
+    and settings that memory *might* be overwritten in which case the
+    additional check for ev->magic will prevent anything actually happening,
+    but in my case under mingw32 that doesn't happen and we end up
+    eventually crashing.
+
+    This fixes the crash by properly NULLing the pointer so that the second
+    ub_winsock_unregister_wsaevent(...) becomes a no-op.
+
+diff --git a/util/tube.c b/util/tube.c
+index 43455fee..a92dfa77 100644
+--- a/util/tube.c
++++ b/util/tube.c
+@@ -570,6 +570,7 @@ void tube_remove_bg_listen(struct tube* tube)
+ {
+ 	verbose(VERB_ALGO, "tube remove_bg_listen");
+ 	ub_winsock_unregister_wsaevent(tube->ev_listen);
++	tube->ev_listen = NULL;
+ }
+
+ void tube_remove_bg_write(struct tube* tube)

daemon/lokinet.cpp

@@ -7,7 +7,10 @@
 #include <llarp/util/str.hpp>
 
 #ifdef _WIN32
+#include <llarp/win32/service_manager.hpp>
 #include <dbghelp.h>
+#else
+#include <llarp/util/service_manager.hpp>
 #endif
 
 #include <csignal>
@@ -21,25 +24,24 @@ int
 lokinet_main(int, char**);
 
 #ifdef _WIN32
-#include <strsafe.h>
 extern "C" LONG FAR PASCAL
 win32_signal_handler(EXCEPTION_POINTERS*);
 extern "C" VOID FAR PASCAL
 win32_daemon_entry(DWORD, LPTSTR*);
-BOOL ReportSvcStatus(DWORD, DWORD, DWORD);
+
 VOID
 insert_description();
-SERVICE_STATUS SvcStatus;
-SERVICE_STATUS_HANDLE SvcStatusHandle;
-bool start_as_daemon = false;
+
 #endif
 
+static auto logcat = llarp::log::Cat("main");
 std::shared_ptr<llarp::Context> ctx;
 std::promise<int> exit_code;
 
 void
 handle_signal(int sig)
 {
+  llarp::log::info(logcat, "Handling signal {}", sig);
   if (ctx)
     ctx->loop->call([sig] { ctx->HandleSignal(sig); });
   else
@@ -82,9 +84,6 @@ install_win32_daemon()
     llarp::LogError("Cannot install service ", GetLastError());
     return;
   }
-  // just put the flag here. we eat it later on and specify the
-  // config path in the daemon entry point
-  StringCchCat(szPath.data(), 1024, " --win32-daemon");
 
   // Get a handle to the SCM database.
   schSCManager = OpenSCManager(
@@ -292,37 +291,6 @@ run_main_context(std::optional<fs::path> confFile, const llarp::RuntimeOptions o
 }
 
 #ifdef _WIN32
-void
-TellWindowsServiceStopped()
-{
-  ::WSACleanup();
-  if (not start_as_daemon)
-    return;
-
-  llarp::LogInfo("Telling Windows the service has stopped.");
-  if (not ReportSvcStatus(SERVICE_STOPPED, NO_ERROR, 0))
-  {
-    auto error_code = GetLastError();
-    if (error_code == ERROR_INVALID_DATA)
-      llarp::LogError(
-          "SetServiceStatus failed: \"The specified service status structure is invalid.\"");
-    else if (error_code == ERROR_INVALID_HANDLE)
-      llarp::LogError("SetServiceStatus failed: \"The specified handle is invalid.\"");
-    else
-      llarp::LogError("SetServiceStatus failed with an unknown error.");
-  }
-}
-
-class WindowsServiceStopped
-{
- public:
-  WindowsServiceStopped() = default;
-
-  ~WindowsServiceStopped()
-  {
-    TellWindowsServiceStopped();
-  }
-};
 
 /// minidump generation for windows jizz
 /// will make a coredump when there is an unhandled exception
@@ -363,46 +331,57 @@ GenerateDump(EXCEPTION_POINTERS* pExceptionPointers)
 int
 main(int argc, char* argv[])
 {
+  // Set up a default, stderr logging for very early logging; we'll replace this later once we read
+  // the desired log info from config.
+  llarp::log::add_sink(llarp::log::Type::Print, "stderr");
+  llarp::log::reset_level(llarp::log::Level::info);
+
+  llarp::logRingBuffer = std::make_shared<llarp::log::RingBufferSink>(100);
+  llarp::log::add_sink(llarp::logRingBuffer, llarp::log::DEFAULT_PATTERN_MONO);
+
 #ifndef _WIN32
   return lokinet_main(argc, argv);
 #else
   SERVICE_TABLE_ENTRY DispatchTable[] = {
       {strdup("lokinet"), (LPSERVICE_MAIN_FUNCTION)win32_daemon_entry}, {NULL, NULL}};
-  if (lstrcmpi(argv[1], "--win32-daemon") == 0)
+
+  // Try first to run as a service; if this works it fires off to win32_daemon_entry and doesn't
+  // return until the service enters STOPPED state.
+  if (StartServiceCtrlDispatcher(DispatchTable))
+    return 0;
+
+  auto error = GetLastError();
+
+  // We'll get this error if not invoked as a service, which is fine: we can just run directly
+  if (error == ERROR_FAILED_SERVICE_CONTROLLER_CONNECT)
   {
-    start_as_daemon = true;
-    StartServiceCtrlDispatcher(DispatchTable);
+    llarp::sys::service_manager->disable();
+    return lokinet_main(argc, argv);
   }
   else
-    return lokinet_main(argc, argv);
+  {
+    llarp::log::critical(
+        logcat, "Error launching service: {}", std::system_category().message(error));
+    return 1;
+  }
 #endif
 }
 
 int
-lokinet_main(int argc, char* argv[])
+lokinet_main(int argc, char** argv)
 {
   if (auto result = Lokinet_INIT())
     return result;
 
-  // Set up a default, stderr logging for very early logging; we'll replace this later once we read
-  // the desired log info from config.
-  llarp::log::add_sink(llarp::log::Type::Print, "stderr");
-  llarp::log::reset_level(llarp::log::Level::info);
-
-  llarp::logRingBuffer = std::make_shared<llarp::log::RingBufferSink>(100);
-  llarp::log::add_sink(llarp::logRingBuffer, llarp::log::DEFAULT_PATTERN_MONO);
-
   llarp::RuntimeOptions opts;
   opts.showBanner = false;
 
 #ifdef _WIN32
-  WindowsServiceStopped stopped_raii;
   if (startWinsock())
     return -1;
   SetConsoleCtrlHandler(handle_signal_win32, TRUE);
-
-  // SetUnhandledExceptionFilter(win32_signal_handler);
 #endif
+
   cxxopts::Options options(
       "lokinet",
       "LokiNET is a free, open source, private, "
@@ -543,13 +522,9 @@ lokinet_main(int argc, char* argv[])
   SetUnhandledExceptionFilter(&GenerateDump);
 #endif
 
-  std::thread main_thread{[&] { run_main_context(configFile, opts); }};
+  std::thread main_thread{[configFile, opts] { run_main_context(configFile, opts); }};
   auto ftr = exit_code.get_future();
 
-#ifdef _WIN32
-  ReportSvcStatus(SERVICE_RUNNING, NO_ERROR, 0);
-#endif
-
   do
   {
     // do periodic non lokinet related tasks here
@@ -580,9 +555,7 @@ lokinet_main(int argc, char* argv[])
         llarp::log::critical(deadlock_cat, wtf);
         llarp::log::flush();
       }
-#ifdef _WIN32
-      TellWindowsServiceStopped();
-#endif
+      llarp::sys::service_manager->failed();
       std::abort();
     }
   } while (ftr.wait_for(std::chrono::seconds(1)) != std::future_status::ready);
@@ -607,6 +580,7 @@ lokinet_main(int argc, char* argv[])
   }
 
   llarp::log::flush();
+  llarp::sys::service_manager->stopped();
   if (ctx)
   {
     ctx.reset();
@@ -615,29 +589,6 @@ lokinet_main(int argc, char* argv[])
 }
 
 #ifdef _WIN32
-BOOL
-ReportSvcStatus(DWORD dwCurrentState, DWORD dwWin32ExitCode, DWORD dwWaitHint)
-{
-  static DWORD dwCheckPoint = 1;
-
-  // Fill in the SERVICE_STATUS structure.
-  SvcStatus.dwCurrentState = dwCurrentState;
-  SvcStatus.dwWin32ExitCode = dwWin32ExitCode;
-  SvcStatus.dwWaitHint = dwWaitHint;
-
-  if (dwCurrentState == SERVICE_START_PENDING)
-    SvcStatus.dwControlsAccepted = 0;
-  else
-    SvcStatus.dwControlsAccepted = SERVICE_ACCEPT_STOP;
-
-  if ((dwCurrentState == SERVICE_RUNNING) || (dwCurrentState == SERVICE_STOPPED))
-    SvcStatus.dwCheckPoint = 0;
-  else
-    SvcStatus.dwCheckPoint = dwCheckPoint++;
-
-  // Report the status of the service to the SCM.
-  return SetServiceStatus(SvcStatusHandle, &SvcStatus);
-}
 
 VOID FAR PASCAL
 SvcCtrlHandler(DWORD dwCtrl)
@@ -647,44 +598,45 @@ SvcCtrlHandler(DWORD dwCtrl)
   switch (dwCtrl)
   {
     case SERVICE_CONTROL_STOP:
-      ReportSvcStatus(SERVICE_STOPPED, NO_ERROR, 0);
-      // Signal the service to stop.
+      // tell service we are stopping
+      llarp::log::debug(logcat, "Windows service controller gave SERVICE_CONTROL_STOP");
+      llarp::sys::service_manager->system_changed_our_state(llarp::sys::ServiceState::Stopping);
       handle_signal(SIGINT);
       return;
 
     case SERVICE_CONTROL_INTERROGATE:
-      break;
+      // 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;
   }
 }
 
-// The win32 daemon entry point is just a trampoline that returns control
-// to the original lokinet entry
-// and only gets called if we get --win32-daemon in the command line
+// The win32 daemon entry point is where we go when invoked as a windows service; we do the required
+// service dance and then pretend we were invoked via main().
 VOID FAR PASCAL
-win32_daemon_entry(DWORD argc, LPTSTR* argv)
+win32_daemon_entry(DWORD, LPTSTR* argv)
 {
   // Register the handler function for the service
-  SvcStatusHandle = RegisterServiceCtrlHandler("lokinet", SvcCtrlHandler);
+  auto* svc = dynamic_cast<llarp::sys::SVC_Manager*>(llarp::sys::service_manager);
+  svc->handle = RegisterServiceCtrlHandler("lokinet", SvcCtrlHandler);
 
-  if (!SvcStatusHandle)
+  if (svc->handle == nullptr)
   {
     llarp::LogError("failed to register daemon control handler");
     return;
   }
 
-  // These SERVICE_STATUS members remain as set here
-  SvcStatus.dwServiceType = SERVICE_WIN32_OWN_PROCESS;
-  SvcStatus.dwServiceSpecificExitCode = 0;
-
-  // Report initial status to the SCM
-  ReportSvcStatus(SERVICE_START_PENDING, NO_ERROR, 3000);
-  // SCM clobbers startup args, regenerate them here
-  argc = 2;
-  argv[1] = strdup("c:\\programdata\\lokinet\\lokinet.ini");
-  argv[2] = nullptr;
-  lokinet_main(argc, argv);
+  // we hard code the args to lokinet_main.
+  // we yoink argv[0] (lokinet.exe path) and pass in the new args.
+  std::array args = {
+      reinterpret_cast<char*>(argv[0]),
+      reinterpret_cast<char*>(strdup("c:\\programdata\\lokinet\\lokinet.ini")),
+      reinterpret_cast<char*>(0)};
+  lokinet_main(args.size() - 1, args.data());
 }
 #endif

include/llarp.hpp

@@ -45,6 +45,7 @@ namespace llarp
     std::shared_ptr<NodeDB> nodedb = nullptr;
     std::string nodedb_dir;
 
+    Context();
     virtual ~Context() = default;
 
     void

llarp/CMakeLists.txt

@@ -49,17 +49,23 @@ target_link_libraries(lokinet-platform PUBLIC lokinet-cryptography lokinet-util
 target_link_libraries(lokinet-platform PRIVATE oxenmq::oxenmq)
 
 if (ANDROID)
-  target_sources(lokinet-platform PRIVATE android/ifaddrs.c)
+  target_sources(lokinet-platform PRIVATE android/ifaddrs.c util/nop_service_manager.cpp)
 endif()
 
 if(CMAKE_SYSTEM_NAME MATCHES "Linux")
   target_sources(lokinet-platform PRIVATE linux/dbus.cpp)
+  if(WITH_SYSTEMD)
+    target_sources(lokinet-platform PRIVATE linux/sd_service_manager.cpp)
+  else()
+    target_sources(lokinet-platform PRIVATE util/nop_service_manager.cpp)
+  endif()
 endif()
 
 if (WIN32)
   target_sources(lokinet-platform PRIVATE
     net/win32.cpp
     vpn/win32.cpp
+    win32/service_manager.cpp
     win32/exec.cpp)
   add_library(lokinet-win32 STATIC
     win32/dll.cpp
@@ -312,6 +318,7 @@ endif()
 
 if(APPLE)
   add_subdirectory(apple)
+  target_sources(lokinet-platform PRIVATE util/nop_service_manager.cpp)
 endif()
 
 file(GLOB_RECURSE docs_SRC */*.hpp *.hpp)