From fcc4f6050db153dc16aebe2ac283b0f77a24508e Mon Sep 17 00:00:00 2001 From: Thomas Winget Date: Thu, 27 Oct 2022 00:29:50 -0400 Subject: [PATCH 01/46] wait until actually stopped to tell windows we are We should send STOP_PENDING rather than STOPPED while we aren't yet actually stopped; STOPPED is already sent when we actually finish stopping. Also fixes some silly argc/argv shenanigans, I think. --- daemon/lokinet.cpp | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/daemon/lokinet.cpp b/daemon/lokinet.cpp index af71310a5f..a618e5aa72 100644 --- a/daemon/lokinet.cpp +++ b/daemon/lokinet.cpp @@ -379,7 +379,7 @@ main(int argc, char* argv[]) } int -lokinet_main(int argc, char* argv[]) +lokinet_main(int argc, char** argv) { if (auto result = Lokinet_INIT()) return result; @@ -647,7 +647,7 @@ SvcCtrlHandler(DWORD dwCtrl) switch (dwCtrl) { case SERVICE_CONTROL_STOP: - ReportSvcStatus(SERVICE_STOPPED, NO_ERROR, 0); + ReportSvcStatus(SERVICE_STOP_PENDING, NO_ERROR, 0); // Signal the service to stop. handle_signal(SIGINT); return; @@ -664,7 +664,7 @@ SvcCtrlHandler(DWORD dwCtrl) // to the original lokinet entry // and only gets called if we get --win32-daemon in the command line 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); @@ -681,10 +681,14 @@ win32_daemon_entry(DWORD argc, LPTSTR* argv) // 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); + // SCM calls this function with different args than a normal "main" expects, + // but lokinet_main expects normal args, so set them here instead. At the + // moment we are not passing any args to lokinet_main this way anyway though. + + std::array args = { + reinterpret_cast(argv[0]), + reinterpret_cast(strdup("c:\\programdata\\lokinet\\lokinet.ini")), + reinterpret_cast(0)}; + lokinet_main(args.size() - 1, args.data()); } #endif From a9a2a115bc2df60f6895d2717ceb5d3380d29e3b Mon Sep 17 00:00:00 2001 From: Thomas Winget Date: Thu, 27 Oct 2022 00:55:03 -0400 Subject: [PATCH 02/46] debian missing yacc apparently all of a sudden --- .drone.jsonnet | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.drone.jsonnet b/.drone.jsonnet index 10405784ea..d73d4193a5 100644 --- a/.drone.jsonnet +++ b/.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' + From 9cdfae2e42aa22b348c62973fa7cea4f9b9a3fe2 Mon Sep 17 00:00:00 2001 From: Jeff Becker Date: Thu, 27 Oct 2022 10:54:43 -0400 Subject: [PATCH 03/46] correct windows service manager behavior. report status to window service manager when we get and iterogate message from the service manager. update comments to reflect these changes. --- daemon/lokinet.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/daemon/lokinet.cpp b/daemon/lokinet.cpp index a618e5aa72..7a20d2742b 100644 --- a/daemon/lokinet.cpp +++ b/daemon/lokinet.cpp @@ -647,13 +647,16 @@ SvcCtrlHandler(DWORD dwCtrl) switch (dwCtrl) { case SERVICE_CONTROL_STOP: + // tell servicve we are stopping ReportSvcStatus(SERVICE_STOP_PENDING, NO_ERROR, 0); - // Signal the service to stop. + // do the actual tear down handle_signal(SIGINT); return; case SERVICE_CONTROL_INTERROGATE: - break; + // report status + SetServiceStatus(SvcStatusHandle, &SvcStatus); + return; default: break; From 7ddad87dbf5c7f9a76c0ffae8a4adaa6c45c5f61 Mon Sep 17 00:00:00 2001 From: Thomas Winget Date: Thu, 27 Oct 2022 11:00:40 -0400 Subject: [PATCH 04/46] some useful log statements --- daemon/lokinet.cpp | 5 ++++- llarp/context.cpp | 4 ++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/daemon/lokinet.cpp b/daemon/lokinet.cpp index 7a20d2742b..d8d59bf7c8 100644 --- a/daemon/lokinet.cpp +++ b/daemon/lokinet.cpp @@ -34,12 +34,14 @@ SERVICE_STATUS_HANDLE SvcStatusHandle; bool start_as_daemon = false; #endif +static auto logcat = llarp::log::Cat("main"); std::shared_ptr ctx; std::promise exit_code; void handle_signal(int sig) { + llarp::log::info(logcat, "Handling signal {}", sig); if (ctx) ctx->loop->call([sig] { ctx->HandleSignal(sig); }); else @@ -647,7 +649,8 @@ SvcCtrlHandler(DWORD dwCtrl) switch (dwCtrl) { case SERVICE_CONTROL_STOP: - // tell servicve we are stopping + // tell service we are stopping + llarp::log::info(logcat, "Windows service controller gave SERVICE_CONTROL_STOP"); ReportSvcStatus(SERVICE_STOP_PENDING, NO_ERROR, 0); // do the actual tear down handle_signal(SIGINT); diff --git a/llarp/context.cpp b/llarp/context.cpp index e5b2074e21..9e818e5891 100644 --- a/llarp/context.cpp +++ b/llarp/context.cpp @@ -20,6 +20,8 @@ #include #endif +static auto logcat = llarp::log::Cat("llarp-context"); + namespace llarp { bool @@ -159,6 +161,7 @@ namespace llarp void Context::HandleSignal(int sig) { + llarp::log::debug(logcat, "Handling signal {}", sig); if (sig == SIGINT || sig == SIGTERM) { SigINT(); @@ -188,6 +191,7 @@ namespace llarp { if (router) { + llarp::log::debug(logcat, "Handling SIGINT"); /// async stop router on sigint router->Stop(); } From a7f3c3595b6211d055e454b73c2f821c25bfc727 Mon Sep 17 00:00:00 2001 From: Jeff Becker Date: Thu, 27 Oct 2022 11:03:45 -0400 Subject: [PATCH 05/46] fix comment to reflect reality --- daemon/lokinet.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/daemon/lokinet.cpp b/daemon/lokinet.cpp index d8d59bf7c8..391495509c 100644 --- a/daemon/lokinet.cpp +++ b/daemon/lokinet.cpp @@ -687,10 +687,9 @@ win32_daemon_entry(DWORD, LPTSTR* argv) // Report initial status to the SCM ReportSvcStatus(SERVICE_START_PENDING, NO_ERROR, 3000); - // SCM calls this function with different args than a normal "main" expects, - // but lokinet_main expects normal args, so set them here instead. At the - // moment we are not passing any args to lokinet_main this way anyway though. + // 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(argv[0]), reinterpret_cast(strdup("c:\\programdata\\lokinet\\lokinet.ini")), From a5b5fbaffe1efe545a1d333dfa1fda50d73cd7fa Mon Sep 17 00:00:00 2001 From: Jeff Becker Date: Thu, 27 Oct 2022 13:51:45 -0400 Subject: [PATCH 06/46] system layer manager add an interface type for platform dependant system layer behavior. this gives lokinet a platform agnostic way to inform the system layer of its current state. --- daemon/lokinet.cpp | 107 ++++++---------------------- include/llarp.hpp | 1 + llarp/CMakeLists.txt | 5 +- llarp/android/service_manager.cpp | 8 +++ llarp/apple/CMakeLists.txt | 2 +- llarp/apple/service_manager.cpp | 7 ++ llarp/context.cpp | 8 +++ llarp/linux/service_manager.cpp | 59 ++++++++++++++++ llarp/router/router.cpp | 14 ++-- llarp/router/router.hpp | 1 + llarp/util/service_manager.hpp | 114 ++++++++++++++++++++++++++++++ llarp/win32/service_manager.cpp | 83 ++++++++++++++++++++++ llarp/win32/service_manager.hpp | 24 +++++++ 13 files changed, 335 insertions(+), 98 deletions(-) create mode 100644 llarp/android/service_manager.cpp create mode 100644 llarp/apple/service_manager.cpp create mode 100644 llarp/linux/service_manager.cpp create mode 100644 llarp/util/service_manager.hpp create mode 100644 llarp/win32/service_manager.cpp create mode 100644 llarp/win32/service_manager.hpp diff --git a/daemon/lokinet.cpp b/daemon/lokinet.cpp index 391495509c..346df607c7 100644 --- a/daemon/lokinet.cpp +++ b/daemon/lokinet.cpp @@ -7,7 +7,10 @@ #include #ifdef _WIN32 +#include #include +#else +#include #endif #include @@ -21,19 +24,18 @@ int lokinet_main(int, char**); #ifdef _WIN32 -#include 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 +bool run_as_daemon{false}; + static auto logcat = llarp::log::Cat("main"); std::shared_ptr ctx; std::promise exit_code; @@ -84,9 +86,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( @@ -294,37 +293,6 @@ run_main_context(std::optional 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 @@ -370,9 +338,9 @@ main(int argc, char* argv[]) #else SERVICE_TABLE_ENTRY DispatchTable[] = { {strdup("lokinet"), (LPSERVICE_MAIN_FUNCTION)win32_daemon_entry}, {NULL, NULL}}; - if (lstrcmpi(argv[1], "--win32-daemon") == 0) + if (std::string{argv[1]} == "--win32-daemon") { - start_as_daemon = true; + run_as_daemon = true; StartServiceCtrlDispatcher(DispatchTable); } else @@ -383,6 +351,10 @@ main(int argc, char* argv[]) int lokinet_main(int argc, char** argv) { + // if we are not running as a service disable reporting + if (llarp::platform::is_windows and not run_as_daemon) + llarp::sys::service_manager->disable(); + if (auto result = Lokinet_INIT()) return result; @@ -398,7 +370,6 @@ lokinet_main(int argc, char** argv) opts.showBanner = false; #ifdef _WIN32 - WindowsServiceStopped stopped_raii; if (startWinsock()) return -1; SetConsoleCtrlHandler(handle_signal_win32, TRUE); @@ -545,13 +516,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 @@ -582,9 +549,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); @@ -609,6 +574,7 @@ lokinet_main(int argc, char** argv) } llarp::log::flush(); + llarp::sys::service_manager->stopped(); if (ctx) { ctx.reset(); @@ -617,29 +583,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) @@ -651,14 +594,12 @@ SvcCtrlHandler(DWORD dwCtrl) case SERVICE_CONTROL_STOP: // tell service we are stopping llarp::log::info(logcat, "Windows service controller gave SERVICE_CONTROL_STOP"); - ReportSvcStatus(SERVICE_STOP_PENDING, NO_ERROR, 0); - // do the actual tear down - handle_signal(SIGINT); + llarp::sys::service_manager->system_changed_our_state(llarp::sys::ServiceState::Stopping); return; case SERVICE_CONTROL_INTERROGATE: // report status - SetServiceStatus(SvcStatusHandle, &SvcStatus); + llarp::sys::service_manager->report_our_state(); return; default: @@ -673,21 +614,15 @@ VOID FAR PASCAL win32_daemon_entry(DWORD, LPTSTR* argv) { // Register the handler function for the service - SvcStatusHandle = RegisterServiceCtrlHandler("lokinet", SvcCtrlHandler); + auto* svc = dynamic_cast(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); - // we hard code the args to lokinet_main. // we yoink argv[0] (lokinet.exe path) and pass in the new args. std::array args = { diff --git a/include/llarp.hpp b/include/llarp.hpp index 402ffa52e5..cb8ca495bf 100644 --- a/include/llarp.hpp +++ b/include/llarp.hpp @@ -45,6 +45,7 @@ namespace llarp std::shared_ptr nodedb = nullptr; std::string nodedb_dir; + Context(); virtual ~Context() = default; void diff --git a/llarp/CMakeLists.txt b/llarp/CMakeLists.txt index 2e3c558dfa..15bb865add 100644 --- a/llarp/CMakeLists.txt +++ b/llarp/CMakeLists.txt @@ -49,17 +49,18 @@ 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 android/service_manager.cpp) endif() if(CMAKE_SYSTEM_NAME MATCHES "Linux") - target_sources(lokinet-platform PRIVATE linux/dbus.cpp) + target_sources(lokinet-platform PRIVATE linux/dbus.cpp linux/service_manager.cpp) 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 diff --git a/llarp/android/service_manager.cpp b/llarp/android/service_manager.cpp new file mode 100644 index 0000000000..694dfadf79 --- /dev/null +++ b/llarp/android/service_manager.cpp @@ -0,0 +1,8 @@ +#include + +namespace llarp::sys +{ + // we will have this implemented on android when there is time to + NOP_SystemLayerHandler _manager{}; + I_SystemLayerManager* const service_manager = &_manager; +} // namespace llarp::sys diff --git a/llarp/apple/CMakeLists.txt b/llarp/apple/CMakeLists.txt index 8dd561ef75..3b12e17626 100644 --- a/llarp/apple/CMakeLists.txt +++ b/llarp/apple/CMakeLists.txt @@ -13,7 +13,7 @@ find_library(COREFOUNDATION CoreFoundation REQUIRED) target_link_libraries(lokinet-util PUBLIC ${FOUNDATION}) -target_sources(lokinet-platform PRIVATE vpn_platform.cpp vpn_interface.cpp route_manager.cpp context_wrapper.cpp) +target_sources(lokinet-platform PRIVATE vpn_platform.cpp vpn_interface.cpp route_manager.cpp context_wrapper.cpp service_manager.cpp) add_executable(lokinet-extension MACOSX_BUNDLE PacketTunnelProvider.m diff --git a/llarp/apple/service_manager.cpp b/llarp/apple/service_manager.cpp new file mode 100644 index 0000000000..bf79071b72 --- /dev/null +++ b/llarp/apple/service_manager.cpp @@ -0,0 +1,7 @@ +#include + +namespace llarp::sys +{ + NOP_SystemLayerHandler _manager{}; + I_SystemLayerManager* const service_manager = &_manager; +} // namespace llarp::sys diff --git a/llarp/context.cpp b/llarp/context.cpp index 9e818e5891..1901afc6e0 100644 --- a/llarp/context.cpp +++ b/llarp/context.cpp @@ -12,6 +12,8 @@ #include "service/context.hpp" #include "util/logging.hpp" +#include + #include #include #include @@ -213,4 +215,10 @@ namespace llarp loop.reset(); } + Context::Context() + { + // service_manager is a global and context isnt + llarp::sys::service_manager->give_context(this); + } + } // namespace llarp diff --git a/llarp/linux/service_manager.cpp b/llarp/linux/service_manager.cpp new file mode 100644 index 0000000000..b395dae133 --- /dev/null +++ b/llarp/linux/service_manager.cpp @@ -0,0 +1,59 @@ +#include + +#if defined(WITH_SYSTEMD) +#include +#include + +namespace llarp::sys +{ + class SD_Manager : public I_SystemLayerManager + { + llarp::sys::ServiceState m_State{ServiceState::Initial}; + + public: + /// change our state and report it to the system layer + void + we_changed_our_state(ServiceState st) override + { + assert(m_State != st); + m_State = st; + report_our_state(); + } + + /// report our current state to the system layer + void + report_our_state() override + { + if (m_State == ServiceState::Starting) + { + // TODO: maybe request more time? + ::sd_notify(0, "READY=0"); + return; + } + if (m_State == ServiceState::Running) + { + ::sd_notify(0, "READY=1"); + return; + } + if (m_State == ServiceState::Stopping) + { + ::sd_notify(0, "STOPPING=1"); + return; + } + } + + void + system_changed_our_state(ServiceState) override + { + // not applicable on systemd + } + }; + + SD_Manager _manager{}; +#else +NOP_SystemLayerHandler _manager{}; +#endif + + I_SystemLayerManager* const service_manager = &_manager; + +} // namespace llarp::sys diff --git a/llarp/router/router.cpp b/llarp/router/router.cpp index d0edc5cfae..09d0dd6375 100644 --- a/llarp/router/router.cpp +++ b/llarp/router/router.cpp @@ -393,6 +393,8 @@ namespace llarp bool Router::Configure(std::shared_ptr c, bool isSNode, std::shared_ptr nodedb) { + llarp::sys::service_manager->starting(); + m_Config = std::move(c); auto& conf = *m_Config; @@ -1399,9 +1401,6 @@ namespace llarp m_RoutePoker->Start(this); _running.store(true); _startedAt = Now(); -#if defined(WITH_SYSTEMD) - ::sd_notify(0, "READY=1"); -#endif if (whitelistRouters) { // do service node testing if we are in service node whitelist mode @@ -1474,6 +1473,7 @@ namespace llarp } }); } + llarp::sys::service_manager->ready(); return _running; } @@ -1525,9 +1525,7 @@ namespace llarp if (log::get_level_default() != log::Level::off) log::reset_level(log::Level::info); LogWarn("stopping router hard"); -#if defined(WITH_SYSTEMD) - sd_notify(0, "STOPPING=1\nSTATUS=Shutting down HARD"); -#endif + llarp::sys::service_manager->stopping(); hiddenServiceContext().StopAll(); _exitContext.Stop(); StopLinks(); @@ -1546,9 +1544,7 @@ namespace llarp if (log::get_level_default() != log::Level::off) log::reset_level(log::Level::info); LogInfo("stopping router"); -#if defined(WITH_SYSTEMD) - sd_notify(0, "STOPPING=1\nSTATUS=Shutting down"); -#endif + llarp::sys::service_manager->stopping(); hiddenServiceContext().StopAll(); _exitContext.Stop(); paths.PumpUpstream(); diff --git a/llarp/router/router.hpp b/llarp/router/router.hpp index 53b24e4aab..0556013949 100644 --- a/llarp/router/router.hpp +++ b/llarp/router/router.hpp @@ -35,6 +35,7 @@ #include #include #include +#include #include #include diff --git a/llarp/util/service_manager.hpp b/llarp/util/service_manager.hpp new file mode 100644 index 0000000000..8531b2cc0e --- /dev/null +++ b/llarp/util/service_manager.hpp @@ -0,0 +1,114 @@ +#pragma once + +namespace llarp +{ + struct Context; +} + +namespace llarp::sys +{ + + // what state lokinet will report we are in to the system layer + enum class ServiceState + { + Initial, + Starting, + Running, + Stopping, + Stopped, + HardStop, + Failed, + }; + + /// interface type for interacting with the os dependant system layer + class I_SystemLayerManager + { + protected: + bool m_disable{false}; + llarp::Context* m_Context{nullptr}; + + /// change our state and report it to the system layer + virtual void + we_changed_our_state(ServiceState st) = 0; + + public: + virtual ~I_SystemLayerManager() = default; + + /// disable all reporting to system layer + inline void + disable() + { + m_disable = true; + } + + /// give our current lokinet context to the system layer manager + inline void + give_context(llarp::Context* ctx) + { + m_Context = ctx; + } + + /// system told us to enter this state + virtual void + system_changed_our_state(ServiceState st) = 0; + + /// report our current state to the system layer + virtual void + report_our_state() = 0; + + void + starting() + { + if (m_disable) + return; + we_changed_our_state(ServiceState::Starting); + } + + void + ready() + { + if (m_disable) + return; + we_changed_our_state(ServiceState::Running); + } + + void + stopping() + { + if (m_disable) + return; + we_changed_our_state(ServiceState::Stopping); + } + + void + stopped() + { + if (m_disable) + return; + we_changed_our_state(ServiceState::Stopped); + } + + void + failed() + { + if (m_disable) + return; + we_changed_our_state(ServiceState::Failed); + } + }; + + extern I_SystemLayerManager* const service_manager; + + class NOP_SystemLayerHandler : public I_SystemLayerManager + { + protected: + void + we_changed_our_state(ServiceState) override + {} + + public: + void + report_our_state() override{}; + void system_changed_our_state(ServiceState) override{}; + }; +} // namespace llarp::sys diff --git a/llarp/win32/service_manager.cpp b/llarp/win32/service_manager.cpp new file mode 100644 index 0000000000..a2ced51cbc --- /dev/null +++ b/llarp/win32/service_manager.cpp @@ -0,0 +1,83 @@ +#include +#include +#include "service_manager.hpp" +#include +#include +#include +#include + +namespace llarp::sys +{ + + namespace + { + + std::optional + to_win32_state(ServiceState st) + { + switch (st) + { + case ServiceState::Starting: + return SERVICE_START_PENDING; + case ServiceState::Running: + return SERVICE_RUNNING; + case ServiceState::Stopping: + return SERVICE_STOP_PENDING; + case ServiceState::Stopped: + return SERVICE_STOPPED; + default: + return std::nullopt; + } + } + } // namespace + + SVC_Manager::SVC_Manager() + { + _status.dwServiceType = SERVICE_WIN32_OWN_PROCESS; + } + + void + SVC_Manager::system_changed_our_state(ServiceState st) + { + if (m_disable) + return; + if (st == ServiceState::Stopping) + { + we_changed_our_state(st); + m_Context->HandleSignal(SIGINT); + } + } + + void + SVC_Manager::report_our_state() + { + if (m_disable) + return; + SetServiceStatus(handle, &_status); + } + + void + SVC_Manager::we_changed_our_state(ServiceState st) + { + if (st == ServiceState::Failed) + { + _status.dwWin32ExitCode = 2; // TODO: propagate more info ? + report_our_state(); + } + else if (auto maybe_state = to_win32_state(st)) + { + auto new_state = *maybe_state; + assert(_status.dwCurrentState != new_state); + _status.dwCurrentState = new_state; + // tell windows it takes 5s at most to start or stop + if (st == ServiceState::Starting or st == ServiceState::Stopping) + _status.dwWaitHint = 5000; + else // other state changes are a half second + _status.dwWaitHint = 500; + report_our_state(); + } + } + + SVC_Manager _manager{}; + I_SystemLayerManager* const service_manager = &_manager; +} // namespace llarp::sys diff --git a/llarp/win32/service_manager.hpp b/llarp/win32/service_manager.hpp new file mode 100644 index 0000000000..71c782cab5 --- /dev/null +++ b/llarp/win32/service_manager.hpp @@ -0,0 +1,24 @@ +#pragma once +#include +namespace llarp::sys +{ + + class SVC_Manager : public I_SystemLayerManager + { + SERVICE_STATUS _status; + + public: + SERVICE_STATUS_HANDLE handle; + + SVC_Manager(); + + void + system_changed_our_state(ServiceState st) override; + + void + report_our_state() override; + + void + we_changed_our_state(ServiceState st) override; + }; +} // namespace llarp::sys From cbb57fba97c5b3ccbb83981bcf960dea69bb4503 Mon Sep 17 00:00:00 2001 From: Jeff Becker Date: Thu, 27 Oct 2022 14:17:37 -0400 Subject: [PATCH 07/46] fix ifdef guards --- llarp/linux/service_manager.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/llarp/linux/service_manager.cpp b/llarp/linux/service_manager.cpp index b395dae133..4964f40d3a 100644 --- a/llarp/linux/service_manager.cpp +++ b/llarp/linux/service_manager.cpp @@ -3,9 +3,11 @@ #if defined(WITH_SYSTEMD) #include #include +#endif namespace llarp::sys { +#if defined(WITH_SYSTEMD) class SD_Manager : public I_SystemLayerManager { llarp::sys::ServiceState m_State{ServiceState::Initial}; @@ -51,7 +53,7 @@ namespace llarp::sys SD_Manager _manager{}; #else -NOP_SystemLayerHandler _manager{}; + NOP_SystemLayerHandler _manager{}; #endif I_SystemLayerManager* const service_manager = &_manager; From 35fe9b38495bb2ac475579e72a2a55eaf2adad1e Mon Sep 17 00:00:00 2001 From: Jeff Becker Date: Thu, 27 Oct 2022 14:52:39 -0400 Subject: [PATCH 08/46] turn off service manager in unit tests --- test/check_main.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/check_main.cpp b/test/check_main.cpp index e98bdcbfb6..87299c8339 100644 --- a/test/check_main.cpp +++ b/test/check_main.cpp @@ -2,6 +2,7 @@ #include #include +#include #ifdef _WIN32 #include @@ -23,6 +24,7 @@ startWinsock() int main(int argc, char* argv[]) { + llarp::sys::service_manager->disable(); llarp::log::reset_level(llarp::log::Level::off); #ifdef _WIN32 From f4974a886062dd660dcbee5f4c32973583e06b8a Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Thu, 27 Oct 2022 19:01:56 -0300 Subject: [PATCH 09/46] Remove dead/redundant code - win32_platform.cpp is dead - win32_platform.hpp is useless Style changes from clang-tidy warnings: - remove `virtual` from some definitions that already have `override` - remove virtual destructor from NetworkInterface because it already has a virtual destructor via the base type (and clang-tiny warns about it) --- daemon/lokinet.cpp | 3 +-- llarp/dns/server.cpp | 5 ---- llarp/dns/win32_platform.cpp | 51 ------------------------------------ llarp/dns/win32_platform.hpp | 8 ------ llarp/vpn/platform.hpp | 2 -- llarp/win32/windivert.cpp | 8 +++--- llarp/win32/wintun.cpp | 4 +-- 7 files changed, 6 insertions(+), 75 deletions(-) delete mode 100644 llarp/dns/win32_platform.cpp delete mode 100644 llarp/dns/win32_platform.hpp diff --git a/daemon/lokinet.cpp b/daemon/lokinet.cpp index 346df607c7..08eae6d44c 100644 --- a/daemon/lokinet.cpp +++ b/daemon/lokinet.cpp @@ -373,9 +373,8 @@ lokinet_main(int argc, char** argv) 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, " diff --git a/llarp/dns/server.cpp b/llarp/dns/server.cpp index 49e4fef146..4ef4ce919c 100644 --- a/llarp/dns/server.cpp +++ b/llarp/dns/server.cpp @@ -16,7 +16,6 @@ #include "oxen/log.hpp" #include "sd_platform.hpp" #include "nm_platform.hpp" -#include "win32_platform.hpp" namespace llarp::dns { @@ -570,10 +569,6 @@ namespace llarp::dns plat->add_impl(std::make_unique()); plat->add_impl(std::make_unique()); } - if constexpr (llarp::platform::is_windows) - { - plat->add_impl(std::make_unique()); - } return plat; } diff --git a/llarp/dns/win32_platform.cpp b/llarp/dns/win32_platform.cpp deleted file mode 100644 index 6c268b05cd..0000000000 --- a/llarp/dns/win32_platform.cpp +++ /dev/null @@ -1,51 +0,0 @@ -#include "win32_platform.hpp" -#include - -namespace llarp::dns::win32 -{ - void - Platform::set_resolver(unsigned int index, llarp::SockAddr dns, bool) - { -#ifdef _WIN32 - - // clear any previous dns settings - m_UndoDNS.clear(); - - auto interfaces = m_Loop->Net_ptr()->AllNetworkInterfaces(); - // remove dns - { - std::vector jobs; - for (const auto& ent : interfaces) - { - if (ent.index == index) - continue; - jobs.emplace_back( - "netsh.exe", fmt::format("interface ipv4 delete dns \"{}\" all", ent.name)); - jobs.emplace_back( - "netsh.exe", fmt::format("interface ipv6 delete dns \"{}\" all", ent.name)); - } - } - // add new dns - { - std::vector jobs; - for (const auto& ent : interfaces) - { - if (ent.index == index) - continue; - jobs.emplace_back( - "netsh.exe", - fmt::format("interface ipv4 add dns \"{}\" {} validate=no", ent.name, dns.asIPv4())); - jobs.emplace_back( - "netsh.exe", - fmt::format("interface ipv6 add dns \"{}\" {} validate=no", ent.name, dns.asIPv6())); - m_UndoDNS.emplace_back("netsh.exe", fmt::format("", index)); - } - m_UndoDNS.emplace_back("netsh.exe", "winsock reset"); - } - // flush dns - llarp::win32::Exec("ipconfig.exe", "/flushdns"); - -#endif - } - -} // namespace llarp::dns::win32 diff --git a/llarp/dns/win32_platform.hpp b/llarp/dns/win32_platform.hpp deleted file mode 100644 index cb57a206c2..0000000000 --- a/llarp/dns/win32_platform.hpp +++ /dev/null @@ -1,8 +0,0 @@ -#pragma once -#include "platform.hpp" - -namespace llarp::dns -{ - // TODO: implement me - using Win32_Platform_t = Null_Platform; -} // namespace llarp::dns diff --git a/llarp/vpn/platform.hpp b/llarp/vpn/platform.hpp index ebba6dee8c..9ae880bb9a 100644 --- a/llarp/vpn/platform.hpp +++ b/llarp/vpn/platform.hpp @@ -59,8 +59,6 @@ namespace llarp::vpn NetworkInterface(const NetworkInterface&) = delete; NetworkInterface(NetworkInterface&&) = delete; - virtual ~NetworkInterface() = default; - const InterfaceInfo& Info() const { diff --git a/llarp/win32/windivert.cpp b/llarp/win32/windivert.cpp index 7dc181e912..739e15638a 100644 --- a/llarp/win32/windivert.cpp +++ b/llarp/win32/windivert.cpp @@ -125,13 +125,13 @@ namespace llarp::win32 return -1; } - virtual bool + bool WritePacket(net::IPPacket) override { return false; } - virtual net::IPPacket + net::IPPacket ReadNextPacket() override { auto w_pkt = m_RecvQueue.tryPopFront(); @@ -144,7 +144,7 @@ namespace llarp::win32 return pkt; } - virtual void + void Start() override { L::info(cat, "starting windivert"); @@ -172,7 +172,7 @@ namespace llarp::win32 m_Runner = std::thread{std::move(read_loop)}; } - virtual void + void Stop() override { L::info(cat, "stopping windivert"); diff --git a/llarp/win32/wintun.cpp b/llarp/win32/wintun.cpp index b6fb30ba0d..0a48b32072 100644 --- a/llarp/win32/wintun.cpp +++ b/llarp/win32/wintun.cpp @@ -233,11 +233,9 @@ namespace llarp::win32 [[nodiscard]] std::pair, bool> ReadPacket() const { - // typedef so the return statement fits on 1 line :^D - using Pkt_ptr = std::unique_ptr; DWORD sz; if (auto* ptr = read_packet(_impl, &sz)) - return {Pkt_ptr{new PacketWrapper{ptr, sz, _impl}}, false}; + return {std::unique_ptr{new PacketWrapper{ptr, sz, _impl}}, false}; const auto err = GetLastError(); if (err == ERROR_NO_MORE_ITEMS or err == ERROR_HANDLE_EOF) { From f0c2a9ccbaa93e6a5a609113fb6f0c90b9121076 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Thu, 27 Oct 2022 19:11:11 -0300 Subject: [PATCH 10/46] 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 to early return. - fixes up some settings we send for windows service manager notify --- daemon/lokinet.cpp | 4 +++- llarp/handlers/tun.cpp | 2 ++ llarp/service/context.cpp | 1 + llarp/service/endpoint.cpp | 3 +++ llarp/win32/service_manager.cpp | 33 +++++++++++++++++++++++++++--- llarp/win32/windivert.cpp | 36 +++++++++++++++++---------------- llarp/win32/wintun.cpp | 7 ++++++- 7 files changed, 64 insertions(+), 22 deletions(-) diff --git a/daemon/lokinet.cpp b/daemon/lokinet.cpp index 08eae6d44c..1e7a9ff15b 100644 --- a/daemon/lokinet.cpp +++ b/daemon/lokinet.cpp @@ -592,16 +592,18 @@ 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::sys::service_manager->report_our_state(); return; default: + llarp::log::debug(logcat, "Got win32 unhandled signal {}", dwCtrl); break; } } diff --git a/llarp/handlers/tun.cpp b/llarp/handlers/tun.cpp index 89a27437b7..e04e44ea72 100644 --- a/llarp/handlers/tun.cpp +++ b/llarp/handlers/tun.cpp @@ -35,6 +35,8 @@ namespace llarp { namespace handlers { + static auto logcat = log::Cat("tun"); + bool TunEndpoint::MaybeHookDNS( std::shared_ptr source, diff --git a/llarp/service/context.cpp b/llarp/service/context.cpp index 60d23a916e..487e69f662 100644 --- a/llarp/service/context.cpp +++ b/llarp/service/context.cpp @@ -11,6 +11,7 @@ namespace llarp { namespace service { + static auto logcat = log::Cat("service"); namespace { using EndpointConstructor = diff --git a/llarp/service/endpoint.cpp b/llarp/service/endpoint.cpp index 48c33543ac..ee7418fc15 100644 --- a/llarp/service/endpoint.cpp +++ b/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} diff --git a/llarp/win32/service_manager.cpp b/llarp/win32/service_manager.cpp index a2ced51cbc..b193944fdb 100644 --- a/llarp/win32/service_manager.cpp +++ b/llarp/win32/service_manager.cpp @@ -1,5 +1,6 @@ #include #include +#include #include "service_manager.hpp" #include #include @@ -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); } @@ -61,19 +79,28 @@ namespace llarp::sys { if (st == ServiceState::Failed) { - _status.dwWin32ExitCode = 2; // TODO: propagate more info ? + _status.dwWin32ExitCode = ERROR_SERVICE_SPECIFIC_ERROR; + _status.dwServiceSpecificExitCode = 2; // TODO: propagate more info ? report_our_state(); } else if (auto maybe_state = to_win32_state(st)) { 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.dwWaitHint = 5000; - else // other state changes are a half second - _status.dwWaitHint = 500; + _status.dwCheckPoint++; + } + else + { + _status.dwWaitHint = 0; + _status.dwCheckPoint = 0; + } report_our_state(); } } diff --git a/llarp/win32/windivert.cpp b/llarp/win32/windivert.cpp index 739e15638a..02a799bc7a 100644 --- a/llarp/win32/windivert.cpp +++ b/llarp/win32/windivert.cpp @@ -11,14 +11,11 @@ extern "C" { #include } -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(); } diff --git a/llarp/win32/wintun.cpp b/llarp/win32/wintun.cpp index 0a48b32072..ca77c61fa4 100644 --- a/llarp/win32/wintun.cpp +++ b/llarp/win32/wintun.cpp @@ -199,6 +199,8 @@ namespace llarp::win32 { WINTUN_SESSION_HANDLE _impl; HANDLE _handle; + std::atomic ended{false}; + static_assert(std::atomic::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, bool> ReadPacket() const { + if (ended) + return {nullptr, true}; DWORD sz; if (auto* ptr = read_packet(_impl, &sz)) return {std::unique_ptr{new PacketWrapper{ptr, sz, _impl}}, false}; From 100ff1dc30dea8f706721e2177c272a3fa90107c Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Thu, 27 Oct 2022 22:40:38 -0300 Subject: [PATCH 11/46] Add some more debugging --- llarp/router/router.cpp | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/llarp/router/router.cpp b/llarp/router/router.cpp index 09d0dd6375..3afd0bd6e6 100644 --- a/llarp/router/router.cpp +++ b/llarp/router/router.cpp @@ -517,9 +517,10 @@ namespace llarp void Router::Close() { + log::info(logcat, "closing"); if (_onDown) _onDown(); - LogInfo("closing router"); + log::debug(logcat, "stopping mainloop"); _loop->stop(); _running.store(false); } @@ -1496,13 +1497,16 @@ namespace llarp Router::AfterStopLinks() { Close(); + log::debug(logcat, "stopping oxenmq"); m_lmq.reset(); } void Router::AfterStopIssued() { + log::debug(logcat, "stopping links"); StopLinks(); + log::debug(logcat, "saving nodedb to disk"); nodedb()->SaveToDisk(); _loop->call_later(200ms, [this] { AfterStopLinks(); }); } @@ -1536,18 +1540,28 @@ namespace llarp Router::Stop() { if (!_running) + { + log::debug(logcat, "Stop called, but not running"); return; + } if (_stopping) + { + log::debug(logcat, "Stop called, but already stopping"); return; + } _stopping.store(true); if (log::get_level_default() != log::Level::off) log::reset_level(log::Level::info); - LogInfo("stopping router"); + log::info(logcat, "stopping"); llarp::sys::service_manager->stopping(); + log::debug(logcat, "stopping hidden service context"); hiddenServiceContext().StopAll(); + log::debug(logcat, "stopping exit context"); _exitContext.Stop(); + log::debug(logcat, "final upstream pump"); paths.PumpUpstream(); + log::debug(logcat, "final links pump"); _linkManager.PumpLinks(); _loop->call_later(200ms, [this] { AfterStopIssued(); }); } From 3f2311887ffb4082d4891da7332b26c6dae4ce6a Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Thu, 27 Oct 2022 22:42:15 -0300 Subject: [PATCH 12/46] =?UTF-8?q?=F0=9F=A4=B7=20Work=20around=20ub=5Fctx?= =?UTF-8?q?=5Fdelete=20crash?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When unbound is in threaded async mode on Windows it crashes in ub_ctx_delete during shutdown for unknown reasons, but only when stopped via windows service control. Switch it to process async mode which doesn't crash. --- llarp/dns/server.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/llarp/dns/server.cpp b/llarp/dns/server.cpp index 4ef4ce919c..bd5d93c367 100644 --- a/llarp/dns/server.cpp +++ b/llarp/dns/server.cpp @@ -370,8 +370,15 @@ namespace llarp::dns ConfigureUpstream(conf); - // set async +#ifdef _WIN32 + // threaded async currently crashes on ub_ctx_delete so use process async mode instead on + // Windows: + ub_ctx_async(m_ctx, 0); +#else + // set up threaded async mode: ub_ctx_async(m_ctx, 1); +#endif + // setup mainloop #ifdef _WIN32 running = true; From c7aeb3760dd6de2665706655a67520d32f09147d Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Thu, 27 Oct 2022 22:45:27 -0300 Subject: [PATCH 13/46] Update gui to latest --- gui | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gui b/gui index 7b0f1aacdf..a66130d8d6 160000 --- a/gui +++ b/gui @@ -1 +1 @@ -Subproject commit 7b0f1aacdf79b558adfc39dc9cccb7e348aeec03 +Subproject commit a66130d8d6d246737265d19249c3456575c0f1f1 From 7bc722c151b7ca96f42a8f0e52a8c4e9a226fea0 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Thu, 27 Oct 2022 22:57:21 -0300 Subject: [PATCH 14/46] Add networkReady to stats result --- llarp/router/router.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/llarp/router/router.cpp b/llarp/router/router.cpp index 3afd0bd6e6..c918ea2d67 100644 --- a/llarp/router/router.cpp +++ b/llarp/router/router.cpp @@ -200,6 +200,7 @@ namespace llarp { stats["authCodes"] = services["default"]["authCodes"]; stats["exitMap"] = services["default"]["exitMap"]; + stats["networkReady"] = services["default"]["networkReady"]; stats["lokiAddress"] = services["default"]["identity"]; } return stats; From b993ee5362867af2ef66772ec24e0b30aef54e8e Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Fri, 28 Oct 2022 11:50:50 -0300 Subject: [PATCH 15/46] Extend windows startup timeout If wintun fails it seems to take about 15s, so extend the startup timeout so that it can fail gracefully (and let us clean up before exiting). Also refactors the timeouts to chrono constants. --- llarp/win32/service_manager.cpp | 18 +++++++++++------- llarp/win32/service_manager.hpp | 11 +++++++++++ 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/llarp/win32/service_manager.cpp b/llarp/win32/service_manager.cpp index b193944fdb..6fb66dca06 100644 --- a/llarp/win32/service_manager.cpp +++ b/llarp/win32/service_manager.cpp @@ -1,4 +1,5 @@ #include +#include #include #include #include "service_manager.hpp" @@ -90,17 +91,20 @@ namespace llarp::sys _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 + _status.dwWaitHint = + std::chrono::milliseconds{ + st == ServiceState::Starting ? StartupTimeout + : st == ServiceState::Stopping ? StopTimeout + : 0s} + .count(); + // dwCheckPoint gets incremented during a start/stop to tell windows "we're still + // starting/stopping" and to reset its must-be-hung timer. We increment it here so that this + // can be called multiple times to tells Windows something is happening. if (st == ServiceState::Starting or st == ServiceState::Stopping) - { - _status.dwWaitHint = 5000; _status.dwCheckPoint++; - } else - { - _status.dwWaitHint = 0; _status.dwCheckPoint = 0; - } + report_our_state(); } } diff --git a/llarp/win32/service_manager.hpp b/llarp/win32/service_manager.hpp index 71c782cab5..fdb59a1b88 100644 --- a/llarp/win32/service_manager.hpp +++ b/llarp/win32/service_manager.hpp @@ -1,5 +1,8 @@ #pragma once +#include #include +#include + namespace llarp::sys { @@ -10,6 +13,14 @@ namespace llarp::sys public: SERVICE_STATUS_HANDLE handle; + // How long we tell Windows to give us to startup before assuming we have stalled/hung. The + // biggest potential time here is wintun, which if it is going to fail appears to take around + // 15s before doing so. + static constexpr auto StartupTimeout = 17s; + + // How long we tell Windows to give us to fully stop before killing us. + static constexpr auto StopTimeout = 5s; + SVC_Manager(); void From 0b2a4787c836edbafd106a13fb3ca687be5ca8e3 Mon Sep 17 00:00:00 2001 From: Jeff Becker Date: Fri, 28 Oct 2022 11:31:25 -0400 Subject: [PATCH 16/46] fixups * move nop service manager to its own compilation unit and remove old compilation units (touches cmake) * change service manager interface to also be able to report stats * fixup service manager interface's method names * move sd_notify to service manager --- daemon/lokinet.cpp | 3 +- llarp/CMakeLists.txt | 10 +- llarp/android/service_manager.cpp | 8 -- llarp/apple/CMakeLists.txt | 2 +- llarp/linux/sd_service_manager.cpp | 60 ++++++++++ llarp/linux/service_manager.cpp | 18 +-- llarp/router/abstractrouter.hpp | 3 + llarp/router/router.cpp | 106 +++++++++--------- llarp/router/router.hpp | 3 + .../nop_service_manager.cpp} | 2 +- llarp/util/service_manager.hpp | 8 +- llarp/win32/service_manager.cpp | 9 +- llarp/win32/service_manager.hpp | 2 +- 13 files changed, 148 insertions(+), 86 deletions(-) delete mode 100644 llarp/android/service_manager.cpp create mode 100644 llarp/linux/sd_service_manager.cpp rename llarp/{apple/service_manager.cpp => util/nop_service_manager.cpp} (77%) diff --git a/daemon/lokinet.cpp b/daemon/lokinet.cpp index 1e7a9ff15b..8f045f0c76 100644 --- a/daemon/lokinet.cpp +++ b/daemon/lokinet.cpp @@ -598,8 +598,7 @@ SvcCtrlHandler(DWORD dwCtrl) case SERVICE_CONTROL_INTERROGATE: llarp::log::debug(logcat, "Got win32 service interrogate signal"); - // report status - llarp::sys::service_manager->report_our_state(); + llarp::sys::service_manager->report_changed_state(); return; default: diff --git a/llarp/CMakeLists.txt b/llarp/CMakeLists.txt index 15bb865add..77542c1058 100644 --- a/llarp/CMakeLists.txt +++ b/llarp/CMakeLists.txt @@ -49,11 +49,16 @@ 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 android/service_manager.cpp) + 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 linux/service_manager.cpp) + 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) @@ -313,6 +318,7 @@ endif() if(APPLE) add_subdirectory(apple) + target_sources(lokinet-platform PRIVATE util/nop_system_manager.cpp) endif() file(GLOB_RECURSE docs_SRC */*.hpp *.hpp) diff --git a/llarp/android/service_manager.cpp b/llarp/android/service_manager.cpp deleted file mode 100644 index 694dfadf79..0000000000 --- a/llarp/android/service_manager.cpp +++ /dev/null @@ -1,8 +0,0 @@ -#include - -namespace llarp::sys -{ - // we will have this implemented on android when there is time to - NOP_SystemLayerHandler _manager{}; - I_SystemLayerManager* const service_manager = &_manager; -} // namespace llarp::sys diff --git a/llarp/apple/CMakeLists.txt b/llarp/apple/CMakeLists.txt index 3b12e17626..8dd561ef75 100644 --- a/llarp/apple/CMakeLists.txt +++ b/llarp/apple/CMakeLists.txt @@ -13,7 +13,7 @@ find_library(COREFOUNDATION CoreFoundation REQUIRED) target_link_libraries(lokinet-util PUBLIC ${FOUNDATION}) -target_sources(lokinet-platform PRIVATE vpn_platform.cpp vpn_interface.cpp route_manager.cpp context_wrapper.cpp service_manager.cpp) +target_sources(lokinet-platform PRIVATE vpn_platform.cpp vpn_interface.cpp route_manager.cpp context_wrapper.cpp) add_executable(lokinet-extension MACOSX_BUNDLE PacketTunnelProvider.m diff --git a/llarp/linux/sd_service_manager.cpp b/llarp/linux/sd_service_manager.cpp new file mode 100644 index 0000000000..cc260dd8a0 --- /dev/null +++ b/llarp/linux/sd_service_manager.cpp @@ -0,0 +1,60 @@ +#include + +#include +#include +#include +#include +#include + +namespace llarp::sys +{ + class SD_Manager : public I_SystemLayerManager + { + llarp::sys::ServiceState m_State{ServiceState::Initial}; + + public: + /// change our state and report it to the system layer + void + we_changed_our_state(ServiceState st) override + { + assert(m_State != st); + m_State = st; + report_changed_state(); + } + + void + report_changed_state() override + { + if (m_State == ServiceState::Running) + { + ::sd_notify(0, "READY=1"); + return; + } + if (m_State == ServiceState::Stopping) + { + ::sd_notify(0, "STOPPING=1"); + return; + } + } + + void + report_periodic_stats() override + { + if (m_Context and m_Context->router and not m_disable) + { + auto status = m_Context->router->status_line(); + ::sd_notify(0, status.c_str()); + } + } + + void + system_changed_our_state(ServiceState) override + { + // not applicable on systemd + } + }; + + SD_Manager _manager{}; + I_SystemLayerManager* const service_manager = &_manager; + +} // namespace llarp::sys diff --git a/llarp/linux/service_manager.cpp b/llarp/linux/service_manager.cpp index 4964f40d3a..6fa950d7ec 100644 --- a/llarp/linux/service_manager.cpp +++ b/llarp/linux/service_manager.cpp @@ -1,13 +1,10 @@ #include -#if defined(WITH_SYSTEMD) #include #include -#endif namespace llarp::sys { -#if defined(WITH_SYSTEMD) class SD_Manager : public I_SystemLayerManager { llarp::sys::ServiceState m_State{ServiceState::Initial}; @@ -26,12 +23,6 @@ namespace llarp::sys void report_our_state() override { - if (m_State == ServiceState::Starting) - { - // TODO: maybe request more time? - ::sd_notify(0, "READY=0"); - return; - } if (m_State == ServiceState::Running) { ::sd_notify(0, "READY=1"); @@ -51,11 +42,10 @@ namespace llarp::sys } }; - SD_Manager _manager{}; -#else - NOP_SystemLayerHandler _manager{}; -#endif - + SD_Manager _manager + { + ; + } I_SystemLayerManager* const service_manager = &_manager; } // namespace llarp::sys diff --git a/llarp/router/abstractrouter.hpp b/llarp/router/abstractrouter.hpp index 7f6a368a79..162b506167 100644 --- a/llarp/router/abstractrouter.hpp +++ b/llarp/router/abstractrouter.hpp @@ -361,6 +361,9 @@ namespace llarp virtual void GossipRCIfNeeded(const RouterContact rc) = 0; + virtual std::string + status_line() = 0; + /// Templated convenience function to generate a RouterHive event and /// delegate to non-templated (and overridable) function for handling. template diff --git a/llarp/router/router.cpp b/llarp/router/router.cpp index c918ea2d67..466bd942a0 100644 --- a/llarp/router/router.cpp +++ b/llarp/router/router.cpp @@ -874,6 +874,60 @@ namespace llarp m_LastStatsReport = now; } + std::string + Router::status_line() + { + std::string status; +#if defined(WITH_SYSTEMD) + auto out = std::back_inserter(status); + fmt::format_to(out, "WATCHDOG=1\nSTATUS=v{}", llarp::VERSION_STR); + if (IsServiceNode()) + { + fmt::format_to( + out, + " snode | known/svc/clients: {}/{}/{}", + nodedb()->NumLoaded(), + NumberOfConnectedRouters(), + NumberOfConnectedClients()); + fmt::format_to( + out, + " | {} active paths | block {} ", + pathContext().CurrentTransitPaths(), + (m_lokidRpcClient ? m_lokidRpcClient->BlockHeight() : 0)); + auto maybe_last = _rcGossiper.LastGossipAt(); + fmt::format_to( + out, + " | gossip: (next/last) {} / {}", + short_time_from_now(_rcGossiper.NextGossipAt()), + maybe_last ? short_time_from_now(*maybe_last) : "never"); + } + else + { + fmt::format_to( + out, + " client | known/connected: {}/{}", + nodedb()->NumLoaded(), + NumberOfConnectedRouters()); + + if (auto ep = hiddenServiceContext().GetDefault()) + { + fmt::format_to( + out, + " | paths/endpoints {}/{}", + pathContext().CurrentOwnedPaths(), + ep->UniqueEndpoints()); + + if (auto success_rate = ep->CurrentBuildStats().SuccessRatio(); success_rate < 0.5) + { + fmt::format_to( + out, " [ !!! Low Build Success Rate ({:.1f}%) !!! ]", (100.0 * success_rate)); + } + }; + } +#endif + return status; + } + void Router::Tick() { @@ -888,57 +942,7 @@ namespace llarp Thaw(); } -#if defined(WITH_SYSTEMD) - { - std::string status; - auto out = std::back_inserter(status); - fmt::format_to(out, "WATCHDOG=1\nSTATUS=v{}", llarp::VERSION_STR); - if (IsServiceNode()) - { - fmt::format_to( - out, - " snode | known/svc/clients: {}/{}/{}", - nodedb()->NumLoaded(), - NumberOfConnectedRouters(), - NumberOfConnectedClients()); - fmt::format_to( - out, - " | {} active paths | block {} ", - pathContext().CurrentTransitPaths(), - (m_lokidRpcClient ? m_lokidRpcClient->BlockHeight() : 0)); - auto maybe_last = _rcGossiper.LastGossipAt(); - fmt::format_to( - out, - " | gossip: (next/last) {} / {}", - short_time_from_now(_rcGossiper.NextGossipAt()), - maybe_last ? short_time_from_now(*maybe_last) : "never"); - } - else - { - fmt::format_to( - out, - " client | known/connected: {}/{}", - nodedb()->NumLoaded(), - NumberOfConnectedRouters()); - - if (auto ep = hiddenServiceContext().GetDefault()) - { - fmt::format_to( - out, - " | paths/endpoints {}/{}", - pathContext().CurrentOwnedPaths(), - ep->UniqueEndpoints()); - - if (auto success_rate = ep->CurrentBuildStats().SuccessRatio(); success_rate < 0.5) - { - fmt::format_to( - out, " [ !!! Low Build Success Rate ({:.1f}%) !!! ]", (100.0 * success_rate)); - } - }; - } - ::sd_notify(0, status.c_str()); - } -#endif + llarp::sys::service_manager->report_periodic_stats(); m_PathBuildLimiter.Decay(now); diff --git a/llarp/router/router.hpp b/llarp/router/router.hpp index 0556013949..3e86cff07d 100644 --- a/llarp/router/router.hpp +++ b/llarp/router/router.hpp @@ -316,6 +316,9 @@ namespace llarp RCLookupHandler _rcLookupHandler; RCGossiper _rcGossiper; + std::string + status_line() override; + using Clock_t = std::chrono::steady_clock; using TimePoint_t = Clock_t::time_point; diff --git a/llarp/apple/service_manager.cpp b/llarp/util/nop_service_manager.cpp similarity index 77% rename from llarp/apple/service_manager.cpp rename to llarp/util/nop_service_manager.cpp index bf79071b72..e070250878 100644 --- a/llarp/apple/service_manager.cpp +++ b/llarp/util/nop_service_manager.cpp @@ -1,4 +1,4 @@ -#include +#include "service_manager.hpp" namespace llarp::sys { diff --git a/llarp/util/service_manager.hpp b/llarp/util/service_manager.hpp index 8531b2cc0e..d4f66eb229 100644 --- a/llarp/util/service_manager.hpp +++ b/llarp/util/service_manager.hpp @@ -54,7 +54,11 @@ namespace llarp::sys /// report our current state to the system layer virtual void - report_our_state() = 0; + report_changed_state() = 0; + + /// report our stats on each timer tick + virtual void + report_periodic_stats(){}; void starting() @@ -108,7 +112,7 @@ namespace llarp::sys public: void - report_our_state() override{}; + report_changed_state() override{}; void system_changed_our_state(ServiceState) override{}; }; } // namespace llarp::sys diff --git a/llarp/win32/service_manager.cpp b/llarp/win32/service_manager.cpp index 6fb66dca06..ecd1fc3150 100644 --- a/llarp/win32/service_manager.cpp +++ b/llarp/win32/service_manager.cpp @@ -53,7 +53,7 @@ namespace llarp::sys } void - SVC_Manager::report_our_state() + SVC_Manager::report_changed_state() { if (m_disable) return; @@ -82,7 +82,7 @@ namespace llarp::sys { _status.dwWin32ExitCode = ERROR_SERVICE_SPECIFIC_ERROR; _status.dwServiceSpecificExitCode = 2; // TODO: propagate more info ? - report_our_state(); + report_changed_state(); } else if (auto maybe_state = to_win32_state(st)) { @@ -104,9 +104,10 @@ namespace llarp::sys _status.dwCheckPoint++; else _status.dwCheckPoint = 0; - - report_our_state(); + + report_changed_state(); } + } SVC_Manager _manager{}; diff --git a/llarp/win32/service_manager.hpp b/llarp/win32/service_manager.hpp index fdb59a1b88..6896e69e05 100644 --- a/llarp/win32/service_manager.hpp +++ b/llarp/win32/service_manager.hpp @@ -27,7 +27,7 @@ namespace llarp::sys system_changed_our_state(ServiceState st) override; void - report_our_state() override; + report_changed_state() override; void we_changed_our_state(ServiceState st) override; From 20ccd57ccd1acdc182973be29eeba0446deb2afb Mon Sep 17 00:00:00 2001 From: Jeff Becker Date: Fri, 28 Oct 2022 11:37:07 -0400 Subject: [PATCH 17/46] format --- llarp/win32/service_manager.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/llarp/win32/service_manager.cpp b/llarp/win32/service_manager.cpp index ecd1fc3150..95f6818093 100644 --- a/llarp/win32/service_manager.cpp +++ b/llarp/win32/service_manager.cpp @@ -104,10 +104,9 @@ namespace llarp::sys _status.dwCheckPoint++; else _status.dwCheckPoint = 0; - + report_changed_state(); } - } SVC_Manager _manager{}; From 32ef862e13a31e6703c8719589aa9c5aa98dc7c0 Mon Sep 17 00:00:00 2001 From: Jeff Becker Date: Fri, 28 Oct 2022 12:39:57 -0400 Subject: [PATCH 18/46] clean up service manager for systemd * remove old compilation unit * move systemd specific parts of status to service manager * make router status not systemd specific --- llarp/linux/sd_service_manager.cpp | 2 +- llarp/linux/service_manager.cpp | 51 ------------------------------ llarp/router/router.cpp | 4 +-- 3 files changed, 2 insertions(+), 55 deletions(-) delete mode 100644 llarp/linux/service_manager.cpp diff --git a/llarp/linux/sd_service_manager.cpp b/llarp/linux/sd_service_manager.cpp index cc260dd8a0..1ea2e8fbec 100644 --- a/llarp/linux/sd_service_manager.cpp +++ b/llarp/linux/sd_service_manager.cpp @@ -42,7 +42,7 @@ namespace llarp::sys { if (m_Context and m_Context->router and not m_disable) { - auto status = m_Context->router->status_line(); + auto status = fmt::format("WATCHDOG=1\nSTATUS={}", m_Context->router->status_line()); ::sd_notify(0, status.c_str()); } } diff --git a/llarp/linux/service_manager.cpp b/llarp/linux/service_manager.cpp deleted file mode 100644 index 6fa950d7ec..0000000000 --- a/llarp/linux/service_manager.cpp +++ /dev/null @@ -1,51 +0,0 @@ -#include - -#include -#include - -namespace llarp::sys -{ - class SD_Manager : public I_SystemLayerManager - { - llarp::sys::ServiceState m_State{ServiceState::Initial}; - - public: - /// change our state and report it to the system layer - void - we_changed_our_state(ServiceState st) override - { - assert(m_State != st); - m_State = st; - report_our_state(); - } - - /// report our current state to the system layer - void - report_our_state() override - { - if (m_State == ServiceState::Running) - { - ::sd_notify(0, "READY=1"); - return; - } - if (m_State == ServiceState::Stopping) - { - ::sd_notify(0, "STOPPING=1"); - return; - } - } - - void - system_changed_our_state(ServiceState) override - { - // not applicable on systemd - } - }; - - SD_Manager _manager - { - ; - } - I_SystemLayerManager* const service_manager = &_manager; - -} // namespace llarp::sys diff --git a/llarp/router/router.cpp b/llarp/router/router.cpp index 466bd942a0..ba9b8d4bfc 100644 --- a/llarp/router/router.cpp +++ b/llarp/router/router.cpp @@ -878,9 +878,8 @@ namespace llarp Router::status_line() { std::string status; -#if defined(WITH_SYSTEMD) auto out = std::back_inserter(status); - fmt::format_to(out, "WATCHDOG=1\nSTATUS=v{}", llarp::VERSION_STR); + fmt::format_to(out, "v{}", llarp::VERSION_STR); if (IsServiceNode()) { fmt::format_to( @@ -924,7 +923,6 @@ namespace llarp } }; } -#endif return status; } From fbea8bea73e7e52127667c847ae35fb8d2b39093 Mon Sep 17 00:00:00 2001 From: majestrate Date: Fri, 28 Oct 2022 12:53:49 -0400 Subject: [PATCH 19/46] Update llarp/router/router.cpp use `fmt::join` for lokinet version string in status Co-authored-by: Jason Rhinelander --- llarp/router/router.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llarp/router/router.cpp b/llarp/router/router.cpp index ba9b8d4bfc..dd4cdb14da 100644 --- a/llarp/router/router.cpp +++ b/llarp/router/router.cpp @@ -879,7 +879,7 @@ namespace llarp { std::string status; auto out = std::back_inserter(status); - fmt::format_to(out, "v{}", llarp::VERSION_STR); + fmt::format_to(out, "v{}", fmt::join(llarp::VERSION, ".")); if (IsServiceNode()) { fmt::format_to( From cfffd89895fa25766b7fb229e354eb968d82c05a Mon Sep 17 00:00:00 2001 From: Jeff Becker Date: Fri, 28 Oct 2022 13:04:05 -0400 Subject: [PATCH 20/46] simplify logic for disabling service manager on windows --- daemon/lokinet.cpp | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/daemon/lokinet.cpp b/daemon/lokinet.cpp index 8f045f0c76..3bb44cca40 100644 --- a/daemon/lokinet.cpp +++ b/daemon/lokinet.cpp @@ -34,8 +34,6 @@ insert_description(); #endif -bool run_as_daemon{false}; - static auto logcat = llarp::log::Cat("main"); std::shared_ptr ctx; std::promise exit_code; @@ -340,21 +338,19 @@ main(int argc, char* argv[]) {strdup("lokinet"), (LPSERVICE_MAIN_FUNCTION)win32_daemon_entry}, {NULL, NULL}}; if (std::string{argv[1]} == "--win32-daemon") { - run_as_daemon = true; - StartServiceCtrlDispatcher(DispatchTable); + return StartServiceCtrlDispatcher(DispatchTable); } else + { + llarp::sys::service_manager->disable(); return lokinet_main(argc, argv); + } #endif } int lokinet_main(int argc, char** argv) { - // if we are not running as a service disable reporting - if (llarp::platform::is_windows and not run_as_daemon) - llarp::sys::service_manager->disable(); - if (auto result = Lokinet_INIT()) return result; From 5776269c93e6812776720dc1ab3088f48fbdfe86 Mon Sep 17 00:00:00 2001 From: Jeff Becker Date: Fri, 28 Oct 2022 14:17:34 -0400 Subject: [PATCH 21/46] fix typo in filename --- llarp/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llarp/CMakeLists.txt b/llarp/CMakeLists.txt index 77542c1058..ce7eb71429 100644 --- a/llarp/CMakeLists.txt +++ b/llarp/CMakeLists.txt @@ -318,7 +318,7 @@ endif() if(APPLE) add_subdirectory(apple) - target_sources(lokinet-platform PRIVATE util/nop_system_manager.cpp) + target_sources(lokinet-platform PRIVATE util/nop_service_manager.cpp) endif() file(GLOB_RECURSE docs_SRC */*.hpp *.hpp) From 8a517913432df14a9a6c35162872cf3b85c42679 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Fri, 28 Oct 2022 21:45:47 -0300 Subject: [PATCH 22/46] Move log init even earlier --- daemon/lokinet.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/daemon/lokinet.cpp b/daemon/lokinet.cpp index 3bb44cca40..c17ea727c3 100644 --- a/daemon/lokinet.cpp +++ b/daemon/lokinet.cpp @@ -331,6 +331,14 @@ 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(100); + llarp::log::add_sink(llarp::logRingBuffer, llarp::log::DEFAULT_PATTERN_MONO); + #ifndef _WIN32 return lokinet_main(argc, argv); #else @@ -354,14 +362,6 @@ 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(100); - llarp::log::add_sink(llarp::logRingBuffer, llarp::log::DEFAULT_PATTERN_MONO); - llarp::RuntimeOptions opts; opts.showBanner = false; From d62e880b577032806e6ba1880f6cfcf3844db6cc Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Fri, 28 Oct 2022 21:46:06 -0300 Subject: [PATCH 23/46] Remove unused VERSION_STR constant --- llarp/constants/version.cpp.in | 1 - llarp/constants/version.hpp | 1 - 2 files changed, 2 deletions(-) diff --git a/llarp/constants/version.cpp.in b/llarp/constants/version.cpp.in index 57156338db..bca06675cc 100644 --- a/llarp/constants/version.cpp.in +++ b/llarp/constants/version.cpp.in @@ -6,7 +6,6 @@ namespace llarp // clang-format off const std::array VERSION{{@lokinet_VERSION_MAJOR@, @lokinet_VERSION_MINOR@, @lokinet_VERSION_PATCH@}}; const std::array ROUTER_VERSION{{llarp::constants::proto_version, @lokinet_VERSION_MAJOR@, @lokinet_VERSION_MINOR@, @lokinet_VERSION_PATCH@}}; - const char* const VERSION_STR = "@lokinet_VERSION_MAJOR@.@lokinet_VERSION_MINOR@.@lokinet_VERSION_PATCH@"; const char* const VERSION_TAG = "@VERSIONTAG@"; const char* const VERSION_FULL = "lokinet-@lokinet_VERSION_MAJOR@.@lokinet_VERSION_MINOR@.@lokinet_VERSION_PATCH@-@VERSIONTAG@"; diff --git a/llarp/constants/version.hpp b/llarp/constants/version.hpp index 2cdfc77600..caa58b0af9 100644 --- a/llarp/constants/version.hpp +++ b/llarp/constants/version.hpp @@ -8,7 +8,6 @@ namespace llarp // Given a full lokinet version of: lokinet-1.2.3-abc these are: extern const std::array VERSION; // [1, 2, 3] extern const std::array ROUTER_VERSION; // [proto, 1, 2, 3] - extern const char* const VERSION_STR; // "1.2.3" extern const char* const VERSION_TAG; // "abc" extern const char* const VERSION_FULL; // "lokinet-1.2.3-abc" From f5e4af93f6b8f4d3b657a76f9af1177b2c36c09e Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Fri, 28 Oct 2022 21:46:44 -0300 Subject: [PATCH 24/46] Improve windows running-as-a-service detection works Get rid of the --win32-daemon hack (which was removed from the service itself earlier in this PR, by mistake) and replace it with detection of the error code for "not running as a service" that windows gives us back if we try to set up service controller dispatching but aren't a service. --- daemon/lokinet.cpp | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/daemon/lokinet.cpp b/daemon/lokinet.cpp index c17ea727c3..3044fe5221 100644 --- a/daemon/lokinet.cpp +++ b/daemon/lokinet.cpp @@ -344,14 +344,25 @@ main(int argc, char* argv[]) #else SERVICE_TABLE_ENTRY DispatchTable[] = { {strdup("lokinet"), (LPSERVICE_MAIN_FUNCTION)win32_daemon_entry}, {NULL, NULL}}; - if (std::string{argv[1]} == "--win32-daemon") + + // 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) { - return StartServiceCtrlDispatcher(DispatchTable); + llarp::sys::service_manager->disable(); + return lokinet_main(argc, argv); } else { - llarp::sys::service_manager->disable(); - return lokinet_main(argc, argv); + llarp::log::critical( + logcat, "Error launching service: {}", std::system_category().message(error)); + return 1; } #endif } @@ -603,9 +614,8 @@ SvcCtrlHandler(DWORD dwCtrl) } } -// 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, LPTSTR* argv) { From 73b571139ac9e0efb7d0925a52110f3cd69790a3 Mon Sep 17 00:00:00 2001 From: Thomas Winget Date: Mon, 31 Oct 2022 10:32:37 -0400 Subject: [PATCH 25/46] remove dead dns resolver code --- llarp/dns/unbound_resolver.cpp | 240 --------------------------------- llarp/dns/unbound_resolver.hpp | 75 ----------- 2 files changed, 315 deletions(-) delete mode 100644 llarp/dns/unbound_resolver.cpp delete mode 100644 llarp/dns/unbound_resolver.hpp diff --git a/llarp/dns/unbound_resolver.cpp b/llarp/dns/unbound_resolver.cpp deleted file mode 100644 index e29c393899..0000000000 --- a/llarp/dns/unbound_resolver.cpp +++ /dev/null @@ -1,240 +0,0 @@ -#include "unbound_resolver.hpp" - -#include "server.hpp" -#include -#include -#include -#include -#include - -#include - -namespace llarp::dns -{ - static auto logcat = log::Cat("dns"); - - struct PendingUnboundLookup - { - std::weak_ptr resolver; - Message msg; - SockAddr resolverAddr; - SockAddr askerAddr; - }; - - void - UnboundResolver::Stop() - { - Reset(); - } - - void - UnboundResolver::Reset() - { - started = false; -#ifdef _WIN32 - if (runner.joinable()) - { - runner.join(); - } -#else - if (udp) - { - udp->close(); - } - udp.reset(); -#endif - if (unboundContext) - { - ub_ctx_delete(unboundContext); - } - unboundContext = nullptr; - } - - UnboundResolver::UnboundResolver(EventLoop_ptr _loop, ReplyFunction reply, FailFunction fail) - : unboundContext{nullptr} - , started{false} - , replyFunc{_loop->make_caller(std::move(reply))} - , failFunc{_loop->make_caller(std::move(fail))} - { -#ifndef _WIN32 - loop = _loop->MaybeGetUVWLoop(); -#endif - } - - // static callback - void - UnboundResolver::Callback(void* data, int err, ub_result* result) - { - std::unique_ptr lookup{static_cast(data)}; - - auto this_ptr = lookup->resolver.lock(); - if (not this_ptr) - return; // resolver is gone, so we don't reply. - - if (err != 0) - { - Message& msg = lookup->msg; - msg.AddServFail(); - this_ptr->failFunc(lookup->askerAddr, lookup->resolverAddr, msg); - ub_resolve_free(result); - return; - } - OwnedBuffer pkt{(size_t)result->answer_len}; - std::memcpy(pkt.buf.get(), result->answer_packet, pkt.sz); - llarp_buffer_t buf(pkt); - - MessageHeader hdr; - hdr.Decode(&buf); - hdr.id = lookup->msg.hdr_id; - - buf.cur = buf.base; - hdr.Encode(&buf); - - this_ptr->replyFunc(lookup->askerAddr, lookup->resolverAddr, std::move(pkt)); - - ub_resolve_free(result); - } - - bool - UnboundResolver::Init() - { - if (started) - { - Reset(); - } - - unboundContext = ub_ctx_create(); - - if (not unboundContext) - { - return false; - } - - // disable ip6 for upstream dns - ub_ctx_set_option(unboundContext, "prefer-ip6", "0"); - // enable async - ub_ctx_async(unboundContext, 1); -#ifdef _WIN32 - runner = std::thread{[&]() { - while (started) - { - if (unboundContext) - ub_wait(unboundContext); - std::this_thread::sleep_for(25ms); - } - if (unboundContext) - ub_process(unboundContext); - }}; -#else - if (auto loop_ptr = loop.lock()) - { - udp = loop_ptr->resource(ub_fd(unboundContext)); - udp->on([ptr = weak_from_this()](auto&, auto&) { - if (auto self = ptr.lock()) - { - if (self->unboundContext) - { - ub_process(self->unboundContext); - } - } - }); - udp->start(uvw::PollHandle::Event::READABLE); - } -#endif - started = true; - return true; - } - - bool - UnboundResolver::AddUpstreamResolver(const SockAddr& upstreamResolver) - { - const auto hoststr = upstreamResolver.hostString(); - std::string upstream = hoststr; - - const auto port = upstreamResolver.getPort(); - if (port != 53) - { - upstream += '@'; - upstream += std::to_string(port); - } - - log::info("Adding upstream resolver ", upstream); - if (ub_ctx_set_fwd(unboundContext, upstream.c_str()) != 0) - { - Reset(); - return false; - } - - if constexpr (platform::is_apple) - { - // On Apple, when we turn on exit mode, we can't directly connect to upstream from here - // because, from within the network extension, macOS ignores setting the tunnel as the default - // route and would leak all DNS; instead we have to bounce things through the objective C - // trampoline code so that it can call into Apple's special snowflake API to set up a socket - // that has the magic Apple snowflake sauce added on top so that it actually routes through - // the tunnel instead of around it. - // - // This behaviour is all carefully and explicitly documented by Apple with plenty of examples - // and other exposition, of course, just like all of their wonderful new APIs to reinvent - // standard unix interfaces. - if (hoststr == "127.0.0.1" && port == apple::dns_trampoline_port) - { - // Not at all clear why this is needed but without it we get "send failed: Can't assign - // requested address" when unbound tries to connect to the localhost address using a source - // address of 0.0.0.0. Yay apple. - ub_ctx_set_option(unboundContext, "outgoing-interface:", "127.0.0.1"); - - // The trampoline expects just a single source port (and sends everything back to it) - ub_ctx_set_option(unboundContext, "outgoing-range:", "1"); - ub_ctx_set_option(unboundContext, "outgoing-port-avoid:", "0-65535"); - ub_ctx_set_option( - unboundContext, - "outgoing-port-permit:", - std::to_string(apple::dns_trampoline_source_port).c_str()); - } - } - - return true; - } - - void - UnboundResolver::AddHostsFile(const fs::path& file) - { - log::debug(logcat, "adding hosts file {}", file); - const auto str = file.u8string(); - if (auto ret = ub_ctx_hosts(unboundContext, str.c_str())) - throw std::runtime_error{ - fmt::format("Failed to add host file {}: {}", file, ub_strerror(ret))}; - log::info(logcat, "added hosts file {}", file); - } - - void - UnboundResolver::Lookup(SockAddr to, SockAddr from, Message msg) - { - if (not unboundContext) - { - msg.AddServFail(); - failFunc(from, to, std::move(msg)); - return; - } - - const auto& q = msg.questions[0]; - auto* lookup = new PendingUnboundLookup{weak_from_this(), msg, to, from}; - int err = ub_resolve_async( - unboundContext, - q.Name().c_str(), - q.qtype, - q.qclass, - (void*)lookup, - &UnboundResolver::Callback, - nullptr); - - if (err != 0) - { - msg.AddServFail(); - failFunc(from, to, std::move(msg)); - return; - } - } - -} // namespace llarp::dns diff --git a/llarp/dns/unbound_resolver.hpp b/llarp/dns/unbound_resolver.hpp deleted file mode 100644 index 4d79569ce9..0000000000 --- a/llarp/dns/unbound_resolver.hpp +++ /dev/null @@ -1,75 +0,0 @@ -#pragma once - -#include -#include -#include -#include - -#include -#include - -#include "message.hpp" - -#ifdef _WIN32 -#include -#else -#include -#endif - -extern "C" -{ - struct ub_ctx; - struct ub_result; -} - -namespace llarp::dns -{ - using ReplyFunction = - std::function; - using FailFunction = - std::function; - - class UnboundResolver : public std::enable_shared_from_this - { - private: - ub_ctx* unboundContext; - - std::atomic started; - -#ifdef _WIN32 - std::thread runner; -#else - std::weak_ptr loop; - std::shared_ptr udp; -#endif - - ReplyFunction replyFunc; - FailFunction failFunc; - void - Reset(); - - public: - UnboundResolver(EventLoop_ptr loop, ReplyFunction replyFunc, FailFunction failFunc); - - static void - Callback(void* data, int err, ub_result* result); - - // stop resolver thread - void - Stop(); - - // upstream resolver IP can be IPv4 or IPv6 - bool - Init(); - - bool - AddUpstreamResolver(const SockAddr& upstreamResolverIP); - - void - AddHostsFile(const fs::path& file); - - void - Lookup(SockAddr to, SockAddr from, Message msg); - }; - -} // namespace llarp::dns From 82e85bdd61f9195970844d7b9d72925b851bc554 Mon Sep 17 00:00:00 2001 From: Jeff Becker Date: Mon, 31 Oct 2022 10:53:56 -0400 Subject: [PATCH 26/46] fix crash on shutdown we were calling llarp::Context::HandleSignal from a non mainloop thread when running as a win32 service. this caused issues with a non clean destruction. call our signal handler instead of llarp::Context::HandleSignal --- daemon/lokinet.cpp | 1 + llarp/win32/service_manager.cpp | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/daemon/lokinet.cpp b/daemon/lokinet.cpp index 3044fe5221..7a6a37de17 100644 --- a/daemon/lokinet.cpp +++ b/daemon/lokinet.cpp @@ -601,6 +601,7 @@ SvcCtrlHandler(DWORD dwCtrl) // 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: diff --git a/llarp/win32/service_manager.cpp b/llarp/win32/service_manager.cpp index 95f6818093..0015158de3 100644 --- a/llarp/win32/service_manager.cpp +++ b/llarp/win32/service_manager.cpp @@ -48,7 +48,6 @@ namespace llarp::sys if (st == ServiceState::Stopping) { we_changed_our_state(st); - m_Context->HandleSignal(SIGINT); } } From d128182875997c68db61706df6c90c28143d18e4 Mon Sep 17 00:00:00 2001 From: Thomas Winget Date: Mon, 31 Oct 2022 12:46:21 -0400 Subject: [PATCH 27/46] stop-time debug statements --- llarp/service/context.cpp | 2 ++ llarp/service/endpoint.cpp | 3 +++ 2 files changed, 5 insertions(+) diff --git a/llarp/service/context.cpp b/llarp/service/context.cpp index 487e69f662..4b7c00bb44 100644 --- a/llarp/service/context.cpp +++ b/llarp/service/context.cpp @@ -47,7 +47,9 @@ namespace llarp auto itr = m_Endpoints.begin(); while (itr != m_Endpoints.end()) { + log::debug(logcat, "Stopping endpoint {}.", itr->first); itr->second->Stop(); + log::debug(logcat, "Endpoint {} stopped.", itr->first); m_Stopped.emplace_back(std::move(itr->second)); itr = m_Endpoints.erase(itr); } diff --git a/llarp/service/endpoint.cpp b/llarp/service/endpoint.cpp index ee7418fc15..d835f816d6 100644 --- a/llarp/service/endpoint.cpp +++ b/llarp/service/endpoint.cpp @@ -387,9 +387,12 @@ namespace llarp Endpoint::Stop() { // stop remote sessions + log::debug(logcat, "Endpoint stopping remote sessions."); EndpointUtil::StopRemoteSessions(m_state->m_RemoteSessions); // stop snode sessions + log::debug(logcat, "Endpoint stopping snode sessions."); EndpointUtil::StopSnodeSessions(m_state->m_SNodeSessions); + log::debug(logcat, "Endpoint stopping its path builder."); return path::Builder::Stop(); } From 4612a45cc364f2ae6eba0892514d50a404453ba9 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Mon, 31 Oct 2022 13:01:47 -0300 Subject: [PATCH 28/46] =?UTF-8?q?Revert=20"=F0=9F=A4=B7=20Work=20around=20?= =?UTF-8?q?ub=5Fctx=5Fdelete=20crash"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 3f2311887ffb4082d4891da7332b26c6dae4ce6a. --- llarp/dns/server.cpp | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/llarp/dns/server.cpp b/llarp/dns/server.cpp index bd5d93c367..4ef4ce919c 100644 --- a/llarp/dns/server.cpp +++ b/llarp/dns/server.cpp @@ -370,15 +370,8 @@ namespace llarp::dns ConfigureUpstream(conf); -#ifdef _WIN32 - // threaded async currently crashes on ub_ctx_delete so use process async mode instead on - // Windows: - ub_ctx_async(m_ctx, 0); -#else - // set up threaded async mode: + // set async ub_ctx_async(m_ctx, 1); -#endif - // setup mainloop #ifdef _WIN32 running = true; From febfa731d9d5b5f2b23613708fb487dc384d0d91 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Mon, 31 Oct 2022 13:27:38 -0300 Subject: [PATCH 29/46] Don't raise log level on shutdown If already below info (e.g. debug) it should stay there; we only want to *lower* it to info if above info. --- llarp/router/router.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/llarp/router/router.cpp b/llarp/router/router.cpp index dd4cdb14da..b5d1312ba7 100644 --- a/llarp/router/router.cpp +++ b/llarp/router/router.cpp @@ -1554,7 +1554,8 @@ namespace llarp } _stopping.store(true); - if (log::get_level_default() != log::Level::off) + if (auto level = log::get_level_default(); + level > log::Level::info and level != log::Level::off) log::reset_level(log::Level::info); log::info(logcat, "stopping"); llarp::sys::service_manager->stopping(); From 465d4f8d3b23c5c7ca543dab33be4766984d076f Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Mon, 31 Oct 2022 13:32:46 -0300 Subject: [PATCH 30/46] Remove bad assert We do and should be able to call this multiple times during shutdown to signal that we are advancing through shutdown. --- llarp/win32/service_manager.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/llarp/win32/service_manager.cpp b/llarp/win32/service_manager.cpp index 0015158de3..986b0c0244 100644 --- a/llarp/win32/service_manager.cpp +++ b/llarp/win32/service_manager.cpp @@ -86,7 +86,6 @@ namespace llarp::sys else if (auto maybe_state = to_win32_state(st)) { 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; From 953da5dc6ce16e2aa7dcf7dc04fe093b2c71c4b5 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Mon, 31 Oct 2022 13:34:26 -0300 Subject: [PATCH 31/46] Add more stopping signals --- llarp/router/router.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/llarp/router/router.cpp b/llarp/router/router.cpp index b5d1312ba7..f3a807edd4 100644 --- a/llarp/router/router.cpp +++ b/llarp/router/router.cpp @@ -1499,6 +1499,7 @@ namespace llarp void Router::AfterStopLinks() { + llarp::sys::service_manager->stopping(); Close(); log::debug(logcat, "stopping oxenmq"); m_lmq.reset(); @@ -1507,6 +1508,7 @@ namespace llarp void Router::AfterStopIssued() { + llarp::sys::service_manager->stopping(); log::debug(logcat, "stopping links"); StopLinks(); log::debug(logcat, "saving nodedb to disk"); @@ -1561,10 +1563,13 @@ namespace llarp llarp::sys::service_manager->stopping(); log::debug(logcat, "stopping hidden service context"); hiddenServiceContext().StopAll(); + llarp::sys::service_manager->stopping(); log::debug(logcat, "stopping exit context"); _exitContext.Stop(); + llarp::sys::service_manager->stopping(); log::debug(logcat, "final upstream pump"); paths.PumpUpstream(); + llarp::sys::service_manager->stopping(); log::debug(logcat, "final links pump"); _linkManager.PumpLinks(); _loop->call_later(200ms, [this] { AfterStopIssued(); }); From 916c8c48fa17fb9a9b7b1516b633042518c53257 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Mon, 31 Oct 2022 14:05:22 -0300 Subject: [PATCH 32/46] windivert: avoid trying to send during shutdown Occasionally during shutdown windivert will crash because a thread tries sending after we've called wd::shutdown, which isn't allowed. Add an atomic bool to prevent this. --- llarp/win32/windivert.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/llarp/win32/windivert.cpp b/llarp/win32/windivert.cpp index 02a799bc7a..344718db1b 100644 --- a/llarp/win32/windivert.cpp +++ b/llarp/win32/windivert.cpp @@ -61,6 +61,7 @@ namespace llarp::win32 HANDLE m_Handle; std::thread m_Runner; + std::atomic m_Shutdown{false}; thread::Queue m_RecvQueue; // dns packet queue size static constexpr size_t recv_queue_size = 64; @@ -93,6 +94,7 @@ namespace llarp::win32 { auto err = GetLastError(); if (err == ERROR_NO_DATA) + // The handle is shut down and the packet queue is empty return std::nullopt; if (err == ERROR_BROKEN_PIPE) { @@ -116,9 +118,8 @@ namespace llarp::win32 const auto* addr = &w_pkt.addr; 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; - throw win32::error{"windivert send failed"}; + if (!wd::send(m_Handle, pkt.data(), pkt.size(), &sz, addr)) + throw win32::error{"windivert send failed"}; } virtual int @@ -141,7 +142,8 @@ namespace llarp::win32 return net::IPPacket{}; net::IPPacket pkt{std::move(w_pkt->pkt)}; pkt.reply = [this, addr = std::move(w_pkt->addr)](auto pkt) { - send_packet(Packet{pkt.steal(), addr}); + if (!m_Shutdown) + send_packet(Packet{pkt.steal(), addr}); }; return pkt; } @@ -178,6 +180,7 @@ namespace llarp::win32 Stop() override { log::info(logcat, "stopping windivert"); + m_Shutdown = true; wd::shutdown(m_Handle, WINDIVERT_SHUTDOWN_BOTH); m_Runner.join(); } From de6f9d416cd1c7ea3b370861a422c90d77d33517 Mon Sep 17 00:00:00 2001 From: Jeff Becker Date: Mon, 31 Oct 2022 13:15:29 -0400 Subject: [PATCH 33/46] rewordme --- llarp/dns/server.cpp | 56 +++++++++++++++++++++----------------------- 1 file changed, 27 insertions(+), 29 deletions(-) diff --git a/llarp/dns/server.cpp b/llarp/dns/server.cpp index bd5d93c367..36395cf0ce 100644 --- a/llarp/dns/server.cpp +++ b/llarp/dns/server.cpp @@ -86,9 +86,9 @@ namespace llarp::dns { class Resolver; - class Query : public QueryJob_Base + class Query : public QueryJob_Base, public std::enable_shared_from_this { - std::shared_ptr src; + std::weak_ptr src; SockAddr resolverAddr; SockAddr askerAddr; @@ -100,7 +100,7 @@ namespace llarp::dns SockAddr toaddr, SockAddr fromaddr) : QueryJob_Base{std::move(query)} - , src{std::move(pktsrc)} + , src{pktsrc} , resolverAddr{std::move(toaddr)} , askerAddr{std::move(fromaddr)} , parent{parent_} @@ -126,7 +126,7 @@ namespace llarp::dns #endif std::optional m_LocalAddr; - std::set m_Pending; + std::unordered_map> m_Pending; struct ub_result_deleter { @@ -148,9 +148,8 @@ namespace llarp::dns { // take ownership of ub_result std::unique_ptr result{_result}; - // take ownership of our query - std::unique_ptr query{static_cast(data)}; - + // borrow query + auto query = reinterpret_cast(data)->shared_from_this(); if (err) { // some kind of error from upstream @@ -370,14 +369,8 @@ namespace llarp::dns ConfigureUpstream(conf); -#ifdef _WIN32 - // threaded async currently crashes on ub_ctx_delete so use process async mode instead on - // Windows: - ub_ctx_async(m_ctx, 0); -#else // set up threaded async mode: ub_ctx_async(m_ctx, 1); -#endif // setup mainloop #ifdef _WIN32 @@ -385,10 +378,14 @@ namespace llarp::dns runner = std::thread{[this]() { while (running) { - ub_wait(m_ctx); - std::this_thread::sleep_for(10ms); + // poll and process callbacks it this thread + if (ub_poll(m_ctx)) + { + ub_process(m_ctx); + } + else // nothing to do, sleep. + std::this_thread::sleep_for(10ms); } - ub_process(m_ctx); }}; #else if (auto loop = m_Loop.lock()) @@ -418,12 +415,14 @@ namespace llarp::dns if (m_ctx) { // cancel pending queries - // make copy as ub_cancel modifies m_Pending - const auto pending = m_Pending; - for (auto id : pending) - ::ub_cancel(m_ctx, id); + for (const auto& [id, query] : m_Pending) + query->Cancel(); + m_Pending.clear(); + if (auto err = ::ub_wait(m_ctx)) + log::warning(logcat, "issue tearing down unbound: {}", ub_strerror(err)); + ::ub_ctx_delete(m_ctx); m_ctx = nullptr; } @@ -478,7 +477,7 @@ namespace llarp::dns if (WouldLoop(to, from)) return false; // we use this unique ptr to clean up on fail - auto tmp = std::make_unique(weak_from_this(), query, source, to, from); + auto tmp = std::make_shared(weak_from_this(), query, source, to, from); // no questions, send fail if (query.questions.empty()) { @@ -516,11 +515,8 @@ namespace llarp::dns tmp->Cancel(); } else - { - m_Pending.insert(tmp->id); - // Leak the bare pointer we gave to unbound; we'll recapture it in Callback - (void)tmp.release(); - } + m_Pending.emplace(tmp->id, tmp); + return true; } }; @@ -528,10 +524,12 @@ namespace llarp::dns void Query::SendReply(llarp::OwnedBuffer replyBuf) const { - if (auto ptr = parent.lock()) + auto parent_ptr = parent.lock(); + auto src_ptr = src.lock(); + if (parent_ptr and src_ptr) { - ptr->call([src = src, from = resolverAddr, to = askerAddr, buf = replyBuf.copy()] { - src->SendTo(to, from, OwnedBuffer::copy_from(buf)); + parent_ptr->call([src_ptr, from = resolverAddr, to = askerAddr, buf = replyBuf.copy()] { + src_ptr->SendTo(to, from, OwnedBuffer::copy_from(buf)); }); } else From df416eaff19d86fe3a00039f4390352daa0f966f Mon Sep 17 00:00:00 2001 From: Jeff Becker Date: Mon, 31 Oct 2022 13:18:59 -0400 Subject: [PATCH 34/46] format pedant --- llarp/dns/server.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/llarp/dns/server.cpp b/llarp/dns/server.cpp index 4b185f3186..bcec5fb433 100644 --- a/llarp/dns/server.cpp +++ b/llarp/dns/server.cpp @@ -369,7 +369,6 @@ namespace llarp::dns ConfigureUpstream(conf); - // set async ub_ctx_async(m_ctx, 1); // setup mainloop From 7988724db161bd9be7a9e1724ba74fb7d6d112d7 Mon Sep 17 00:00:00 2001 From: Jeff Becker Date: Mon, 31 Oct 2022 13:26:33 -0400 Subject: [PATCH 35/46] dont clear m_pending on shutdown that is taken care of that for us on win32 make sure we do not query when we are shutting down --- llarp/dns/server.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/llarp/dns/server.cpp b/llarp/dns/server.cpp index bcec5fb433..9879a97250 100644 --- a/llarp/dns/server.cpp +++ b/llarp/dns/server.cpp @@ -417,8 +417,6 @@ namespace llarp::dns for (const auto& [id, query] : m_Pending) query->Cancel(); - m_Pending.clear(); - if (auto err = ::ub_wait(m_ctx)) log::warning(logcat, "issue tearing down unbound: {}", ub_strerror(err)); @@ -499,6 +497,15 @@ namespace llarp::dns tmp->Cancel(); return true; } + +#ifdef _WIN32 + if (not running) + { + // we are stopping the win32 thread + tmp->Cancel(); + return true; + } +#endif const auto& q = query.questions[0]; if (auto err = ub_resolve_async( m_ctx, From 3548b7576395a51885161cbfe8d061f5b6aed396 Mon Sep 17 00:00:00 2001 From: Jeff Becker Date: Mon, 31 Oct 2022 13:37:43 -0400 Subject: [PATCH 36/46] i hate this, a query's packetsource_base needed to be a shared_ptr because the query is kept alive by it (sometimes) --- llarp/dns/server.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/llarp/dns/server.cpp b/llarp/dns/server.cpp index 9879a97250..4e07f954c1 100644 --- a/llarp/dns/server.cpp +++ b/llarp/dns/server.cpp @@ -88,7 +88,7 @@ namespace llarp::dns class Query : public QueryJob_Base, public std::enable_shared_from_this { - std::weak_ptr src; + std::shared_ptr src; SockAddr resolverAddr; SockAddr askerAddr; @@ -100,7 +100,7 @@ namespace llarp::dns SockAddr toaddr, SockAddr fromaddr) : QueryJob_Base{std::move(query)} - , src{pktsrc} + , src{std::move(pktsrc)} , resolverAddr{std::move(toaddr)} , askerAddr{std::move(fromaddr)} , parent{parent_} From a72e94c9c15aa1c2ff5179ae97cc824a31e298f9 Mon Sep 17 00:00:00 2001 From: Jeff Becker Date: Mon, 31 Oct 2022 13:39:00 -0400 Subject: [PATCH 37/46] fix previous commit --- llarp/dns/server.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/llarp/dns/server.cpp b/llarp/dns/server.cpp index 4e07f954c1..51a5ad78d5 100644 --- a/llarp/dns/server.cpp +++ b/llarp/dns/server.cpp @@ -531,15 +531,14 @@ namespace llarp::dns Query::SendReply(llarp::OwnedBuffer replyBuf) const { auto parent_ptr = parent.lock(); - auto src_ptr = src.lock(); - if (parent_ptr and src_ptr) + if (parent_ptr) { - parent_ptr->call([src_ptr, from = resolverAddr, to = askerAddr, buf = replyBuf.copy()] { - src_ptr->SendTo(to, from, OwnedBuffer::copy_from(buf)); + parent_ptr->call([src, from = resolverAddr, to = askerAddr, buf = replyBuf.copy()] { + src->SendTo(to, from, OwnedBuffer::copy_from(buf)); }); } else - log::error(logcat, "no source or parent"); + log::error(logcat, "no parent"); } } // namespace libunbound From 5fa87c0cca2e7273f7bb5c93e926e1ab640c7c2b Mon Sep 17 00:00:00 2001 From: Jeff Becker Date: Mon, 31 Oct 2022 13:40:43 -0400 Subject: [PATCH 38/46] fix typo remove comment --- llarp/dns/server.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/llarp/dns/server.cpp b/llarp/dns/server.cpp index 51a5ad78d5..06d3b49cbf 100644 --- a/llarp/dns/server.cpp +++ b/llarp/dns/server.cpp @@ -473,7 +473,7 @@ namespace llarp::dns { if (WouldLoop(to, from)) return false; - // we use this unique ptr to clean up on fail + auto tmp = std::make_shared(weak_from_this(), query, source, to, from); // no questions, send fail if (query.questions.empty()) @@ -533,7 +533,7 @@ namespace llarp::dns auto parent_ptr = parent.lock(); if (parent_ptr) { - parent_ptr->call([src, from = resolverAddr, to = askerAddr, buf = replyBuf.copy()] { + parent_ptr->call([src=src, from = resolverAddr, to = askerAddr, buf = replyBuf.copy()] { src->SendTo(to, from, OwnedBuffer::copy_from(buf)); }); } From 7f9843fd5272bd2583902a5372bba30dcc7f7034 Mon Sep 17 00:00:00 2001 From: Jeff Becker Date: Mon, 31 Oct 2022 13:54:22 -0400 Subject: [PATCH 39/46] remove pending query after sending a reply of any kind --- llarp/dns/server.cpp | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/llarp/dns/server.cpp b/llarp/dns/server.cpp index 06d3b49cbf..f37adfa272 100644 --- a/llarp/dns/server.cpp +++ b/llarp/dns/server.cpp @@ -149,7 +149,7 @@ namespace llarp::dns // take ownership of ub_result std::unique_ptr result{_result}; // borrow query - auto query = reinterpret_cast(data)->shared_from_this(); + auto query = static_cast(data)->shared_from_this(); if (err) { // some kind of error from upstream @@ -166,9 +166,7 @@ namespace llarp::dns hdr.id = query->Underlying().hdr_id; buf.cur = buf.base; hdr.Encode(&buf); - // remove pending query - if (auto ptr = query->parent.lock()) - ptr->call([id = query->id, ptr]() { ptr->m_Pending.erase(id); }); + // send reply query->SendReply(std::move(pkt)); } @@ -342,6 +340,12 @@ namespace llarp::dns return m_LocalAddr; } + void + RemovePending(int id) + { + m_Pending.erase(id); + } + void Up(const llarp::DnsConfig& conf) { @@ -473,7 +477,7 @@ namespace llarp::dns { if (WouldLoop(to, from)) return false; - + auto tmp = std::make_shared(weak_from_this(), query, source, to, from); // no questions, send fail if (query.questions.empty()) @@ -533,8 +537,15 @@ namespace llarp::dns auto parent_ptr = parent.lock(); if (parent_ptr) { - parent_ptr->call([src=src, from = resolverAddr, to = askerAddr, buf = replyBuf.copy()] { + parent_ptr->call([parent_ptr, + id = id, + src = src, + from = resolverAddr, + to = askerAddr, + buf = replyBuf.copy()] { src->SendTo(to, from, OwnedBuffer::copy_from(buf)); + // remove query + parent_ptr->RemovePending(id); }); } else From cf25c0bf30fa23096625b4e2614afd1dd70b2940 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Mon, 31 Oct 2022 14:58:35 -0300 Subject: [PATCH 40/46] More debug --- llarp/dns/server.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/llarp/dns/server.cpp b/llarp/dns/server.cpp index f37adfa272..03f6d2049f 100644 --- a/llarp/dns/server.cpp +++ b/llarp/dns/server.cpp @@ -410,13 +410,17 @@ namespace llarp::dns { #ifdef _WIN32 if (running.exchange(false)) + { + log::debug(logcat, "shutting down win32 dns thread"); runner.join(); + } #else if (m_Poller) m_Poller->close(); #endif if (m_ctx) { + log::debug(logcat, "cancelling {} pending queries", m_Pending.size()); // cancel pending queries for (const auto& [id, query] : m_Pending) query->Cancel(); From a390f2a60a99a5cdd7dd88695398e587db5547c5 Mon Sep 17 00:00:00 2001 From: Jeff Becker Date: Mon, 31 Oct 2022 14:49:15 -0400 Subject: [PATCH 41/46] use weak_from_this() --- llarp/dns/server.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/llarp/dns/server.cpp b/llarp/dns/server.cpp index 03f6d2049f..5667d2922e 100644 --- a/llarp/dns/server.cpp +++ b/llarp/dns/server.cpp @@ -149,7 +149,10 @@ namespace llarp::dns // take ownership of ub_result std::unique_ptr result{_result}; // borrow query - auto query = static_cast(data)->shared_from_this(); + auto weak_query = static_cast(data)->weak_from_this(); + auto query = weak_query.lock(); + if(not query) + return; if (err) { // some kind of error from upstream From 35eb542a8c2a5f3aee4a2f69ee10e93b335dbc5a Mon Sep 17 00:00:00 2001 From: Jeff Becker Date: Mon, 31 Oct 2022 14:49:47 -0400 Subject: [PATCH 42/46] format --- llarp/dns/server.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/llarp/dns/server.cpp b/llarp/dns/server.cpp index 5667d2922e..feb99344de 100644 --- a/llarp/dns/server.cpp +++ b/llarp/dns/server.cpp @@ -151,8 +151,8 @@ namespace llarp::dns // borrow query auto weak_query = static_cast(data)->weak_from_this(); auto query = weak_query.lock(); - if(not query) - return; + if (not query) + return; if (err) { // some kind of error from upstream From 387e1d7a850110b8ad7c92f62bba3682b848bccf Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Mon, 31 Oct 2022 17:06:11 -0300 Subject: [PATCH 43/46] Fix crashy race condition in shutdown Query->Cancel() will remove the Query, but that introduces a race condition where unbound may still try to invoke the callback (with a no-longer-valid pointer) if we do it before the ub_ctx_delete call. Move to it afterwards so that we only cancel things that unbound didn't --- llarp/dns/server.cpp | 58 +++++++++++++++++++++----------------------- llarp/dns/server.hpp | 9 ++++--- 2 files changed, 33 insertions(+), 34 deletions(-) diff --git a/llarp/dns/server.cpp b/llarp/dns/server.cpp index feb99344de..d94186aaba 100644 --- a/llarp/dns/server.cpp +++ b/llarp/dns/server.cpp @@ -22,7 +22,7 @@ namespace llarp::dns static auto logcat = log::Cat("dns"); void - QueryJob_Base::Cancel() const + QueryJob_Base::Cancel() { Message reply{m_Query}; reply.AddServFail(); @@ -108,8 +108,8 @@ namespace llarp::dns std::weak_ptr parent; int id{}; - virtual void - SendReply(llarp::OwnedBuffer replyBuf) const override; + void + SendReply(llarp::OwnedBuffer replyBuf) override; }; /// Resolver_Base that uses libunbound @@ -126,7 +126,7 @@ namespace llarp::dns #endif std::optional m_LocalAddr; - std::unordered_map> m_Pending; + std::unordered_set> m_Pending; struct ub_result_deleter { @@ -149,10 +149,7 @@ namespace llarp::dns // take ownership of ub_result std::unique_ptr result{_result}; // borrow query - auto weak_query = static_cast(data)->weak_from_this(); - auto query = weak_query.lock(); - if (not query) - return; + auto* query = static_cast(data); if (err) { // some kind of error from upstream @@ -344,9 +341,9 @@ namespace llarp::dns } void - RemovePending(int id) + RemovePending(const std::shared_ptr& query) { - m_Pending.erase(id); + m_Pending.erase(query); } void @@ -423,16 +420,17 @@ namespace llarp::dns #endif if (m_ctx) { - log::debug(logcat, "cancelling {} pending queries", m_Pending.size()); - // cancel pending queries - for (const auto& [id, query] : m_Pending) - query->Cancel(); - - if (auto err = ::ub_wait(m_ctx)) - log::warning(logcat, "issue tearing down unbound: {}", ub_strerror(err)); - ::ub_ctx_delete(m_ctx); m_ctx = nullptr; + + // destroy any outstanding queries that unbound hasn't fired yet + if (not m_Pending.empty()) + { + log::debug(logcat, "cancelling {} pending queries", m_Pending.size()); + for (const auto& query : m_Pending) + query->Cancel(); + m_Pending.clear(); + } } } @@ -525,35 +523,33 @@ namespace llarp::dns q.qclass, tmp.get(), &Resolver::Callback, - &tmp->id)) + nullptr)) { log::warning( logcat, "failed to send upstream query with libunbound: {}", ub_strerror(err)); tmp->Cancel(); } else - m_Pending.emplace(tmp->id, tmp); + m_Pending.insert(std::move(tmp)); return true; } }; void - Query::SendReply(llarp::OwnedBuffer replyBuf) const + Query::SendReply(llarp::OwnedBuffer replyBuf) { + if (m_Done.test_and_set()) + return; auto parent_ptr = parent.lock(); if (parent_ptr) { - parent_ptr->call([parent_ptr, - id = id, - src = src, - from = resolverAddr, - to = askerAddr, - buf = replyBuf.copy()] { - src->SendTo(to, from, OwnedBuffer::copy_from(buf)); - // remove query - parent_ptr->RemovePending(id); - }); + parent_ptr->call( + [self = shared_from_this(), parent_ptr = std::move(parent_ptr), buf = replyBuf.copy()] { + self->src->SendTo(self->askerAddr, self->resolverAddr, OwnedBuffer::copy_from(buf)); + // remove query + parent_ptr->RemovePending(self); + }); } else log::error(logcat, "no parent"); diff --git a/llarp/dns/server.hpp b/llarp/dns/server.hpp index fcf72111a9..7f22df48ee 100644 --- a/llarp/dns/server.hpp +++ b/llarp/dns/server.hpp @@ -17,6 +17,9 @@ namespace llarp::dns /// the original dns query Message m_Query; + /// True if we've sent a reply (including via a call to cancel) + std::atomic_flag m_Done = ATOMIC_FLAG_INIT; + public: explicit QueryJob_Base(Message query) : m_Query{std::move(query)} {} @@ -37,11 +40,11 @@ namespace llarp::dns /// cancel this operation and inform anyone who cares void - Cancel() const; + Cancel(); /// send a raw buffer back to the querier virtual void - SendReply(llarp::OwnedBuffer replyBuf) const = 0; + SendReply(llarp::OwnedBuffer replyBuf) = 0; }; class PacketSource_Base @@ -130,7 +133,7 @@ namespace llarp::dns {} void - SendReply(llarp::OwnedBuffer replyBuf) const override + SendReply(llarp::OwnedBuffer replyBuf) override { src->SendTo(asker, resolver, std::move(replyBuf)); } From eb78f961fa6072adf6f1e7803430fab12f1e2c22 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Mon, 31 Oct 2022 21:22:13 -0300 Subject: [PATCH 44/46] Fix crash on unbound cleanup We need to make a copy here because (see comment). --- llarp/dns/server.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/llarp/dns/server.cpp b/llarp/dns/server.cpp index d94186aaba..4a0d22da4d 100644 --- a/llarp/dns/server.cpp +++ b/llarp/dns/server.cpp @@ -427,9 +427,12 @@ namespace llarp::dns if (not m_Pending.empty()) { log::debug(logcat, "cancelling {} pending queries", m_Pending.size()); - for (const auto& query : m_Pending) + // We must copy because Cancel does a loop call to remove itself, but since we are + // already in the main loop it happens immediately, which would invalidate our iterator + // if we were looping through m_Pending at the time. + auto copy = m_Pending; + for (const auto& query : copy) query->Cancel(); - m_Pending.clear(); } } } From 3fc097c972c93683e97d5d4e4ef55af33a8f53e4 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Mon, 31 Oct 2022 22:27:20 -0300 Subject: [PATCH 45/46] patch unbound to fix windows shutdown crash --- cmake/StaticBuild.cmake | 7 ++++ .../patches/unbound-delete-crash-fix.patch | 33 +++++++++++++++++++ 2 files changed, 40 insertions(+) create mode 100644 contrib/patches/unbound-delete-crash-fix.patch diff --git a/cmake/StaticBuild.cmake b/cmake/StaticBuild.cmake index d0b9f7b291..d7a6f19d57 100644 --- a/cmake/StaticBuild.cmake +++ b/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 --$,enable,disable>-flto --with-ssl=${DEPS_DESTDIR} diff --git a/contrib/patches/unbound-delete-crash-fix.patch b/contrib/patches/unbound-delete-crash-fix.patch new file mode 100644 index 0000000000..d80799d5f8 --- /dev/null +++ b/contrib/patches/unbound-delete-crash-fix.patch @@ -0,0 +1,33 @@ +commit 56d816014d5e8a7eb055169c7e13a303dad5e50f +Author: Jason Rhinelander +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) From 5868828b01287d55723ba99971b0c38fafea2132 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Mon, 31 Oct 2022 22:28:31 -0300 Subject: [PATCH 46/46] RPC: fix "halt" command --- llarp/rpc/rpc_server.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llarp/rpc/rpc_server.cpp b/llarp/rpc/rpc_server.cpp index 9ba21868d0..afab58387a 100644 --- a/llarp/rpc/rpc_server.cpp +++ b/llarp/rpc/rpc_server.cpp @@ -146,7 +146,7 @@ namespace llarp::rpc m_LMQ->listen_plain(url.zmq_address()); m_LMQ->add_category("llarp", oxenmq::AuthLevel::none) .add_request_command("logs", [this](oxenmq::Message& msg) { HandleLogsSubRequest(msg); }) - .add_command( + .add_request_command( "halt", [&](oxenmq::Message& msg) { if (not m_Router->IsRunning())