From c9939d9c093b9379e3143329e0ef9660193339e1 Mon Sep 17 00:00:00 2001 From: Sotiris Nanopoulos Date: Fri, 6 Nov 2020 19:22:30 -0800 Subject: [PATCH 01/33] unix builds Signed-off-by: Sotiris Nanopoulos --- .bazelrc | 1 + include/envoy/common/platform.h | 14 ++++++ include/envoy/event/dispatcher.h | 2 +- source/common/event/BUILD | 33 +++++++++++-- source/common/event/dispatcher_impl.cc | 2 +- source/common/event/dispatcher_impl.h | 2 +- .../common/event/{ => posix}/signal_impl.cc | 2 +- source/common/event/{ => posix}/signal_impl.h | 5 +- source/common/event/win32/signal_impl.cc | 40 ++++++++++++++++ source/common/event/win32/signal_impl.h | 32 +++++++++++++ source/exe/BUILD | 1 + source/exe/win32/platform_impl.cc | 23 +++++++++ source/server/server.cc | 48 ++++++++++--------- test/mocks/event/mocks.h | 4 +- test/mocks/event/wrapped_dispatcher.h | 2 +- 15 files changed, 175 insertions(+), 36 deletions(-) rename source/common/event/{ => posix}/signal_impl.cc (81%) rename source/common/event/{ => posix}/signal_impl.h (81%) create mode 100644 source/common/event/win32/signal_impl.cc create mode 100644 source/common/event/win32/signal_impl.h diff --git a/.bazelrc b/.bazelrc index 3f2fab533837..8d3ec16264ae 100644 --- a/.bazelrc +++ b/.bazelrc @@ -302,6 +302,7 @@ build:windows --action_env=TMPDIR build:windows --define signal_trace=disabled build:windows --define hot_restart=disabled build:windows --define tcmalloc=disabled +build:windows --define wasm=disabled build:windows --define manual_stamp=manual_stamp build:windows --cxxopt="/std:c++17" diff --git a/include/envoy/common/platform.h b/include/envoy/common/platform.h index 07bf4319ef2d..ef0558cf0e96 100644 --- a/include/envoy/common/platform.h +++ b/include/envoy/common/platform.h @@ -65,6 +65,7 @@ typedef uint32_t mode_t; typedef SOCKET os_fd_t; typedef HANDLE filesystem_os_id_t; // NOLINT(modernize-use-using) +typedef DWORD signat_t; // NOLINT(modernize-use-using) typedef unsigned int sa_family_t; @@ -151,6 +152,12 @@ struct msghdr { #define HANDLE_ERROR_PERM ERROR_ACCESS_DENIED #define HANDLE_ERROR_INVALID ERROR_INVALID_HANDLE +#define ENVOY_SIGINIT CTRL_C_EVENT +#define ENVOY_SIGTERM CTRL_CLOSE_EVENT +#define ENVOY_SIGUSR1 -1 +#define ENVOY_SIGHUP -1 + + namespace Platform { constexpr absl::string_view null_device_path{"NUL"}; } @@ -215,6 +222,8 @@ constexpr absl::string_view null_device_path{"NUL"}; typedef int os_fd_t; typedef int filesystem_os_id_t; // NOLINT(modernize-use-using) +typedef int signat_t; // NOLINT(modernize-use-using) + #define INVALID_HANDLE -1 #define INVALID_SOCKET -1 @@ -245,6 +254,11 @@ typedef int filesystem_os_id_t; // NOLINT(modernize-use-using) #define HANDLE_ERROR_PERM EACCES #define HANDLE_ERROR_INVALID EBADF +#define ENVOY_SIGTERM SIGTERM +#define ENVOY_SIGINIT SIGINT +#define ENVOY_SIGUSR1 SIGUSR1 +#define ENVOY_SIGHUP SIGHUP + namespace Platform { constexpr absl::string_view null_device_path{"/dev/null"}; } diff --git a/include/envoy/event/dispatcher.h b/include/envoy/event/dispatcher.h index bd4e9512aa4b..5467be4231ed 100644 --- a/include/envoy/event/dispatcher.h +++ b/include/envoy/event/dispatcher.h @@ -205,7 +205,7 @@ class Dispatcher { * @param cb supplies the callback to invoke when the signal fires. * @return SignalEventPtr a signal event that is owned by the caller. */ - virtual SignalEventPtr listenForSignal(int signal_num, SignalCb cb) PURE; + virtual SignalEventPtr listenForSignal(signat_t signal_num, SignalCb cb) PURE; /** * Posts a functor to the dispatcher. This is safe cross thread. The functor runs in the context diff --git a/source/common/event/BUILD b/source/common/event/BUILD index 5e538bc52cf6..a992429ec8af 100644 --- a/source/common/event/BUILD +++ b/source/common/event/BUILD @@ -1,6 +1,9 @@ load( "//bazel:envoy_build_system.bzl", "envoy_cc_library", + "envoy_cc_platform_dep", + "envoy_cc_posix_library", + "envoy_cc_win32_library", "envoy_package", ) @@ -13,10 +16,6 @@ envoy_cc_library( srcs = [ "dispatcher_impl.cc", "file_event_impl.cc", - "signal_impl.cc", - ], - hdrs = [ - "signal_impl.h", ], deps = [ ":dispatcher_includes", @@ -37,7 +36,31 @@ envoy_cc_library( ] + select({ "//bazel:apple": ["//source/common/network:apple_dns_lib"], "//conditions:default": [], - }), + }) + envoy_cc_platform_dep("signal_impl_lib"), +) + +envoy_cc_posix_library( + name = "signal_impl_lib", + srcs = ["posix/signal_impl.cc"], + hdrs = ["posix/signal_impl.h"], + strip_include_prefix = "posix", + deps = [ + ":dispatcher_includes", + "//include/envoy/event:signal_interface", + "//source/common/common:thread_lib", + ], +) + +envoy_cc_win32_library( + name = "signal_impl_lib", + srcs = ["win32/signal_impl.cc"], + hdrs = ["win32/signal_impl.h"], + strip_include_prefix = "win32", + deps = [ + ":dispatcher_includes", + "//include/envoy/event:signal_interface", + "//source/common/common:thread_lib", + ], ) envoy_cc_library( diff --git a/source/common/event/dispatcher_impl.cc b/source/common/event/dispatcher_impl.cc index 803ff4558ad5..5cacc0d9fdbf 100644 --- a/source/common/event/dispatcher_impl.cc +++ b/source/common/event/dispatcher_impl.cc @@ -218,7 +218,7 @@ void DispatcherImpl::deferredDelete(DeferredDeletablePtr&& to_delete) { void DispatcherImpl::exit() { base_scheduler_.loopExit(); } -SignalEventPtr DispatcherImpl::listenForSignal(int signal_num, SignalCb cb) { +SignalEventPtr DispatcherImpl::listenForSignal(signat_t signal_num, SignalCb cb) { ASSERT(isThreadSafe()); return SignalEventPtr{new SignalEventImpl(*this, signal_num, cb)}; } diff --git a/source/common/event/dispatcher_impl.h b/source/common/event/dispatcher_impl.h index 62c10920d7fe..cb798c47d120 100644 --- a/source/common/event/dispatcher_impl.h +++ b/source/common/event/dispatcher_impl.h @@ -71,7 +71,7 @@ class DispatcherImpl : Logger::Loggable, Event::SchedulableCallbackPtr createSchedulableCallback(std::function cb) override; void deferredDelete(DeferredDeletablePtr&& to_delete) override; void exit() override; - SignalEventPtr listenForSignal(int signal_num, SignalCb cb) override; + SignalEventPtr listenForSignal(signat_t signal_num, SignalCb cb) override; void post(std::function callback) override; void run(RunType type) override; Buffer::WatermarkFactory& getWatermarkFactory() override { return *buffer_factory_; } diff --git a/source/common/event/signal_impl.cc b/source/common/event/posix/signal_impl.cc similarity index 81% rename from source/common/event/signal_impl.cc rename to source/common/event/posix/signal_impl.cc index bfd103f73e9e..25ba8344bdac 100644 --- a/source/common/event/signal_impl.cc +++ b/source/common/event/posix/signal_impl.cc @@ -7,7 +7,7 @@ namespace Envoy { namespace Event { -SignalEventImpl::SignalEventImpl(DispatcherImpl& dispatcher, int signal_num, SignalCb cb) +SignalEventImpl::SignalEventImpl(DispatcherImpl& dispatcher, signat_t signal_num, SignalCb cb) : cb_(cb) { evsignal_assign( &raw_event_, &dispatcher.base(), signal_num, diff --git a/source/common/event/signal_impl.h b/source/common/event/posix/signal_impl.h similarity index 81% rename from source/common/event/signal_impl.h rename to source/common/event/posix/signal_impl.h index 2e358834b5f3..3078289eb696 100644 --- a/source/common/event/signal_impl.h +++ b/source/common/event/posix/signal_impl.h @@ -5,6 +5,8 @@ #include "common/event/dispatcher_impl.h" #include "common/event/event_impl_base.h" + + namespace Envoy { namespace Event { @@ -13,11 +15,10 @@ namespace Event { */ class SignalEventImpl : public SignalEvent, ImplBase { public: - SignalEventImpl(DispatcherImpl& dispatcher, int signal_num, SignalCb cb); + SignalEventImpl(DispatcherImpl& dispatcher, signat_t signal_num, SignalCb cb); private: SignalCb cb_; }; - } // namespace Event } // namespace Envoy diff --git a/source/common/event/win32/signal_impl.cc b/source/common/event/win32/signal_impl.cc new file mode 100644 index 000000000000..826cf30414a4 --- /dev/null +++ b/source/common/event/win32/signal_impl.cc @@ -0,0 +1,40 @@ +#include "common/event/signal_impl.h" + +#include "common/event/dispatcher_impl.h" + +#include "event2/event.h" + +namespace Envoy { +namespace Event { + +SignalEventImpl::SignalEventImpl(DispatcherImpl& dispatcher, signat_t signal_num, SignalCb cb) + : cb_(cb) { + auto handler_it = eventBridgeHandlersSingleton::get().find(signal_num); + if (handler_it != eventBridgeHandlersSingleton::get().end()) { + ENVOY_BUG(false, fmt::format("Attempting to initialize event {} twice.", signal_num)); + return; + } + + os_fd_t socks[2]; + Api::SysCallIntResult result = Api::OsSysCallsSingleton::get().socketpair(AF_INET, SOCK_STREAM, IPPROTO_TCP, socks); + ASSERT(result.rc_ == 0); + + read_handle_ = std::make_unique(socks[0], false, AF_INET); + result = read_handle_->setBlocking(false); + ASSERT(result.rc_ == 0); + auto write_handle = std::make_shared(socks[1], false, AF_INET); + result = write_handle->setBlocking(false); + ASSERT(result.rc_ == 0); + + read_handle_->initializeFileEvent( + dispatcher, + [this](uint32_t events) -> void { + ASSERT(events == Event::FileReadyType::Read); + cb_(); + }, + Event::FileTriggerType::Level, Event::FileReadyType::Read); + eventBridgeHandlersSingleton::get().insert({signal_num, write_handle}); +} + +} // namespace Event +} // namespace Envoy diff --git a/source/common/event/win32/signal_impl.h b/source/common/event/win32/signal_impl.h new file mode 100644 index 000000000000..91870a113a9b --- /dev/null +++ b/source/common/event/win32/signal_impl.h @@ -0,0 +1,32 @@ +#pragma once + +#include "envoy/event/signal.h" + +#include "common/api/os_sys_calls_impl.h" +#include "common/event/dispatcher_impl.h" +#include "common/event/event_impl_base.h" +#include "common/network/io_socket_handle_impl.h" + +#include "envoy/network/io_handle.h" +#include "common/singleton/threadsafe_singleton.h" + +#include "absl/container/flat_hash_map.h" + +namespace Envoy { +namespace Event { + +/** + * libevent implementation of Event::SignalEvent. + */ +class SignalEventImpl : public SignalEvent, ImplBase { +public: + SignalEventImpl(DispatcherImpl& dispatcher, signat_t signal_num, SignalCb cb); + +private: + SignalCb cb_; + Network::IoHandlePtr read_handle_; +}; + +using eventBridgeHandlersSingleton = ThreadSafeSingleton>>; +} // namespace Event +} // namespace Envoy diff --git a/source/exe/BUILD b/source/exe/BUILD index 3edb3dd917c2..4c234ed183ce 100644 --- a/source/exe/BUILD +++ b/source/exe/BUILD @@ -170,6 +170,7 @@ envoy_cc_win32_library( ":platform_header_lib", "//source/common/common:assert_lib", "//source/common/common:thread_lib", + "//source/common/event:dispatcher_lib", "//source/common/filesystem:filesystem_lib", ], ) diff --git a/source/exe/win32/platform_impl.cc b/source/exe/win32/platform_impl.cc index ffedb3f478f6..5659156be029 100644 --- a/source/exe/win32/platform_impl.cc +++ b/source/exe/win32/platform_impl.cc @@ -1,17 +1,40 @@ #include "common/common/assert.h" #include "common/common/thread_impl.h" #include "common/filesystem/filesystem_impl.h" +#include "common/buffer/buffer_impl.h" +#include "common/event/signal_impl.h" #include "exe/platform_impl.h" namespace Envoy { +BOOL WINAPI CtrlHandler(DWORD fdwCtrlType) { + auto eventBridgeHandlers = Event::eventBridgeHandlersSingleton::get(); + auto handler_it = eventBridgeHandlers.find(fdwCtrlType); + if (handler_it == eventBridgeHandlers.end() || !handler_it->second) { + return 0; + } + + Buffer::OwnedImpl buffer; + constexpr absl::string_view data{"a"}; + buffer.add(data); + auto result = handler_it->second->write(buffer); + RELEASE_ASSERT(result.rc_ == 1, + fmt::format("failed to write 1 byte: {}", result.err_->getErrorDetails())); + return 1; +} + PlatformImpl::PlatformImpl() : thread_factory_(std::make_unique()), file_system_(std::make_unique()) { WSADATA wsa_data; const WORD version_requested = MAKEWORD(2, 2); RELEASE_ASSERT(WSAStartup(version_requested, &wsa_data) == 0, "WSAStartup failed with error"); + + if (!SetConsoleCtrlHandler(CtrlHandler, 1)) + { + ENVOY_LOG_MISC(warn, "Could not set Windows Control Handlers. Continuing without them."); + } } PlatformImpl::~PlatformImpl() { ::WSACleanup(); } diff --git a/source/server/server.cc b/source/server/server.cc index 3af9be2bc127..a99efae92c1b 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -53,6 +53,9 @@ #include "server/listener_hooks.h" #include "server/ssl_context_manager.h" +namespace { +constexpr bool platformSupportsSignal(signat_t signal_num) { return signal_num != -1; } +} // namespace namespace Envoy { namespace Server { @@ -614,28 +617,29 @@ RunHelper::RunHelper(Instance& instance, const Options& options, Event::Dispatch }) { // Setup signals. if (options.signalHandlingEnabled()) { -// TODO(Pivotal): Figure out solution to graceful shutdown on Windows. None of these signals exist -// on Windows. -#ifndef WIN32 - sigterm_ = dispatcher.listenForSignal(SIGTERM, [&instance]() { - ENVOY_LOG(warn, "caught SIGTERM"); - instance.shutdown(); - }); - - sigint_ = dispatcher.listenForSignal(SIGINT, [&instance]() { - ENVOY_LOG(warn, "caught SIGINT"); - instance.shutdown(); - }); - - sig_usr_1_ = dispatcher.listenForSignal(SIGUSR1, [&access_log_manager]() { - ENVOY_LOG(info, "caught SIGUSR1. Reopening access logs."); - access_log_manager.reopen(); - }); - - sig_hup_ = dispatcher.listenForSignal(SIGHUP, []() { - ENVOY_LOG(warn, "caught and eating SIGHUP. See documentation for how to hot restart."); - }); -#endif + if constexpr (platformSupportsSignal(ENVOY_SIGTERM)) { + sigterm_ = dispatcher.listenForSignal(ENVOY_SIGTERM, [&instance]() { + ENVOY_LOG(warn, "caught ENVOY_SIGTERM"); + instance.shutdown(); + }); + } + if constexpr (platformSupportsSignal(ENVOY_SIGINIT)) { + sigint_ = dispatcher.listenForSignal(ENVOY_SIGINIT, [&instance]() { + ENVOY_LOG(warn, "caught SIGINT"); + instance.shutdown(); + }); + } + if constexpr (platformSupportsSignal(ENVOY_SIGUSR1)) { + sig_usr_1_ = dispatcher.listenForSignal(ENVOY_SIGUSR1, [&access_log_manager]() { + ENVOY_LOG(info, "caught SIGUSR1. Reopening access logs."); + access_log_manager.reopen(); + }); + } + if constexpr (platformSupportsSignal(ENVOY_SIGHUP)) { + sig_hup_ = dispatcher.listenForSignal(ENVOY_SIGHUP, []() { + ENVOY_LOG(warn, "caught and eating SIGHUP. See documentation for how to hot restart."); + }); + } } // Start overload manager before workers. diff --git a/test/mocks/event/mocks.h b/test/mocks/event/mocks.h index 861f0a10e340..9ac7a906c4e1 100644 --- a/test/mocks/event/mocks.h +++ b/test/mocks/event/mocks.h @@ -97,7 +97,7 @@ class MockDispatcher : public Dispatcher { } } - SignalEventPtr listenForSignal(int signal_num, SignalCb cb) override { + SignalEventPtr listenForSignal(listenForSignal signal_num, SignalCb cb) override { return SignalEventPtr{listenForSignal_(signal_num, cb)}; } @@ -127,7 +127,7 @@ class MockDispatcher : public Dispatcher { MOCK_METHOD(SchedulableCallback*, createSchedulableCallback_, (std::function cb)); MOCK_METHOD(void, deferredDelete_, (DeferredDeletable * to_delete)); MOCK_METHOD(void, exit, ()); - MOCK_METHOD(SignalEvent*, listenForSignal_, (int signal_num, SignalCb cb)); + MOCK_METHOD(SignalEvent*, listenForSignal_, (listenForSignal signal_num, SignalCb cb)); MOCK_METHOD(void, post, (std::function callback)); MOCK_METHOD(void, run, (RunType type)); MOCK_METHOD(const ScopeTrackedObject*, setTrackedObject, (const ScopeTrackedObject* object)); diff --git a/test/mocks/event/wrapped_dispatcher.h b/test/mocks/event/wrapped_dispatcher.h index 74e935328984..c2b368a07331 100644 --- a/test/mocks/event/wrapped_dispatcher.h +++ b/test/mocks/event/wrapped_dispatcher.h @@ -88,7 +88,7 @@ class WrappedDispatcher : public Dispatcher { void exit() override { impl_.exit(); } - SignalEventPtr listenForSignal(int signal_num, SignalCb cb) override { + SignalEventPtr listenForSignal(listenForSignal signal_num, SignalCb cb) override { return impl_.listenForSignal(signal_num, std::move(cb)); } From ab8b2e2c8b8c3be8d272cd970d4f9cce471f08c5 Mon Sep 17 00:00:00 2001 From: Sotiris Nanopoulos Date: Fri, 6 Nov 2020 19:23:52 -0800 Subject: [PATCH 02/33] fix format 1 Signed-off-by: Sotiris Nanopoulos --- include/envoy/common/platform.h | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/include/envoy/common/platform.h b/include/envoy/common/platform.h index ef0558cf0e96..867be25bacd2 100644 --- a/include/envoy/common/platform.h +++ b/include/envoy/common/platform.h @@ -65,7 +65,7 @@ typedef uint32_t mode_t; typedef SOCKET os_fd_t; typedef HANDLE filesystem_os_id_t; // NOLINT(modernize-use-using) -typedef DWORD signat_t; // NOLINT(modernize-use-using) +typedef DWORD signat_t; // NOLINT(modernize-use-using) typedef unsigned int sa_family_t; @@ -157,7 +157,6 @@ struct msghdr { #define ENVOY_SIGUSR1 -1 #define ENVOY_SIGHUP -1 - namespace Platform { constexpr absl::string_view null_device_path{"NUL"}; } @@ -222,8 +221,7 @@ constexpr absl::string_view null_device_path{"NUL"}; typedef int os_fd_t; typedef int filesystem_os_id_t; // NOLINT(modernize-use-using) -typedef int signat_t; // NOLINT(modernize-use-using) - +typedef int signat_t; // NOLINT(modernize-use-using) #define INVALID_HANDLE -1 #define INVALID_SOCKET -1 From 2819000861dea9716052c854bfacefd05e592448 Mon Sep 17 00:00:00 2001 From: Sotiris Nanopoulos Date: Fri, 6 Nov 2020 19:26:31 -0800 Subject: [PATCH 03/33] format vol2 Signed-off-by: Sotiris Nanopoulos --- source/common/event/posix/signal_impl.cc | 3 +- source/common/event/posix/signal_impl.h | 2 -- source/common/event/win32/signal_impl.cc | 6 ++-- source/common/event/win32/signal_impl.h | 6 ++-- source/exe/win32/platform_impl.cc | 37 ++++++++++++------------ 5 files changed, 25 insertions(+), 29 deletions(-) diff --git a/source/common/event/posix/signal_impl.cc b/source/common/event/posix/signal_impl.cc index 25ba8344bdac..b26f3b4202e0 100644 --- a/source/common/event/posix/signal_impl.cc +++ b/source/common/event/posix/signal_impl.cc @@ -1,6 +1,5 @@ -#include "common/event/signal_impl.h" - #include "common/event/dispatcher_impl.h" +#include "common/event/signal_impl.h" #include "event2/event.h" diff --git a/source/common/event/posix/signal_impl.h b/source/common/event/posix/signal_impl.h index 3078289eb696..40970f68fb0b 100644 --- a/source/common/event/posix/signal_impl.h +++ b/source/common/event/posix/signal_impl.h @@ -5,8 +5,6 @@ #include "common/event/dispatcher_impl.h" #include "common/event/event_impl_base.h" - - namespace Envoy { namespace Event { diff --git a/source/common/event/win32/signal_impl.cc b/source/common/event/win32/signal_impl.cc index 826cf30414a4..51a45038268d 100644 --- a/source/common/event/win32/signal_impl.cc +++ b/source/common/event/win32/signal_impl.cc @@ -1,6 +1,5 @@ -#include "common/event/signal_impl.h" - #include "common/event/dispatcher_impl.h" +#include "common/event/signal_impl.h" #include "event2/event.h" @@ -16,7 +15,8 @@ SignalEventImpl::SignalEventImpl(DispatcherImpl& dispatcher, signat_t signal_num } os_fd_t socks[2]; - Api::SysCallIntResult result = Api::OsSysCallsSingleton::get().socketpair(AF_INET, SOCK_STREAM, IPPROTO_TCP, socks); + Api::SysCallIntResult result = + Api::OsSysCallsSingleton::get().socketpair(AF_INET, SOCK_STREAM, IPPROTO_TCP, socks); ASSERT(result.rc_ == 0); read_handle_ = std::make_unique(socks[0], false, AF_INET); diff --git a/source/common/event/win32/signal_impl.h b/source/common/event/win32/signal_impl.h index 91870a113a9b..9d125baa2295 100644 --- a/source/common/event/win32/signal_impl.h +++ b/source/common/event/win32/signal_impl.h @@ -1,13 +1,12 @@ #pragma once #include "envoy/event/signal.h" +#include "envoy/network/io_handle.h" #include "common/api/os_sys_calls_impl.h" #include "common/event/dispatcher_impl.h" #include "common/event/event_impl_base.h" #include "common/network/io_socket_handle_impl.h" - -#include "envoy/network/io_handle.h" #include "common/singleton/threadsafe_singleton.h" #include "absl/container/flat_hash_map.h" @@ -27,6 +26,7 @@ class SignalEventImpl : public SignalEvent, ImplBase { Network::IoHandlePtr read_handle_; }; -using eventBridgeHandlersSingleton = ThreadSafeSingleton>>; +using eventBridgeHandlersSingleton = + ThreadSafeSingleton>>; } // namespace Event } // namespace Envoy diff --git a/source/exe/win32/platform_impl.cc b/source/exe/win32/platform_impl.cc index 5659156be029..52b293fbe580 100644 --- a/source/exe/win32/platform_impl.cc +++ b/source/exe/win32/platform_impl.cc @@ -1,27 +1,27 @@ +#include "common/buffer/buffer_impl.h" #include "common/common/assert.h" #include "common/common/thread_impl.h" -#include "common/filesystem/filesystem_impl.h" -#include "common/buffer/buffer_impl.h" #include "common/event/signal_impl.h" +#include "common/filesystem/filesystem_impl.h" #include "exe/platform_impl.h" namespace Envoy { BOOL WINAPI CtrlHandler(DWORD fdwCtrlType) { - auto eventBridgeHandlers = Event::eventBridgeHandlersSingleton::get(); - auto handler_it = eventBridgeHandlers.find(fdwCtrlType); - if (handler_it == eventBridgeHandlers.end() || !handler_it->second) { - return 0; - } - - Buffer::OwnedImpl buffer; - constexpr absl::string_view data{"a"}; - buffer.add(data); - auto result = handler_it->second->write(buffer); - RELEASE_ASSERT(result.rc_ == 1, - fmt::format("failed to write 1 byte: {}", result.err_->getErrorDetails())); - return 1; + auto eventBridgeHandlers = Event::eventBridgeHandlersSingleton::get(); + auto handler_it = eventBridgeHandlers.find(fdwCtrlType); + if (handler_it == eventBridgeHandlers.end() || !handler_it->second) { + return 0; + } + + Buffer::OwnedImpl buffer; + constexpr absl::string_view data{"a"}; + buffer.add(data); + auto result = handler_it->second->write(buffer); + RELEASE_ASSERT(result.rc_ == 1, + fmt::format("failed to write 1 byte: {}", result.err_->getErrorDetails())); + return 1; } PlatformImpl::PlatformImpl() @@ -31,10 +31,9 @@ PlatformImpl::PlatformImpl() const WORD version_requested = MAKEWORD(2, 2); RELEASE_ASSERT(WSAStartup(version_requested, &wsa_data) == 0, "WSAStartup failed with error"); - if (!SetConsoleCtrlHandler(CtrlHandler, 1)) - { - ENVOY_LOG_MISC(warn, "Could not set Windows Control Handlers. Continuing without them."); - } + if (!SetConsoleCtrlHandler(CtrlHandler, 1)) { + ENVOY_LOG_MISC(warn, "Could not set Windows Control Handlers. Continuing without them."); + } } PlatformImpl::~PlatformImpl() { ::WSACleanup(); } From 3774592965810edbeb85dc6d65a504dc0c708680 Mon Sep 17 00:00:00 2001 From: Sotiris Nanopoulos Date: Mon, 9 Nov 2020 11:11:39 -0800 Subject: [PATCH 04/33] Win32 changes + tests Signed-off-by: Sotiris Nanopoulos --- source/common/event/BUILD | 22 ++++++++++++- source/common/event/win32/signal_impl.cc | 1 - source/exe/win32/platform_impl.cc | 6 ++-- test/exe/BUILD | 14 +++++++- test/exe/outofproc_main_test.py | 41 ++++++++++++++++++++++++ 5 files changed, 79 insertions(+), 5 deletions(-) create mode 100644 test/exe/outofproc_main_test.py diff --git a/source/common/event/BUILD b/source/common/event/BUILD index a992429ec8af..7919e68a36a1 100644 --- a/source/common/event/BUILD +++ b/source/common/event/BUILD @@ -17,10 +17,23 @@ envoy_cc_library( "dispatcher_impl.cc", "file_event_impl.cc", ], + hdrs = select({ + "//bazel:windows_x86_64": [ + "win32/signal_impl.h", + ], + "//conditions:default": [ + "posix/signal_impl.h", + ], + }), + strip_include_prefix = select({ + "//bazel:windows_x86_64": "win32", + "//conditions:default": "posix", + }), deps = [ ":dispatcher_includes", ":libevent_scheduler_lib", ":real_time_system_lib", + ":signal_lib", "//include/envoy/common:scope_tracker_interface", "//include/envoy/common:time_interface", "//include/envoy/event:signal_interface", @@ -36,7 +49,12 @@ envoy_cc_library( ] + select({ "//bazel:apple": ["//source/common/network:apple_dns_lib"], "//conditions:default": [], - }) + envoy_cc_platform_dep("signal_impl_lib"), + }), +) + +envoy_cc_library( + name = "signal_lib", + deps = envoy_cc_platform_dep("signal_impl_lib"), ) envoy_cc_posix_library( @@ -60,6 +78,8 @@ envoy_cc_win32_library( ":dispatcher_includes", "//include/envoy/event:signal_interface", "//source/common/common:thread_lib", + "//source/common/api:os_sys_calls_lib", + "//source/common/network:default_socket_interface_lib", ], ) diff --git a/source/common/event/win32/signal_impl.cc b/source/common/event/win32/signal_impl.cc index 51a45038268d..e2db2a168d2d 100644 --- a/source/common/event/win32/signal_impl.cc +++ b/source/common/event/win32/signal_impl.cc @@ -10,7 +10,6 @@ SignalEventImpl::SignalEventImpl(DispatcherImpl& dispatcher, signat_t signal_num : cb_(cb) { auto handler_it = eventBridgeHandlersSingleton::get().find(signal_num); if (handler_it != eventBridgeHandlersSingleton::get().end()) { - ENVOY_BUG(false, fmt::format("Attempting to initialize event {} twice.", signal_num)); return; } diff --git a/source/exe/win32/platform_impl.cc b/source/exe/win32/platform_impl.cc index 52b293fbe580..1769301e31ba 100644 --- a/source/exe/win32/platform_impl.cc +++ b/source/exe/win32/platform_impl.cc @@ -8,13 +8,15 @@ namespace Envoy { +static volatile bool shutdown_pending = false; + BOOL WINAPI CtrlHandler(DWORD fdwCtrlType) { auto eventBridgeHandlers = Event::eventBridgeHandlersSingleton::get(); auto handler_it = eventBridgeHandlers.find(fdwCtrlType); - if (handler_it == eventBridgeHandlers.end() || !handler_it->second) { + if (handler_it == eventBridgeHandlers.end() || !handler_it->second || shutdown_pending) { return 0; } - + shutdown_pending = true; Buffer::OwnedImpl buffer; constexpr absl::string_view data{"a"}; buffer.add(data); diff --git a/test/exe/BUILD b/test/exe/BUILD index 283086ab4799..821e158c8484 100644 --- a/test/exe/BUILD +++ b/test/exe/BUILD @@ -4,7 +4,7 @@ load( "envoy_package", "envoy_sh_test", ) - +load("@rules_python//python:defs.bzl", "py_test") licenses(["notice"]) # Apache 2 envoy_package() @@ -81,3 +81,15 @@ envoy_cc_test( "//test/test_common:utility_lib", ], ) + +py_test( + name = "outofproc_main_test", + srcs = [ + "outofproc_main_test.py", + ], + data = [ + "//source/exe:envoy-static", + ], + python_version = "PY3", + srcs_version = "PY3", +) diff --git a/test/exe/outofproc_main_test.py b/test/exe/outofproc_main_test.py new file mode 100644 index 000000000000..aeedbdd45f41 --- /dev/null +++ b/test/exe/outofproc_main_test.py @@ -0,0 +1,41 @@ + +#!/usr/bin/python + +import os +import time +import subprocess +import unittest + +class OutOfProcEnvoyTests(unittest.TestCase): + """ + We test that Envoy process behaves correctly. + """ + + @classmethod + def setUpClass(cls): + envoy_path = os.path.join(os.getenv('TEST_SRCDIR'), '/envoy/source/exe/envoy-static') + envoy_config = os.path.join(os.getenv('TEST_SRCDIR'), '/test/config/integration/server.yaml') + envoy_cl = envoy_path + " " + "--config-path" + " " + envoy_config + try: # Windows-specific. + creationflags = subprocess.CREATE_NEW_PROCESS_GROUP + except AttributeError: + creationflags = 0 + self.envoy_proc = subprocess.Popen(envoy_cl, stdin=subprocess.PIPE, creationflags=creationflags) + time.sleep(2) + + @classmethod + def tearDownClass(cls): + self.envoy_proc.kill() + + @classmethod + def VerifyEnvoyRunning(self): + self.assertTrue(self.envoy_proc.poll() is None) + + def test_envoy_closes_on_ctrl_c_event(self): + self.VerifyEnvoyRunning() + self.envoy_proc.send_signal(CTRL_C_EVENT) + poll = self.envoy_proc.poll() + self.assertTrue(poll != None) + +if __name__ == '__main__': + unittest.main() \ No newline at end of file From f55a38561e98c562403ac2e217c16a604e587ec2 Mon Sep 17 00:00:00 2001 From: Sotiris Nanopoulos Date: Mon, 9 Nov 2020 16:04:38 -0800 Subject: [PATCH 05/33] add tests Signed-off-by: Sotiris Nanopoulos --- test/exe/BUILD | 1 + test/exe/outofproc_main_test.py | 61 ++++++++++++++++++++++++++------- 2 files changed, 50 insertions(+), 12 deletions(-) diff --git a/test/exe/BUILD b/test/exe/BUILD index 821e158c8484..3ea3a6924c0a 100644 --- a/test/exe/BUILD +++ b/test/exe/BUILD @@ -88,6 +88,7 @@ py_test( "outofproc_main_test.py", ], data = [ + "//test/config/integration:google_com_proxy_port_0", "//source/exe:envoy-static", ], python_version = "PY3", diff --git a/test/exe/outofproc_main_test.py b/test/exe/outofproc_main_test.py index aeedbdd45f41..d07a8be40707 100644 --- a/test/exe/outofproc_main_test.py +++ b/test/exe/outofproc_main_test.py @@ -2,7 +2,11 @@ #!/usr/bin/python import os +import sys +import re import time +import tempfile +import signal import subprocess import unittest @@ -11,29 +15,62 @@ class OutOfProcEnvoyTests(unittest.TestCase): We test that Envoy process behaves correctly. """ + @staticmethod + def find_envoy(): + """ + This method locates envoy binary. + It's present at ./source/exe/envoy-static (at least for mac/bazel-asan/bazel-tsan), + or at ./external/envoy/source/exe/envoy-static (for bazel-compile_time_options). + """ + candidate = os.path.join('.', 'source', 'exe', 'envoy-static') + if os.path.isfile(candidate): + return candidate + candidate = os.path.join('.', 'external', 'envoy', 'source', 'exe', 'envoy-static') + if os.path.isfile(candidate): + return candidate + raise Exception("Could not find Envoy") + @classmethod - def setUpClass(cls): - envoy_path = os.path.join(os.getenv('TEST_SRCDIR'), '/envoy/source/exe/envoy-static') - envoy_config = os.path.join(os.getenv('TEST_SRCDIR'), '/test/config/integration/server.yaml') - envoy_cl = envoy_path + " " + "--config-path" + " " + envoy_config + def setUp(self): + envoy_path = OutOfProcEnvoyTests.find_envoy() + envoy_config = os.path.join('.', 'test/config/integration/google_com_proxy_port_0.yaml') + with open(envoy_config, 'r') as f: + content = f.read() + + + content = re.sub("{{ upstream_\d }}", "0", content) + content = content.replace("{{ ip_any_address }}", "127.0.0.1") + content = content.replace("{{ dns_lookup_family }}", "V4_ONLY") + if os.name == 'nt': + content = content.replace("{{ null_device_path }}", "/dev/null") + else: + content = content.replace("{{ null_device_path }}", "NUL") + + print(content) + envoy_config_new = os.path.join(os.getenv('TEST_TMPDIR'), 'google_com_proxy_port_0.yaml') + with open(envoy_config_new, "w") as f: + f.write(content) + try: # Windows-specific. creationflags = subprocess.CREATE_NEW_PROCESS_GROUP except AttributeError: creationflags = 0 - self.envoy_proc = subprocess.Popen(envoy_cl, stdin=subprocess.PIPE, creationflags=creationflags) + self.envoy_proc = subprocess.Popen([envoy_path, "--config-path", envoy_config_new], stdin=subprocess.PIPE, creationflags=creationflags) time.sleep(2) @classmethod - def tearDownClass(cls): + def tearDown(self): self.envoy_proc.kill() - @classmethod - def VerifyEnvoyRunning(self): - self.assertTrue(self.envoy_proc.poll() is None) - def test_envoy_closes_on_ctrl_c_event(self): - self.VerifyEnvoyRunning() - self.envoy_proc.send_signal(CTRL_C_EVENT) + self.assertTrue(self.envoy_proc.poll() == None) + ctr_c_signal = None + if sys.platform == 'win32': + ctr_c_signal = signal.CTRL_C_EVENT + else: + ctr_c_signal = signal.SIGINT + self.envoy_proc.send_signal(ctr_c_signal) + time.sleep(2) poll = self.envoy_proc.poll() self.assertTrue(poll != None) From f5ae7e0a1138b65b5af5d6de5b3e4c1867a5fe63 Mon Sep 17 00:00:00 2001 From: Sotiris Nanopoulos Date: Mon, 9 Nov 2020 16:11:02 -0800 Subject: [PATCH 06/33] fix format Signed-off-by: Sotiris Nanopoulos --- source/common/event/BUILD | 4 ++-- test/exe/BUILD | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/source/common/event/BUILD b/source/common/event/BUILD index 7919e68a36a1..fd0b0706f9d7 100644 --- a/source/common/event/BUILD +++ b/source/common/event/BUILD @@ -77,9 +77,9 @@ envoy_cc_win32_library( deps = [ ":dispatcher_includes", "//include/envoy/event:signal_interface", + "//source/common/api:os_sys_calls_lib", "//source/common/common:thread_lib", - "//source/common/api:os_sys_calls_lib", - "//source/common/network:default_socket_interface_lib", + "//source/common/network:default_socket_interface_lib", ], ) diff --git a/test/exe/BUILD b/test/exe/BUILD index 3ea3a6924c0a..0b1e0519f1eb 100644 --- a/test/exe/BUILD +++ b/test/exe/BUILD @@ -5,6 +5,7 @@ load( "envoy_sh_test", ) load("@rules_python//python:defs.bzl", "py_test") + licenses(["notice"]) # Apache 2 envoy_package() @@ -88,8 +89,8 @@ py_test( "outofproc_main_test.py", ], data = [ - "//test/config/integration:google_com_proxy_port_0", "//source/exe:envoy-static", + "//test/config/integration:google_com_proxy_port_0", ], python_version = "PY3", srcs_version = "PY3", From c5f267f7500176d51de5f1a9c8ee29edc3885a40 Mon Sep 17 00:00:00 2001 From: Sotiris Nanopoulos Date: Mon, 9 Nov 2020 16:13:06 -0800 Subject: [PATCH 07/33] fix pyformat Signed-off-by: Sotiris Nanopoulos --- test/exe/outofproc_main_test.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/test/exe/outofproc_main_test.py b/test/exe/outofproc_main_test.py index d07a8be40707..414e5d581280 100644 --- a/test/exe/outofproc_main_test.py +++ b/test/exe/outofproc_main_test.py @@ -1,4 +1,3 @@ - #!/usr/bin/python import os @@ -10,6 +9,7 @@ import subprocess import unittest + class OutOfProcEnvoyTests(unittest.TestCase): """ We test that Envoy process behaves correctly. @@ -37,7 +37,6 @@ def setUp(self): with open(envoy_config, 'r') as f: content = f.read() - content = re.sub("{{ upstream_\d }}", "0", content) content = content.replace("{{ ip_any_address }}", "127.0.0.1") content = content.replace("{{ dns_lookup_family }}", "V4_ONLY") @@ -55,9 +54,11 @@ def setUp(self): creationflags = subprocess.CREATE_NEW_PROCESS_GROUP except AttributeError: creationflags = 0 - self.envoy_proc = subprocess.Popen([envoy_path, "--config-path", envoy_config_new], stdin=subprocess.PIPE, creationflags=creationflags) + self.envoy_proc = subprocess.Popen([envoy_path, "--config-path", envoy_config_new], + stdin=subprocess.PIPE, + creationflags=creationflags) time.sleep(2) - + @classmethod def tearDown(self): self.envoy_proc.kill() @@ -74,5 +75,6 @@ def test_envoy_closes_on_ctrl_c_event(self): poll = self.envoy_proc.poll() self.assertTrue(poll != None) + if __name__ == '__main__': unittest.main() \ No newline at end of file From 4159136c54647a610f3f54156bbe18899e5cca99 Mon Sep 17 00:00:00 2001 From: Sotiris Nanopoulos Date: Tue, 10 Nov 2020 15:06:31 -0800 Subject: [PATCH 08/33] fix typo Signed-off-by: Sotiris Nanopoulos --- include/envoy/common/platform.h | 4 ++-- include/envoy/event/dispatcher.h | 2 +- source/common/event/dispatcher_impl.cc | 2 +- source/common/event/dispatcher_impl.h | 2 +- source/common/event/posix/signal_impl.cc | 2 +- source/common/event/posix/signal_impl.h | 2 +- source/common/event/win32/signal_impl.cc | 2 +- source/common/event/win32/signal_impl.h | 4 ++-- source/server/server.cc | 2 +- test/mocks/event/mocks.h | 4 ++-- 10 files changed, 13 insertions(+), 13 deletions(-) diff --git a/include/envoy/common/platform.h b/include/envoy/common/platform.h index 867be25bacd2..86b3270db448 100644 --- a/include/envoy/common/platform.h +++ b/include/envoy/common/platform.h @@ -65,7 +65,7 @@ typedef uint32_t mode_t; typedef SOCKET os_fd_t; typedef HANDLE filesystem_os_id_t; // NOLINT(modernize-use-using) -typedef DWORD signat_t; // NOLINT(modernize-use-using) +typedef DWORD signal_t; // NOLINT(modernize-use-using) typedef unsigned int sa_family_t; @@ -221,7 +221,7 @@ constexpr absl::string_view null_device_path{"NUL"}; typedef int os_fd_t; typedef int filesystem_os_id_t; // NOLINT(modernize-use-using) -typedef int signat_t; // NOLINT(modernize-use-using) +typedef int signal_t; // NOLINT(modernize-use-using) #define INVALID_HANDLE -1 #define INVALID_SOCKET -1 diff --git a/include/envoy/event/dispatcher.h b/include/envoy/event/dispatcher.h index 5467be4231ed..42048d138f68 100644 --- a/include/envoy/event/dispatcher.h +++ b/include/envoy/event/dispatcher.h @@ -205,7 +205,7 @@ class Dispatcher { * @param cb supplies the callback to invoke when the signal fires. * @return SignalEventPtr a signal event that is owned by the caller. */ - virtual SignalEventPtr listenForSignal(signat_t signal_num, SignalCb cb) PURE; + virtual SignalEventPtr listenForSignal(signal_t signal_num, SignalCb cb) PURE; /** * Posts a functor to the dispatcher. This is safe cross thread. The functor runs in the context diff --git a/source/common/event/dispatcher_impl.cc b/source/common/event/dispatcher_impl.cc index 5cacc0d9fdbf..7504aabc6714 100644 --- a/source/common/event/dispatcher_impl.cc +++ b/source/common/event/dispatcher_impl.cc @@ -218,7 +218,7 @@ void DispatcherImpl::deferredDelete(DeferredDeletablePtr&& to_delete) { void DispatcherImpl::exit() { base_scheduler_.loopExit(); } -SignalEventPtr DispatcherImpl::listenForSignal(signat_t signal_num, SignalCb cb) { +SignalEventPtr DispatcherImpl::listenForSignal(signal_t signal_num, SignalCb cb) { ASSERT(isThreadSafe()); return SignalEventPtr{new SignalEventImpl(*this, signal_num, cb)}; } diff --git a/source/common/event/dispatcher_impl.h b/source/common/event/dispatcher_impl.h index cb798c47d120..31653a98a302 100644 --- a/source/common/event/dispatcher_impl.h +++ b/source/common/event/dispatcher_impl.h @@ -71,7 +71,7 @@ class DispatcherImpl : Logger::Loggable, Event::SchedulableCallbackPtr createSchedulableCallback(std::function cb) override; void deferredDelete(DeferredDeletablePtr&& to_delete) override; void exit() override; - SignalEventPtr listenForSignal(signat_t signal_num, SignalCb cb) override; + SignalEventPtr listenForSignal(signal_t signal_num, SignalCb cb) override; void post(std::function callback) override; void run(RunType type) override; Buffer::WatermarkFactory& getWatermarkFactory() override { return *buffer_factory_; } diff --git a/source/common/event/posix/signal_impl.cc b/source/common/event/posix/signal_impl.cc index b26f3b4202e0..459e3f9b0dea 100644 --- a/source/common/event/posix/signal_impl.cc +++ b/source/common/event/posix/signal_impl.cc @@ -6,7 +6,7 @@ namespace Envoy { namespace Event { -SignalEventImpl::SignalEventImpl(DispatcherImpl& dispatcher, signat_t signal_num, SignalCb cb) +SignalEventImpl::SignalEventImpl(DispatcherImpl& dispatcher, signal_t signal_num, SignalCb cb) : cb_(cb) { evsignal_assign( &raw_event_, &dispatcher.base(), signal_num, diff --git a/source/common/event/posix/signal_impl.h b/source/common/event/posix/signal_impl.h index 40970f68fb0b..b2d6014b041a 100644 --- a/source/common/event/posix/signal_impl.h +++ b/source/common/event/posix/signal_impl.h @@ -13,7 +13,7 @@ namespace Event { */ class SignalEventImpl : public SignalEvent, ImplBase { public: - SignalEventImpl(DispatcherImpl& dispatcher, signat_t signal_num, SignalCb cb); + SignalEventImpl(DispatcherImpl& dispatcher, signal_t signal_num, SignalCb cb); private: SignalCb cb_; diff --git a/source/common/event/win32/signal_impl.cc b/source/common/event/win32/signal_impl.cc index e2db2a168d2d..35597294c3de 100644 --- a/source/common/event/win32/signal_impl.cc +++ b/source/common/event/win32/signal_impl.cc @@ -6,7 +6,7 @@ namespace Envoy { namespace Event { -SignalEventImpl::SignalEventImpl(DispatcherImpl& dispatcher, signat_t signal_num, SignalCb cb) +SignalEventImpl::SignalEventImpl(DispatcherImpl& dispatcher, signal_t signal_num, SignalCb cb) : cb_(cb) { auto handler_it = eventBridgeHandlersSingleton::get().find(signal_num); if (handler_it != eventBridgeHandlersSingleton::get().end()) { diff --git a/source/common/event/win32/signal_impl.h b/source/common/event/win32/signal_impl.h index 9d125baa2295..9d18839ba646 100644 --- a/source/common/event/win32/signal_impl.h +++ b/source/common/event/win32/signal_impl.h @@ -19,7 +19,7 @@ namespace Event { */ class SignalEventImpl : public SignalEvent, ImplBase { public: - SignalEventImpl(DispatcherImpl& dispatcher, signat_t signal_num, SignalCb cb); + SignalEventImpl(DispatcherImpl& dispatcher, signal_t signal_num, SignalCb cb); private: SignalCb cb_; @@ -27,6 +27,6 @@ class SignalEventImpl : public SignalEvent, ImplBase { }; using eventBridgeHandlersSingleton = - ThreadSafeSingleton>>; + ThreadSafeSingleton>>; } // namespace Event } // namespace Envoy diff --git a/source/server/server.cc b/source/server/server.cc index a99efae92c1b..559329744613 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -54,7 +54,7 @@ #include "server/ssl_context_manager.h" namespace { -constexpr bool platformSupportsSignal(signat_t signal_num) { return signal_num != -1; } +constexpr bool platformSupportsSignal(signal_t signal_num) { return signal_num != -1; } } // namespace namespace Envoy { namespace Server { diff --git a/test/mocks/event/mocks.h b/test/mocks/event/mocks.h index 9ac7a906c4e1..97c0165b820c 100644 --- a/test/mocks/event/mocks.h +++ b/test/mocks/event/mocks.h @@ -97,7 +97,7 @@ class MockDispatcher : public Dispatcher { } } - SignalEventPtr listenForSignal(listenForSignal signal_num, SignalCb cb) override { + SignalEventPtr listenForSignal(signal_t signal_num, SignalCb cb) override { return SignalEventPtr{listenForSignal_(signal_num, cb)}; } @@ -127,7 +127,7 @@ class MockDispatcher : public Dispatcher { MOCK_METHOD(SchedulableCallback*, createSchedulableCallback_, (std::function cb)); MOCK_METHOD(void, deferredDelete_, (DeferredDeletable * to_delete)); MOCK_METHOD(void, exit, ()); - MOCK_METHOD(SignalEvent*, listenForSignal_, (listenForSignal signal_num, SignalCb cb)); + MOCK_METHOD(SignalEvent*, listenForSignal_, (signal_t signal_num, SignalCb cb)); MOCK_METHOD(void, post, (std::function callback)); MOCK_METHOD(void, run, (RunType type)); MOCK_METHOD(const ScopeTrackedObject*, setTrackedObject, (const ScopeTrackedObject* object)); From 709cbac38527dec762273a59ffa80e95c78e1bfd Mon Sep 17 00:00:00 2001 From: Sotiris Nanopoulos Date: Tue, 10 Nov 2020 15:57:49 -0800 Subject: [PATCH 09/33] working through build errors Signed-off-by: Sotiris Nanopoulos --- test/mocks/event/wrapped_dispatcher.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/mocks/event/wrapped_dispatcher.h b/test/mocks/event/wrapped_dispatcher.h index c2b368a07331..974d61b39be2 100644 --- a/test/mocks/event/wrapped_dispatcher.h +++ b/test/mocks/event/wrapped_dispatcher.h @@ -88,7 +88,7 @@ class WrappedDispatcher : public Dispatcher { void exit() override { impl_.exit(); } - SignalEventPtr listenForSignal(listenForSignal signal_num, SignalCb cb) override { + SignalEventPtr listenForSignal(signal_t signal_num, SignalCb cb) override { return impl_.listenForSignal(signal_num, std::move(cb)); } From 3158e46c1181f86eb3ebb75196455f90163f1a54 Mon Sep 17 00:00:00 2001 From: Sotiris Nanopoulos Date: Tue, 10 Nov 2020 21:27:51 -0800 Subject: [PATCH 10/33] fix with runfiles Signed-off-by: Sotiris Nanopoulos --- test/exe/BUILD | 1 + test/exe/outofproc_main_test.py | 45 ++++++++++++++------------------- 2 files changed, 20 insertions(+), 26 deletions(-) diff --git a/test/exe/BUILD b/test/exe/BUILD index 0b1e0519f1eb..2649602bb5d1 100644 --- a/test/exe/BUILD +++ b/test/exe/BUILD @@ -89,6 +89,7 @@ py_test( "outofproc_main_test.py", ], data = [ + "@rules_python//python/runfiles", "//source/exe:envoy-static", "//test/config/integration:google_com_proxy_port_0", ], diff --git a/test/exe/outofproc_main_test.py b/test/exe/outofproc_main_test.py index 414e5d581280..320c35d08c7a 100644 --- a/test/exe/outofproc_main_test.py +++ b/test/exe/outofproc_main_test.py @@ -8,44 +8,35 @@ import signal import subprocess import unittest - +from rules_python.python.runfiles import runfiles class OutOfProcEnvoyTests(unittest.TestCase): """ We test that Envoy process behaves correctly. """ - @staticmethod - def find_envoy(): - """ - This method locates envoy binary. - It's present at ./source/exe/envoy-static (at least for mac/bazel-asan/bazel-tsan), - or at ./external/envoy/source/exe/envoy-static (for bazel-compile_time_options). - """ - candidate = os.path.join('.', 'source', 'exe', 'envoy-static') - if os.path.isfile(candidate): - return candidate - candidate = os.path.join('.', 'external', 'envoy', 'source', 'exe', 'envoy-static') - if os.path.isfile(candidate): - return candidate - raise Exception("Could not find Envoy") - @classmethod def setUp(self): - envoy_path = OutOfProcEnvoyTests.find_envoy() - envoy_config = os.path.join('.', 'test/config/integration/google_com_proxy_port_0.yaml') + r = runfiles.Create() + envoy_path = None + if sys.platform == 'win32': + envoy_path = r.Rlocation("envoy/source/exe/envoy-static.exe") + else: + envoy_path = r.Rlocation("envoy/source/exe/envoy-static") + if not os.path.isfile(envoy_path): + raise Exception("Could not find Envoy") + + envoy_config = r.Rlocation('envoy/test/config/integration/google_com_proxy_port_0.yaml') with open(envoy_config, 'r') as f: content = f.read() - content = re.sub("{{ upstream_\d }}", "0", content) content = content.replace("{{ ip_any_address }}", "127.0.0.1") content = content.replace("{{ dns_lookup_family }}", "V4_ONLY") - if os.name == 'nt': + if os.name != 'nt': content = content.replace("{{ null_device_path }}", "/dev/null") else: content = content.replace("{{ null_device_path }}", "NUL") - print(content) envoy_config_new = os.path.join(os.getenv('TEST_TMPDIR'), 'google_com_proxy_port_0.yaml') with open(envoy_config_new, "w") as f: f.write(content) @@ -63,14 +54,16 @@ def setUp(self): def tearDown(self): self.envoy_proc.kill() - def test_envoy_closes_on_ctrl_c_event(self): + def test_envoy_closes_on_ctrl_break_event(self): self.assertTrue(self.envoy_proc.poll() == None) - ctr_c_signal = None + ctrl_break_signal = None if sys.platform == 'win32': - ctr_c_signal = signal.CTRL_C_EVENT + # On Windows sending a ctrl + c signal is quite tricky + # so we test that Envoy stops on ctrl + break. + ctrl_break_signal = signal.CTRL_BREAK_EVENT else: - ctr_c_signal = signal.SIGINT - self.envoy_proc.send_signal(ctr_c_signal) + ctrl_break_signal = signal.SIGTERM + self.envoy_proc.send_signal(ctrl_break_signal) time.sleep(2) poll = self.envoy_proc.poll() self.assertTrue(poll != None) From 2a163096feff04aaaaad48b899f7d096e0f5a261 Mon Sep 17 00:00:00 2001 From: Sotiris Nanopoulos Date: Tue, 10 Nov 2020 21:29:17 -0800 Subject: [PATCH 11/33] use the same if conditions everywhere Signed-off-by: Sotiris Nanopoulos --- test/exe/outofproc_main_test.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/exe/outofproc_main_test.py b/test/exe/outofproc_main_test.py index 320c35d08c7a..d70847da1034 100644 --- a/test/exe/outofproc_main_test.py +++ b/test/exe/outofproc_main_test.py @@ -32,10 +32,11 @@ def setUp(self): content = content.replace("{{ ip_any_address }}", "127.0.0.1") content = content.replace("{{ dns_lookup_family }}", "V4_ONLY") - if os.name != 'nt': - content = content.replace("{{ null_device_path }}", "/dev/null") - else: + + if sys.platform == 'win32': content = content.replace("{{ null_device_path }}", "NUL") + else: + content = content.replace("{{ null_device_path }}", "/dev/null") envoy_config_new = os.path.join(os.getenv('TEST_TMPDIR'), 'google_com_proxy_port_0.yaml') with open(envoy_config_new, "w") as f: From e9fde7feb9891ecac30c199d804ced22e8ca74ff Mon Sep 17 00:00:00 2001 From: Sotiris Nanopoulos Date: Wed, 11 Nov 2020 09:26:53 -0800 Subject: [PATCH 12/33] fix format + deps Signed-off-by: Sotiris Nanopoulos --- test/exe/BUILD | 4 ++-- test/exe/outofproc_main_test.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/exe/BUILD b/test/exe/BUILD index 2649602bb5d1..efd3ab3162e0 100644 --- a/test/exe/BUILD +++ b/test/exe/BUILD @@ -4,7 +4,7 @@ load( "envoy_package", "envoy_sh_test", ) -load("@rules_python//python:defs.bzl", "py_test") +load("@rules_python//python:defs.bzl", "@rules_python//python/runfiles", "py_test") licenses(["notice"]) # Apache 2 @@ -89,9 +89,9 @@ py_test( "outofproc_main_test.py", ], data = [ - "@rules_python//python/runfiles", "//source/exe:envoy-static", "//test/config/integration:google_com_proxy_port_0", + "@rules_python//python/runfiles", ], python_version = "PY3", srcs_version = "PY3", diff --git a/test/exe/outofproc_main_test.py b/test/exe/outofproc_main_test.py index d70847da1034..2ac44165bb57 100644 --- a/test/exe/outofproc_main_test.py +++ b/test/exe/outofproc_main_test.py @@ -32,7 +32,7 @@ def setUp(self): content = content.replace("{{ ip_any_address }}", "127.0.0.1") content = content.replace("{{ dns_lookup_family }}", "V4_ONLY") - + if sys.platform == 'win32': content = content.replace("{{ null_device_path }}", "NUL") else: @@ -71,4 +71,4 @@ def test_envoy_closes_on_ctrl_break_event(self): if __name__ == '__main__': - unittest.main() \ No newline at end of file + unittest.main() From 598b93344e0decaa6832ce2564dd2e7d6d43a7a2 Mon Sep 17 00:00:00 2001 From: Sotiris Nanopoulos Date: Tue, 17 Nov 2020 21:12:36 -0800 Subject: [PATCH 13/33] PR comments and robust testing Signed-off-by: Sotiris Nanopoulos --- ci/windows_ci_steps.sh | 1 - include/envoy/common/platform.h | 10 +-- source/exe/win32/platform_impl.cc | 22 ++++++- source/server/server.cc | 48 +++++++------- test/exe/BUILD | 28 +++++---- test/exe/outofproc_main_test.py | 74 ---------------------- test/exe/win32_outofproc_main_test.cc | 91 +++++++++++++++++++++++++++ 7 files changed, 153 insertions(+), 121 deletions(-) delete mode 100644 test/exe/outofproc_main_test.py create mode 100644 test/exe/win32_outofproc_main_test.cc diff --git a/ci/windows_ci_steps.sh b/ci/windows_ci_steps.sh index ff77a9ea1465..b6008fee888f 100755 --- a/ci/windows_ci_steps.sh +++ b/ci/windows_ci_steps.sh @@ -47,7 +47,6 @@ BAZEL_BUILD_OPTIONS=( -c opt --show_task_finish --verbose_failures - --define "wasm=disabled" "--test_output=errors" "${BAZEL_BUILD_EXTRA_OPTIONS[@]}" "${BAZEL_EXTRA_TEST_OPTIONS[@]}") diff --git a/include/envoy/common/platform.h b/include/envoy/common/platform.h index 86b3270db448..c1fd99b0bd99 100644 --- a/include/envoy/common/platform.h +++ b/include/envoy/common/platform.h @@ -152,10 +152,7 @@ struct msghdr { #define HANDLE_ERROR_PERM ERROR_ACCESS_DENIED #define HANDLE_ERROR_INVALID ERROR_INVALID_HANDLE -#define ENVOY_SIGINIT CTRL_C_EVENT -#define ENVOY_SIGTERM CTRL_CLOSE_EVENT -#define ENVOY_SIGUSR1 -1 -#define ENVOY_SIGHUP -1 +#define ENVOY_WIN32_SIGTERM 1 namespace Platform { constexpr absl::string_view null_device_path{"NUL"}; @@ -252,11 +249,6 @@ typedef int signal_t; // NOLINT(modernize-use-using) #define HANDLE_ERROR_PERM EACCES #define HANDLE_ERROR_INVALID EBADF -#define ENVOY_SIGTERM SIGTERM -#define ENVOY_SIGINIT SIGINT -#define ENVOY_SIGUSR1 SIGUSR1 -#define ENVOY_SIGHUP SIGHUP - namespace Platform { constexpr absl::string_view null_device_path{"/dev/null"}; } diff --git a/source/exe/win32/platform_impl.cc b/source/exe/win32/platform_impl.cc index 1769301e31ba..d3dfa340d974 100644 --- a/source/exe/win32/platform_impl.cc +++ b/source/exe/win32/platform_impl.cc @@ -1,3 +1,6 @@ +#include +#include + #include "common/buffer/buffer_impl.h" #include "common/common/assert.h" #include "common/common/thread_impl.h" @@ -11,18 +14,30 @@ namespace Envoy { static volatile bool shutdown_pending = false; BOOL WINAPI CtrlHandler(DWORD fdwCtrlType) { - auto eventBridgeHandlers = Event::eventBridgeHandlersSingleton::get(); - auto handler_it = eventBridgeHandlers.find(fdwCtrlType); - if (handler_it == eventBridgeHandlers.end() || !handler_it->second || shutdown_pending) { + if (shutdown_pending) { return 0; } shutdown_pending = true; + + auto eventBridgeHandlers = Event::eventBridgeHandlersSingleton::get(); + auto handler_it = eventBridgeHandlers.find(ENVOY_WIN32_SIGTERM); + if (handler_it == eventBridgeHandlers.end() || !handler_it->second) { + return 0; + } Buffer::OwnedImpl buffer; constexpr absl::string_view data{"a"}; buffer.add(data); auto result = handler_it->second->write(buffer); RELEASE_ASSERT(result.rc_ == 1, fmt::format("failed to write 1 byte: {}", result.err_->getErrorDetails())); + + if (fdwCtrlType == CTRL_LOGOFF_EVENT || fdwCtrlType == CTRL_SHUTDOWN_EVENT ) { + // These events terminate the process immediatelly so we want to give a couple of seconds + // to the dispatcher to shutdown the server. + constexpr size_t delay = 3; + std::chrono::seconds sec(delay); + std::this_thread::sleep_for(sec); + } return 1; } @@ -34,6 +49,7 @@ PlatformImpl::PlatformImpl() RELEASE_ASSERT(WSAStartup(version_requested, &wsa_data) == 0, "WSAStartup failed with error"); if (!SetConsoleCtrlHandler(CtrlHandler, 1)) { + // The Control Handler is executing in a different thread. ENVOY_LOG_MISC(warn, "Could not set Windows Control Handlers. Continuing without them."); } } diff --git a/source/server/server.cc b/source/server/server.cc index 559329744613..7910149719f2 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -617,29 +617,31 @@ RunHelper::RunHelper(Instance& instance, const Options& options, Event::Dispatch }) { // Setup signals. if (options.signalHandlingEnabled()) { - if constexpr (platformSupportsSignal(ENVOY_SIGTERM)) { - sigterm_ = dispatcher.listenForSignal(ENVOY_SIGTERM, [&instance]() { - ENVOY_LOG(warn, "caught ENVOY_SIGTERM"); - instance.shutdown(); - }); - } - if constexpr (platformSupportsSignal(ENVOY_SIGINIT)) { - sigint_ = dispatcher.listenForSignal(ENVOY_SIGINIT, [&instance]() { - ENVOY_LOG(warn, "caught SIGINT"); - instance.shutdown(); - }); - } - if constexpr (platformSupportsSignal(ENVOY_SIGUSR1)) { - sig_usr_1_ = dispatcher.listenForSignal(ENVOY_SIGUSR1, [&access_log_manager]() { - ENVOY_LOG(info, "caught SIGUSR1. Reopening access logs."); - access_log_manager.reopen(); - }); - } - if constexpr (platformSupportsSignal(ENVOY_SIGHUP)) { - sig_hup_ = dispatcher.listenForSignal(ENVOY_SIGHUP, []() { - ENVOY_LOG(warn, "caught and eating SIGHUP. See documentation for how to hot restart."); - }); - } +#ifdef WIN32 + sigterm_ = dispatcher.listenForSignal(ENVOY_WIN32_SIGTERM, [&instance]() { + ENVOY_LOG(warn, "caught ENVOY_WIN32_SIGTERM"); + instance.shutdown(); + }); +#else + sigterm_ = dispatcher.listenForSignal(ENVOY_SIGTERM, [&instance]() { + ENVOY_LOG(warn, "caught ENVOY_SIGTERM"); + instance.shutdown(); + }); + + sigint_ = dispatcher.listenForSignal(ENVOY_SIGINIT, [&instance]() { + ENVOY_LOG(warn, "caught SIGINT"); + instance.shutdown(); + }); + + sig_usr_1_ = dispatcher.listenForSignal(ENVOY_SIGUSR1, [&access_log_manager]() { + ENVOY_LOG(info, "caught SIGUSR1. Reopening access logs."); + access_log_manager.reopen(); + }); + + sig_hup_ = dispatcher.listenForSignal(ENVOY_SIGHUP, []() { + ENVOY_LOG(warn, "caught and eating SIGHUP. See documentation for how to hot restart."); + }); +#endif } // Start overload manager before workers. diff --git a/test/exe/BUILD b/test/exe/BUILD index efd3ab3162e0..4bdf6af15616 100644 --- a/test/exe/BUILD +++ b/test/exe/BUILD @@ -4,7 +4,6 @@ load( "envoy_package", "envoy_sh_test", ) -load("@rules_python//python:defs.bzl", "@rules_python//python/runfiles", "py_test") licenses(["notice"]) # Apache 2 @@ -83,16 +82,23 @@ envoy_cc_test( ], ) -py_test( - name = "outofproc_main_test", - srcs = [ - "outofproc_main_test.py", - ], +# Due to the limitiations of how Windows signals work +# this test cannot be executed through bazel and it must +# be executed as a standalone executable. +# e.g.: `./bazel-bin/test/exe/win32_outofproc_main_test.exe` +envoy_cc_test( + name = "win32_outofproc_main_test", + srcs = ["win32_outofproc_main_test.cc"], + tags = ["manual"], data = [ "//source/exe:envoy-static", - "//test/config/integration:google_com_proxy_port_0", - "@rules_python//python/runfiles", + "//test/config/integration:google_com_proxy_port_0" ], - python_version = "PY3", - srcs_version = "PY3", -) + deps = [ + "//source/common/api:api_lib", + "//source/exe:main_common_lib", + "//test/mocks/runtime:runtime_mocks", + "//test/test_common:contention_lib", + "//test/test_common:environment_lib", + ], +) \ No newline at end of file diff --git a/test/exe/outofproc_main_test.py b/test/exe/outofproc_main_test.py deleted file mode 100644 index 2ac44165bb57..000000000000 --- a/test/exe/outofproc_main_test.py +++ /dev/null @@ -1,74 +0,0 @@ -#!/usr/bin/python - -import os -import sys -import re -import time -import tempfile -import signal -import subprocess -import unittest -from rules_python.python.runfiles import runfiles - -class OutOfProcEnvoyTests(unittest.TestCase): - """ - We test that Envoy process behaves correctly. - """ - - @classmethod - def setUp(self): - r = runfiles.Create() - envoy_path = None - if sys.platform == 'win32': - envoy_path = r.Rlocation("envoy/source/exe/envoy-static.exe") - else: - envoy_path = r.Rlocation("envoy/source/exe/envoy-static") - if not os.path.isfile(envoy_path): - raise Exception("Could not find Envoy") - - envoy_config = r.Rlocation('envoy/test/config/integration/google_com_proxy_port_0.yaml') - with open(envoy_config, 'r') as f: - content = f.read() - - content = content.replace("{{ ip_any_address }}", "127.0.0.1") - content = content.replace("{{ dns_lookup_family }}", "V4_ONLY") - - if sys.platform == 'win32': - content = content.replace("{{ null_device_path }}", "NUL") - else: - content = content.replace("{{ null_device_path }}", "/dev/null") - - envoy_config_new = os.path.join(os.getenv('TEST_TMPDIR'), 'google_com_proxy_port_0.yaml') - with open(envoy_config_new, "w") as f: - f.write(content) - - try: # Windows-specific. - creationflags = subprocess.CREATE_NEW_PROCESS_GROUP - except AttributeError: - creationflags = 0 - self.envoy_proc = subprocess.Popen([envoy_path, "--config-path", envoy_config_new], - stdin=subprocess.PIPE, - creationflags=creationflags) - time.sleep(2) - - @classmethod - def tearDown(self): - self.envoy_proc.kill() - - def test_envoy_closes_on_ctrl_break_event(self): - self.assertTrue(self.envoy_proc.poll() == None) - ctrl_break_signal = None - if sys.platform == 'win32': - # On Windows sending a ctrl + c signal is quite tricky - # so we test that Envoy stops on ctrl + break. - ctrl_break_signal = signal.CTRL_BREAK_EVENT - else: - ctrl_break_signal = signal.SIGTERM - self.envoy_proc.send_signal(ctrl_break_signal) - time.sleep(2) - poll = self.envoy_proc.poll() - self.assertTrue(poll != None) - - -if __name__ == '__main__': - unittest.main() diff --git a/test/exe/win32_outofproc_main_test.cc b/test/exe/win32_outofproc_main_test.cc new file mode 100644 index 000000000000..2b7e0d2ecb65 --- /dev/null +++ b/test/exe/win32_outofproc_main_test.cc @@ -0,0 +1,91 @@ +#include "test/test_common/utility.h" +#include "test/test_common/environment.h" +#include "gtest/gtest.h" + +#include + +namespace Envoy { + +#ifdef WIN32 +class MainCommonTest : public testing::TestWithParam { +protected: + MainCommonTest() + : config_file_(TestEnvironment::temporaryFileSubstitute( + "test/config/integration/google_com_proxy_port_0.yaml", TestEnvironment::ParamMap(), + TestEnvironment::PortMap(), GetParam())), + envoy_exe_(TestEnvironment::runfilesPath("source/exe/envoy-static.exe")) {} + + void cleanup() { + UINT uExitCode = 0; + TerminateProcess(piProcInfo_.hProcess, uExitCode); + remove(log_path_.c_str()); + } + + void sendSingal(DWORD pid, DWORD signal) { + EXPECT_TRUE(FreeConsole()) << fmt::format("Failed to FreeConsole, error {}", GetLastError()); + EXPECT_TRUE(AttachConsole(pid)) + << fmt::format("Failed to AttachConsole, error {}", GetLastError()); + EXPECT_TRUE(SetConsoleCtrlHandler(NULL, 1)) + << fmt::format("Failed to SetConsoleCtrlHandler, error {}", GetLastError()); + EXPECT_TRUE(GenerateConsoleCtrlEvent(signal, pid)) + << fmt::format("Failed to GenerateConsoleCtrlEvent, error {}", GetLastError()); + } + + void createEnvoyProcess() { + STARTUPINFO siStartInfo; + ZeroMemory(&siStartInfo, sizeof(siStartInfo)); + log_path_ = TestEnvironment::temporaryDirectory() + "/output.txt"; + ZeroMemory(&piProcInfo_, sizeof(PROCESS_INFORMATION)); + auto commandLine = fmt::format("{} --config-path {} -l warn --log-path {}", envoy_exe_, + config_file_, log_path_); + ENVOY_LOG_MISC(warn, commandLine); + BOOL bSuccess = CreateProcessA(NULL, + const_cast(commandLine.c_str()), // command line + NULL, // process security attributes + NULL, // primary thread security attributes + 1, // handles are inherited + CREATE_NEW_PROCESS_GROUP, // creation flags + NULL, // use parent's environment + NULL, // use parent's current directory + &siStartInfo, // STARTUPINFO pointer + &piProcInfo_); // receives PROCESS_INFORMATION + EXPECT_TRUE(bSuccess) << fmt::format("Failed to create Envoy process, error {}", + GetLastError()); + } + + std::string config_file_; + std::string envoy_exe_; + PROCESS_INFORMATION piProcInfo_; + std::string log_path_; +}; + +INSTANTIATE_TEST_SUITE_P(IpVersions, MainCommonTest, + testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), + TestUtility::ipTestParamsToString); + +TEST_P(MainCommonTest, Envoy1) { + createEnvoyProcess(); + ENVOY_LOG_MISC(warn, "Envoy process with pid {}", piProcInfo_.dwProcessId); + Sleep(2 * 1000); + sendSingal(piProcInfo_.dwProcessId, CTRL_BREAK_EVENT); + size_t total_sleep = 1; + constexpr size_t resonable_sleep_time = 5; + DWORD exitCode; + do { + Sleep(total_sleep * 1000); + ++total_sleep; + GetExitCodeProcess(piProcInfo_.hProcess, &exitCode); + } while (exitCode == STILL_ACTIVE && total_sleep <= resonable_sleep_time); + EXPECT_TRUE(total_sleep <= resonable_sleep_time); + auto output = TestEnvironment::readFileToStringForTest(log_path_); + size_t count = 0; + for (size_t pos = 0; (pos = output.find("ENVOY_WIN32_SIGTERM", pos)) != std::string::npos; + ++pos, ++count) { + } + EXPECT_EQ(1, count); + ENVOY_LOG_MISC(warn, "Envoy output {}", output); + cleanup(); +} +#endif + +} // namespace Envoy \ No newline at end of file From 8cce5c920dd7b60c04218689df1de4d6f9b83885 Mon Sep 17 00:00:00 2001 From: Sotiris Nanopoulos Date: Tue, 17 Nov 2020 21:33:11 -0800 Subject: [PATCH 14/33] format changes Signed-off-by: Sotiris Nanopoulos --- source/exe/win32/platform_impl.cc | 8 ++++---- test/exe/BUILD | 6 +++--- test/exe/win32_outofproc_main_test.cc | 16 ++++++++-------- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/source/exe/win32/platform_impl.cc b/source/exe/win32/platform_impl.cc index d3dfa340d974..2fabcda19002 100644 --- a/source/exe/win32/platform_impl.cc +++ b/source/exe/win32/platform_impl.cc @@ -18,7 +18,7 @@ BOOL WINAPI CtrlHandler(DWORD fdwCtrlType) { return 0; } shutdown_pending = true; - + auto eventBridgeHandlers = Event::eventBridgeHandlersSingleton::get(); auto handler_it = eventBridgeHandlers.find(ENVOY_WIN32_SIGTERM); if (handler_it == eventBridgeHandlers.end() || !handler_it->second) { @@ -30,9 +30,9 @@ BOOL WINAPI CtrlHandler(DWORD fdwCtrlType) { auto result = handler_it->second->write(buffer); RELEASE_ASSERT(result.rc_ == 1, fmt::format("failed to write 1 byte: {}", result.err_->getErrorDetails())); - - if (fdwCtrlType == CTRL_LOGOFF_EVENT || fdwCtrlType == CTRL_SHUTDOWN_EVENT ) { - // These events terminate the process immediatelly so we want to give a couple of seconds + + if (fdwCtrlType == CTRL_LOGOFF_EVENT || fdwCtrlType == CTRL_SHUTDOWN_EVENT) { + // These events terminate the process immediately so we want to give a couple of seconds // to the dispatcher to shutdown the server. constexpr size_t delay = 3; std::chrono::seconds sec(delay); diff --git a/test/exe/BUILD b/test/exe/BUILD index 4bdf6af15616..1656b7f2c1e5 100644 --- a/test/exe/BUILD +++ b/test/exe/BUILD @@ -89,11 +89,11 @@ envoy_cc_test( envoy_cc_test( name = "win32_outofproc_main_test", srcs = ["win32_outofproc_main_test.cc"], - tags = ["manual"], data = [ "//source/exe:envoy-static", - "//test/config/integration:google_com_proxy_port_0" + "//test/config/integration:google_com_proxy_port_0", ], + tags = ["manual"], deps = [ "//source/common/api:api_lib", "//source/exe:main_common_lib", @@ -101,4 +101,4 @@ envoy_cc_test( "//test/test_common:contention_lib", "//test/test_common:environment_lib", ], -) \ No newline at end of file +) diff --git a/test/exe/win32_outofproc_main_test.cc b/test/exe/win32_outofproc_main_test.cc index 2b7e0d2ecb65..ef3d8eb000e1 100644 --- a/test/exe/win32_outofproc_main_test.cc +++ b/test/exe/win32_outofproc_main_test.cc @@ -41,14 +41,14 @@ class MainCommonTest : public testing::TestWithParam(commandLine.c_str()), // command line - NULL, // process security attributes - NULL, // primary thread security attributes - 1, // handles are inherited - CREATE_NEW_PROCESS_GROUP, // creation flags - NULL, // use parent's environment - NULL, // use parent's current directory - &siStartInfo, // STARTUPINFO pointer - &piProcInfo_); // receives PROCESS_INFORMATION + NULL, // process security attributes + NULL, // primary thread security attributes + 1, // handles are inherited + CREATE_NEW_PROCESS_GROUP, // creation flags + NULL, // use parent's environment + NULL, // use parent's current directory + &siStartInfo, // `STARTUPINFO` pointer + &piProcInfo_); // receives `PROCESS_INFORMATION` EXPECT_TRUE(bSuccess) << fmt::format("Failed to create Envoy process, error {}", GetLastError()); } From 44d460251a20ef25e125a571a8c9fc2b8ba86a26 Mon Sep 17 00:00:00 2001 From: Sotiris Nanopoulos Date: Tue, 17 Nov 2020 21:48:47 -0800 Subject: [PATCH 15/33] order includes Signed-off-by: Sotiris Nanopoulos --- source/exe/win32/platform_impl.cc | 2 +- test/exe/win32_outofproc_main_test.cc | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/source/exe/win32/platform_impl.cc b/source/exe/win32/platform_impl.cc index 2fabcda19002..0351228fbf84 100644 --- a/source/exe/win32/platform_impl.cc +++ b/source/exe/win32/platform_impl.cc @@ -1,5 +1,5 @@ -#include #include +#include #include "common/buffer/buffer_impl.h" #include "common/common/assert.h" diff --git a/test/exe/win32_outofproc_main_test.cc b/test/exe/win32_outofproc_main_test.cc index ef3d8eb000e1..66d44c67ee6b 100644 --- a/test/exe/win32_outofproc_main_test.cc +++ b/test/exe/win32_outofproc_main_test.cc @@ -1,8 +1,9 @@ +#include + #include "test/test_common/utility.h" #include "test/test_common/environment.h" -#include "gtest/gtest.h" -#include +#include "gtest/gtest.h" namespace Envoy { From 4d900be4ae5addf0961bfd662fddb1ddc33e735f Mon Sep 17 00:00:00 2001 From: Sotiris Nanopoulos Date: Tue, 17 Nov 2020 22:02:20 -0800 Subject: [PATCH 16/33] fix include order vol 2 Signed-off-by: Sotiris Nanopoulos --- test/exe/win32_outofproc_main_test.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/exe/win32_outofproc_main_test.cc b/test/exe/win32_outofproc_main_test.cc index 66d44c67ee6b..7b95f81dacf5 100644 --- a/test/exe/win32_outofproc_main_test.cc +++ b/test/exe/win32_outofproc_main_test.cc @@ -1,7 +1,8 @@ #include -#include "test/test_common/utility.h" #include "test/test_common/environment.h" +#include "test/test_common/utility.h" + #include "gtest/gtest.h" From a191b0c3b2afcc094ed061f40f6fbc9447893147 Mon Sep 17 00:00:00 2001 From: Sotiris Nanopoulos Date: Tue, 17 Nov 2020 22:29:31 -0800 Subject: [PATCH 17/33] remove extra empty line Signed-off-by: Sotiris Nanopoulos --- test/exe/win32_outofproc_main_test.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/test/exe/win32_outofproc_main_test.cc b/test/exe/win32_outofproc_main_test.cc index 7b95f81dacf5..e26e86270de2 100644 --- a/test/exe/win32_outofproc_main_test.cc +++ b/test/exe/win32_outofproc_main_test.cc @@ -3,7 +3,6 @@ #include "test/test_common/environment.h" #include "test/test_common/utility.h" - #include "gtest/gtest.h" namespace Envoy { From a20173169c112ffff7303c1bd96c3c3561575607 Mon Sep 17 00:00:00 2001 From: Sotiris Nanopoulos Date: Tue, 17 Nov 2020 23:24:12 -0800 Subject: [PATCH 18/33] switch format to use absl duration Signed-off-by: Sotiris Nanopoulos --- source/exe/win32/platform_impl.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/source/exe/win32/platform_impl.cc b/source/exe/win32/platform_impl.cc index 0351228fbf84..0f381ab79faf 100644 --- a/source/exe/win32/platform_impl.cc +++ b/source/exe/win32/platform_impl.cc @@ -35,8 +35,7 @@ BOOL WINAPI CtrlHandler(DWORD fdwCtrlType) { // These events terminate the process immediately so we want to give a couple of seconds // to the dispatcher to shutdown the server. constexpr size_t delay = 3; - std::chrono::seconds sec(delay); - std::this_thread::sleep_for(sec); + absl::SleepFor(absl::Seconds(delay)); } return 1; } From c484a7512620d167f4f04d6d13568e3714200ea5 Mon Sep 17 00:00:00 2001 From: Sotiris Nanopoulos Date: Wed, 18 Nov 2020 09:31:11 -0800 Subject: [PATCH 19/33] fix linux CI Signed-off-by: Sotiris Nanopoulos --- source/server/server.cc | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/source/server/server.cc b/source/server/server.cc index 7910149719f2..3c2bcaef9f71 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -53,9 +53,6 @@ #include "server/listener_hooks.h" #include "server/ssl_context_manager.h" -namespace { -constexpr bool platformSupportsSignal(signal_t signal_num) { return signal_num != -1; } -} // namespace namespace Envoy { namespace Server { @@ -623,22 +620,22 @@ RunHelper::RunHelper(Instance& instance, const Options& options, Event::Dispatch instance.shutdown(); }); #else - sigterm_ = dispatcher.listenForSignal(ENVOY_SIGTERM, [&instance]() { - ENVOY_LOG(warn, "caught ENVOY_SIGTERM"); + sigterm_ = dispatcher.listenForSignal(SIGTERM, [&instance]() { + ENVOY_LOG(warn, "caught SIGTERM"); instance.shutdown(); }); - sigint_ = dispatcher.listenForSignal(ENVOY_SIGINIT, [&instance]() { + sigint_ = dispatcher.listenForSignal(SIGINIT, [&instance]() { ENVOY_LOG(warn, "caught SIGINT"); instance.shutdown(); }); - sig_usr_1_ = dispatcher.listenForSignal(ENVOY_SIGUSR1, [&access_log_manager]() { + sig_usr_1_ = dispatcher.listenForSignal(SIGUSR1, [&access_log_manager]() { ENVOY_LOG(info, "caught SIGUSR1. Reopening access logs."); access_log_manager.reopen(); }); - sig_hup_ = dispatcher.listenForSignal(ENVOY_SIGHUP, []() { + sig_hup_ = dispatcher.listenForSignal(SIGHUP, []() { ENVOY_LOG(warn, "caught and eating SIGHUP. See documentation for how to hot restart."); }); #endif From 3d71e0482843fae084a19008ac5dbd0d27648a2a Mon Sep 17 00:00:00 2001 From: Sotiris Nanopoulos Date: Wed, 18 Nov 2020 12:14:34 -0800 Subject: [PATCH 20/33] fix linux signal name typo Signed-off-by: Sotiris Nanopoulos --- source/server/server.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/server/server.cc b/source/server/server.cc index 3c2bcaef9f71..cba8b3a480e6 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -625,7 +625,7 @@ RunHelper::RunHelper(Instance& instance, const Options& options, Event::Dispatch instance.shutdown(); }); - sigint_ = dispatcher.listenForSignal(SIGINIT, [&instance]() { + sigint_ = dispatcher.listenForSignal(SIGINT, [&instance]() { ENVOY_LOG(warn, "caught SIGINT"); instance.shutdown(); }); From b05012635a95ddda6c508ea6ead80bc64f10b79c Mon Sep 17 00:00:00 2001 From: Sotiris Nanopoulos Date: Wed, 18 Nov 2020 16:18:49 -0800 Subject: [PATCH 21/33] fix windows crash Signed-off-by: Sotiris Nanopoulos --- source/common/event/BUILD | 1 - source/common/event/win32/signal_impl.h | 6 +++++- test/exe/win32_outofproc_main_test.cc | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/source/common/event/BUILD b/source/common/event/BUILD index fd0b0706f9d7..7c042c325f8a 100644 --- a/source/common/event/BUILD +++ b/source/common/event/BUILD @@ -75,7 +75,6 @@ envoy_cc_win32_library( hdrs = ["win32/signal_impl.h"], strip_include_prefix = "win32", deps = [ - ":dispatcher_includes", "//include/envoy/event:signal_interface", "//source/common/api:os_sys_calls_lib", "//source/common/common:thread_lib", diff --git a/source/common/event/win32/signal_impl.h b/source/common/event/win32/signal_impl.h index 9d18839ba646..b553de89756d 100644 --- a/source/common/event/win32/signal_impl.h +++ b/source/common/event/win32/signal_impl.h @@ -17,7 +17,7 @@ namespace Event { /** * libevent implementation of Event::SignalEvent. */ -class SignalEventImpl : public SignalEvent, ImplBase { +class SignalEventImpl : public SignalEvent { public: SignalEventImpl(DispatcherImpl& dispatcher, signal_t signal_num, SignalCb cb); @@ -26,6 +26,10 @@ class SignalEventImpl : public SignalEvent, ImplBase { Network::IoHandlePtr read_handle_; }; +// Windows ConsoleControlHandler does not allow for a context. As a result the thread +// spawned to handle the console events communicates with the main program with this socketpair. +// Here we have a map from signal types to IoHandle. When we write to this handle we trigger an +// event that notifies Envoy to act on the signal. using eventBridgeHandlersSingleton = ThreadSafeSingleton>>; } // namespace Event diff --git a/test/exe/win32_outofproc_main_test.cc b/test/exe/win32_outofproc_main_test.cc index e26e86270de2..98201c8cf3a7 100644 --- a/test/exe/win32_outofproc_main_test.cc +++ b/test/exe/win32_outofproc_main_test.cc @@ -1,4 +1,4 @@ -#include +#include #include "test/test_common/environment.h" #include "test/test_common/utility.h" From c7186df8cf424ce45ce8f797b8462d9f54470a6a Mon Sep 17 00:00:00 2001 From: Sotiris Nanopoulos Date: Fri, 20 Nov 2020 16:28:03 -0800 Subject: [PATCH 22/33] fix build deps Signed-off-by: Sotiris Nanopoulos --- source/exe/BUILD | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/exe/BUILD b/source/exe/BUILD index 4c234ed183ce..38bd42889e62 100644 --- a/source/exe/BUILD +++ b/source/exe/BUILD @@ -170,7 +170,7 @@ envoy_cc_win32_library( ":platform_header_lib", "//source/common/common:assert_lib", "//source/common/common:thread_lib", - "//source/common/event:dispatcher_lib", + "//source/common/event:signal_lib", "//source/common/filesystem:filesystem_lib", ], ) From e6cc64230e53c0724f72c2e7b3da4f657880d2d3 Mon Sep 17 00:00:00 2001 From: Sotiris Nanopoulos Date: Fri, 20 Nov 2020 16:38:15 -0800 Subject: [PATCH 23/33] try fixing clang-tidy Signed-off-by: Sotiris Nanopoulos --- source/common/event/BUILD | 1 + 1 file changed, 1 insertion(+) diff --git a/source/common/event/BUILD b/source/common/event/BUILD index 7c042c325f8a..fd0b0706f9d7 100644 --- a/source/common/event/BUILD +++ b/source/common/event/BUILD @@ -75,6 +75,7 @@ envoy_cc_win32_library( hdrs = ["win32/signal_impl.h"], strip_include_prefix = "win32", deps = [ + ":dispatcher_includes", "//include/envoy/event:signal_interface", "//source/common/api:os_sys_calls_lib", "//source/common/common:thread_lib", From a295a16b3faff2810f58311a7d9531713e57264e Mon Sep 17 00:00:00 2001 From: Sotiris Nanopoulos Date: Fri, 20 Nov 2020 19:18:11 -0800 Subject: [PATCH 24/33] fix build break Signed-off-by: Sotiris Nanopoulos --- source/common/event/win32/signal_impl.cc | 1 + source/common/event/win32/signal_impl.h | 1 - source/exe/BUILD | 1 + 3 files changed, 2 insertions(+), 1 deletion(-) diff --git a/source/common/event/win32/signal_impl.cc b/source/common/event/win32/signal_impl.cc index 35597294c3de..8f3d8a8b1faf 100644 --- a/source/common/event/win32/signal_impl.cc +++ b/source/common/event/win32/signal_impl.cc @@ -1,3 +1,4 @@ +#include "common/api/os_sys_calls_impl.h" #include "common/event/dispatcher_impl.h" #include "common/event/signal_impl.h" diff --git a/source/common/event/win32/signal_impl.h b/source/common/event/win32/signal_impl.h index b553de89756d..9ad60846f37b 100644 --- a/source/common/event/win32/signal_impl.h +++ b/source/common/event/win32/signal_impl.h @@ -3,7 +3,6 @@ #include "envoy/event/signal.h" #include "envoy/network/io_handle.h" -#include "common/api/os_sys_calls_impl.h" #include "common/event/dispatcher_impl.h" #include "common/event/event_impl_base.h" #include "common/network/io_socket_handle_impl.h" diff --git a/source/exe/BUILD b/source/exe/BUILD index 38bd42889e62..fe46f4574797 100644 --- a/source/exe/BUILD +++ b/source/exe/BUILD @@ -172,6 +172,7 @@ envoy_cc_win32_library( "//source/common/common:thread_lib", "//source/common/event:signal_lib", "//source/common/filesystem:filesystem_lib", + "//source/common/buffer:buffer_lib", ], ) From e9bb5f20f36c73ec69f012bda60aff4e10b53a3a Mon Sep 17 00:00:00 2001 From: Sotiris Nanopoulos Date: Sat, 21 Nov 2020 12:48:03 -0800 Subject: [PATCH 25/33] fix build tools format Signed-off-by: Sotiris Nanopoulos --- source/exe/BUILD | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/exe/BUILD b/source/exe/BUILD index fe46f4574797..1b8bc64bbcb5 100644 --- a/source/exe/BUILD +++ b/source/exe/BUILD @@ -168,11 +168,11 @@ envoy_cc_win32_library( srcs = ["win32/platform_impl.cc"], deps = [ ":platform_header_lib", + "//source/common/buffer:buffer_lib", "//source/common/common:assert_lib", "//source/common/common:thread_lib", "//source/common/event:signal_lib", "//source/common/filesystem:filesystem_lib", - "//source/common/buffer:buffer_lib", ], ) From 67aaa420607db96af110ca969a28f9748796be90 Mon Sep 17 00:00:00 2001 From: Sotiris Nanopoulos Date: Mon, 30 Nov 2020 09:20:42 -0800 Subject: [PATCH 26/33] attempt to fix clang_tidy Signed-off-by: Sotiris Nanopoulos --- ci/run_clang_tidy.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/run_clang_tidy.sh b/ci/run_clang_tidy.sh index b29a624c3f45..76c039761f9e 100755 --- a/ci/run_clang_tidy.sh +++ b/ci/run_clang_tidy.sh @@ -31,7 +31,7 @@ echo "Generating compilation database..." # Do not run clang-tidy against win32 impl # TODO(scw00): We should run clang-tidy against win32 impl once we have clang-cl support for Windows function exclude_win32_impl() { - grep -v source/common/filesystem/win32/ | grep -v source/common/common/win32 | grep -v source/exe/win32 | grep -v source/common/api/win32 + grep -v source/common/filesystem/win32/ | grep -v source/common/common/win32 | grep -v source/exe/win32 | grep -v source/common/api/win32 | grep -v source/source/common/event/win32 } # Do not run clang-tidy against macOS impl From 6cc663b79bf240d8ad2d75dc703a0cbde30d3e90 Mon Sep 17 00:00:00 2001 From: Sotiris Nanopoulos Date: Mon, 30 Nov 2020 10:38:31 -0800 Subject: [PATCH 27/33] fix typo Signed-off-by: Sotiris Nanopoulos --- ci/run_clang_tidy.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/run_clang_tidy.sh b/ci/run_clang_tidy.sh index 76c039761f9e..6412bedc1061 100755 --- a/ci/run_clang_tidy.sh +++ b/ci/run_clang_tidy.sh @@ -31,7 +31,7 @@ echo "Generating compilation database..." # Do not run clang-tidy against win32 impl # TODO(scw00): We should run clang-tidy against win32 impl once we have clang-cl support for Windows function exclude_win32_impl() { - grep -v source/common/filesystem/win32/ | grep -v source/common/common/win32 | grep -v source/exe/win32 | grep -v source/common/api/win32 | grep -v source/source/common/event/win32 + grep -v source/common/filesystem/win32/ | grep -v source/common/common/win32 | grep -v source/exe/win32 | grep -v source/common/api/win32 | grep -v source/common/event/win32 } # Do not run clang-tidy against macOS impl From c378f5f6189624b029a4b5c17e677a25add84ce0 Mon Sep 17 00:00:00 2001 From: Sotiris Nanopoulos Date: Wed, 2 Dec 2020 09:50:34 -0800 Subject: [PATCH 28/33] Change from map to array Signed-off-by: Sotiris Nanopoulos --- include/envoy/common/platform.h | 3 ++- source/common/event/win32/signal_impl.cc | 11 ++++++++--- source/common/event/win32/signal_impl.h | 2 +- source/exe/win32/platform_impl.cc | 12 ++++++++---- test/exe/win32_outofproc_main_test.cc | 2 +- 5 files changed, 20 insertions(+), 10 deletions(-) diff --git a/include/envoy/common/platform.h b/include/envoy/common/platform.h index c1fd99b0bd99..3a1c906e231e 100644 --- a/include/envoy/common/platform.h +++ b/include/envoy/common/platform.h @@ -152,7 +152,8 @@ struct msghdr { #define HANDLE_ERROR_PERM ERROR_ACCESS_DENIED #define HANDLE_ERROR_INVALID ERROR_INVALID_HANDLE -#define ENVOY_WIN32_SIGTERM 1 +#define ENVOY_WIN32_SIGNAL_COUNT 1 +#define ENVOY_WIN32_SIGTERM 0 namespace Platform { constexpr absl::string_view null_device_path{"NUL"}; diff --git a/source/common/event/win32/signal_impl.cc b/source/common/event/win32/signal_impl.cc index 8f3d8a8b1faf..11e63e38a54d 100644 --- a/source/common/event/win32/signal_impl.cc +++ b/source/common/event/win32/signal_impl.cc @@ -9,11 +9,16 @@ namespace Event { SignalEventImpl::SignalEventImpl(DispatcherImpl& dispatcher, signal_t signal_num, SignalCb cb) : cb_(cb) { - auto handler_it = eventBridgeHandlersSingleton::get().find(signal_num); - if (handler_it != eventBridgeHandlersSingleton::get().end()) { + + if (signal_num > eventBridgeHandlersSingleton::get().size()) { + ENVOY_BUG(false, "Attempting to create SignalEventImpl with a signal id that exceeds the " + "number of supported signals."); return; } + if (eventBridgeHandlersSingleton::get()[signal_num]) { + return; + } os_fd_t socks[2]; Api::SysCallIntResult result = Api::OsSysCallsSingleton::get().socketpair(AF_INET, SOCK_STREAM, IPPROTO_TCP, socks); @@ -33,7 +38,7 @@ SignalEventImpl::SignalEventImpl(DispatcherImpl& dispatcher, signal_t signal_num cb_(); }, Event::FileTriggerType::Level, Event::FileReadyType::Read); - eventBridgeHandlersSingleton::get().insert({signal_num, write_handle}); + eventBridgeHandlersSingleton::get()[signal_num] = write_handle; } } // namespace Event diff --git a/source/common/event/win32/signal_impl.h b/source/common/event/win32/signal_impl.h index 9ad60846f37b..51e21e4f1ec9 100644 --- a/source/common/event/win32/signal_impl.h +++ b/source/common/event/win32/signal_impl.h @@ -30,6 +30,6 @@ class SignalEventImpl : public SignalEvent { // Here we have a map from signal types to IoHandle. When we write to this handle we trigger an // event that notifies Envoy to act on the signal. using eventBridgeHandlersSingleton = - ThreadSafeSingleton>>; + ThreadSafeSingleton, ENVOY_WIN32_SIGNAL_COUNT>>; } // namespace Event } // namespace Envoy diff --git a/source/exe/win32/platform_impl.cc b/source/exe/win32/platform_impl.cc index 0f381ab79faf..84f0adcfdf1e 100644 --- a/source/exe/win32/platform_impl.cc +++ b/source/exe/win32/platform_impl.cc @@ -19,15 +19,19 @@ BOOL WINAPI CtrlHandler(DWORD fdwCtrlType) { } shutdown_pending = true; - auto eventBridgeHandlers = Event::eventBridgeHandlersSingleton::get(); - auto handler_it = eventBridgeHandlers.find(ENVOY_WIN32_SIGTERM); - if (handler_it == eventBridgeHandlers.end() || !handler_it->second) { + auto handler = Event::eventBridgeHandlersSingleton::get()[ENVOY_WIN32_SIGTERM]; + if (!handler) { return 0; } + + // This code is executed as part of a thread running under a Windows + // context. For that reason we want to avoid allocating memory or + // taking locks. This is why we use dont want to just write to a socket + // to wake up the signal handler. Buffer::OwnedImpl buffer; constexpr absl::string_view data{"a"}; buffer.add(data); - auto result = handler_it->second->write(buffer); + auto result = handler->write(buffer); RELEASE_ASSERT(result.rc_ == 1, fmt::format("failed to write 1 byte: {}", result.err_->getErrorDetails())); diff --git a/test/exe/win32_outofproc_main_test.cc b/test/exe/win32_outofproc_main_test.cc index 98201c8cf3a7..c3dc7c8db9fa 100644 --- a/test/exe/win32_outofproc_main_test.cc +++ b/test/exe/win32_outofproc_main_test.cc @@ -64,7 +64,7 @@ INSTANTIATE_TEST_SUITE_P(IpVersions, MainCommonTest, testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), TestUtility::ipTestParamsToString); -TEST_P(MainCommonTest, Envoy1) { +TEST_P(MainCommonTest, EnvoyHandlesCtrlBreakEvent) { createEnvoyProcess(); ENVOY_LOG_MISC(warn, "Envoy process with pid {}", piProcInfo_.dwProcessId); Sleep(2 * 1000); From 755009970df84fae5743e91d9b40d303b9e8f2ad Mon Sep 17 00:00:00 2001 From: Sotiris Nanopoulos Date: Wed, 2 Dec 2020 10:51:10 -0800 Subject: [PATCH 29/33] Spelling Signed-off-by: Sotiris Nanopoulos --- source/exe/win32/platform_impl.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/exe/win32/platform_impl.cc b/source/exe/win32/platform_impl.cc index 84f0adcfdf1e..5c91f1a1a789 100644 --- a/source/exe/win32/platform_impl.cc +++ b/source/exe/win32/platform_impl.cc @@ -26,7 +26,7 @@ BOOL WINAPI CtrlHandler(DWORD fdwCtrlType) { // This code is executed as part of a thread running under a Windows // context. For that reason we want to avoid allocating memory or - // taking locks. This is why we use dont want to just write to a socket + // taking locks. This is why we use do not want to just write to a socket // to wake up the signal handler. Buffer::OwnedImpl buffer; constexpr absl::string_view data{"a"}; From 30bbdfa9c6a57e9b792bdc78a1b680246c88dba8 Mon Sep 17 00:00:00 2001 From: Sotiris Nanopoulos Date: Fri, 4 Dec 2020 16:37:58 -0800 Subject: [PATCH 30/33] fix pr comments by greg Signed-off-by: Sotiris Nanopoulos --- include/envoy/common/platform.h | 4 +++- source/exe/win32/platform_impl.cc | 12 ++++++------ source/server/server.cc | 16 +++++++--------- test/exe/BUILD | 1 + test/exe/win32_outofproc_main_test.cc | 2 +- 5 files changed, 18 insertions(+), 17 deletions(-) diff --git a/include/envoy/common/platform.h b/include/envoy/common/platform.h index 3a1c906e231e..ceb6c8b220e7 100644 --- a/include/envoy/common/platform.h +++ b/include/envoy/common/platform.h @@ -153,7 +153,7 @@ struct msghdr { #define HANDLE_ERROR_INVALID ERROR_INVALID_HANDLE #define ENVOY_WIN32_SIGNAL_COUNT 1 -#define ENVOY_WIN32_SIGTERM 0 +#define ENVOY_SIGTERM 0 namespace Platform { constexpr absl::string_view null_device_path{"NUL"}; @@ -250,6 +250,8 @@ typedef int signal_t; // NOLINT(modernize-use-using) #define HANDLE_ERROR_PERM EACCES #define HANDLE_ERROR_INVALID EBADF +#define ENVOY_SIGTERM SIGTERM + namespace Platform { constexpr absl::string_view null_device_path{"/dev/null"}; } diff --git a/source/exe/win32/platform_impl.cc b/source/exe/win32/platform_impl.cc index 5c91f1a1a789..ee729b5a167b 100644 --- a/source/exe/win32/platform_impl.cc +++ b/source/exe/win32/platform_impl.cc @@ -11,7 +11,7 @@ namespace Envoy { -static volatile bool shutdown_pending = false; +static std::atomic shutdown_pending = false; BOOL WINAPI CtrlHandler(DWORD fdwCtrlType) { if (shutdown_pending) { @@ -19,15 +19,15 @@ BOOL WINAPI CtrlHandler(DWORD fdwCtrlType) { } shutdown_pending = true; - auto handler = Event::eventBridgeHandlersSingleton::get()[ENVOY_WIN32_SIGTERM]; + auto handler = Event::eventBridgeHandlersSingleton::get()[ENVOY_SIGTERM]; if (!handler) { return 0; } - // This code is executed as part of a thread running under a Windows - // context. For that reason we want to avoid allocating memory or - // taking locks. This is why we use do not want to just write to a socket - // to wake up the signal handler. + // This code is executed as part of a thread running under a thread owned and + // managed by Windows console host. For that reason we want to avoid allocating + // substantial amount of memory or taking locks. + // This is why we write to a socket to wake up the signal handler. Buffer::OwnedImpl buffer; constexpr absl::string_view data{"a"}; buffer.add(data); diff --git a/source/server/server.cc b/source/server/server.cc index c86f4b6d9d3a..ffe3bf0c7c96 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -643,18 +643,16 @@ RunHelper::RunHelper(Instance& instance, const Options& options, Event::Dispatch } }) { // Setup signals. + // Since signals are not supported on Windows we have an internal definition for `SIGTERM` + // On POSIX it resolves as expected to SIGTERM + // On Windows we use it internaly for all the console events that indicate that we should + // terminate the process. if (options.signalHandlingEnabled()) { -#ifdef WIN32 - sigterm_ = dispatcher.listenForSignal(ENVOY_WIN32_SIGTERM, [&instance]() { - ENVOY_LOG(warn, "caught ENVOY_WIN32_SIGTERM"); + sigterm_ = dispatcher.listenForSignal(ENVOY_SIGTERM, [&instance]() { + ENVOY_LOG(warn, "caught ENVOY_SIGTERM"); instance.shutdown(); }); -#else - sigterm_ = dispatcher.listenForSignal(SIGTERM, [&instance]() { - ENVOY_LOG(warn, "caught SIGTERM"); - instance.shutdown(); - }); - +#ifndef WIN32 sigint_ = dispatcher.listenForSignal(SIGINT, [&instance]() { ENVOY_LOG(warn, "caught SIGINT"); instance.shutdown(); diff --git a/test/exe/BUILD b/test/exe/BUILD index 1656b7f2c1e5..e81ade64cb75 100644 --- a/test/exe/BUILD +++ b/test/exe/BUILD @@ -93,6 +93,7 @@ envoy_cc_test( "//source/exe:envoy-static", "//test/config/integration:google_com_proxy_port_0", ], + # TODO(envoyproxy/windows-dev): Disable the manual tag. tags = ["manual"], deps = [ "//source/common/api:api_lib", diff --git a/test/exe/win32_outofproc_main_test.cc b/test/exe/win32_outofproc_main_test.cc index c3dc7c8db9fa..529d77fa758b 100644 --- a/test/exe/win32_outofproc_main_test.cc +++ b/test/exe/win32_outofproc_main_test.cc @@ -80,7 +80,7 @@ TEST_P(MainCommonTest, EnvoyHandlesCtrlBreakEvent) { EXPECT_TRUE(total_sleep <= resonable_sleep_time); auto output = TestEnvironment::readFileToStringForTest(log_path_); size_t count = 0; - for (size_t pos = 0; (pos = output.find("ENVOY_WIN32_SIGTERM", pos)) != std::string::npos; + for (size_t pos = 0; (pos = output.find("ENVOY_SIGTERM", pos)) != std::string::npos; ++pos, ++count) { } EXPECT_EQ(1, count); From a28a4f0f094864b83a9dc74c07433ff9c097339c Mon Sep 17 00:00:00 2001 From: Sotiris Nanopoulos Date: Fri, 4 Dec 2020 17:26:13 -0800 Subject: [PATCH 31/33] fix spelling mistake Signed-off-by: Sotiris Nanopoulos --- source/server/server.cc | 1684 +++++++++++++++++++-------------------- 1 file changed, 842 insertions(+), 842 deletions(-) diff --git a/source/server/server.cc b/source/server/server.cc index ffe3bf0c7c96..12ea9ab163af 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -1,842 +1,842 @@ -#include "server/server.h" - -#include -#include -#include -#include -#include -#include - -#include "envoy/admin/v3/config_dump.pb.h" -#include "envoy/common/exception.h" -#include "envoy/common/time.h" -#include "envoy/config/bootstrap/v2/bootstrap.pb.h" -#include "envoy/config/bootstrap/v2/bootstrap.pb.validate.h" -#include "envoy/config/bootstrap/v3/bootstrap.pb.h" -#include "envoy/config/bootstrap/v3/bootstrap.pb.validate.h" -#include "envoy/event/dispatcher.h" -#include "envoy/event/signal.h" -#include "envoy/event/timer.h" -#include "envoy/network/dns.h" -#include "envoy/registry/registry.h" -#include "envoy/server/bootstrap_extension_config.h" -#include "envoy/server/instance.h" -#include "envoy/server/options.h" -#include "envoy/upstream/cluster_manager.h" - -#include "common/api/api_impl.h" -#include "common/api/os_sys_calls_impl.h" -#include "common/common/enum_to_int.h" -#include "common/common/mutex_tracer_impl.h" -#include "common/common/utility.h" -#include "common/config/utility.h" -#include "common/config/version_converter.h" -#include "common/http/codes.h" -#include "common/local_info/local_info_impl.h" -#include "common/memory/stats.h" -#include "common/network/address_impl.h" -#include "common/network/socket_interface.h" -#include "common/network/socket_interface_impl.h" -#include "common/network/tcp_listener_impl.h" -#include "common/protobuf/utility.h" -#include "common/router/rds_impl.h" -#include "common/runtime/runtime_impl.h" -#include "common/signal/fatal_error_handler.h" -#include "common/singleton/manager_impl.h" -#include "common/stats/thread_local_store.h" -#include "common/stats/timespan_impl.h" -#include "common/upstream/cluster_manager_impl.h" -#include "common/version/version.h" - -#include "server/admin/utils.h" -#include "server/configuration_impl.h" -#include "server/connection_handler_impl.h" -#include "server/guarddog_impl.h" -#include "server/listener_hooks.h" -#include "server/ssl_context_manager.h" - -namespace Envoy { -namespace Server { - -InstanceImpl::InstanceImpl( - Init::Manager& init_manager, const Options& options, Event::TimeSystem& time_system, - Network::Address::InstanceConstSharedPtr local_address, ListenerHooks& hooks, - HotRestart& restarter, Stats::StoreRoot& store, Thread::BasicLockable& access_log_lock, - ComponentFactory& component_factory, Random::RandomGeneratorPtr&& random_generator, - ThreadLocal::Instance& tls, Thread::ThreadFactory& thread_factory, - Filesystem::Instance& file_system, std::unique_ptr process_context) - : init_manager_(init_manager), workers_started_(false), live_(false), shutdown_(false), - options_(options), validation_context_(options_.allowUnknownStaticFields(), - !options.rejectUnknownDynamicFields(), - options.ignoreUnknownDynamicFields()), - time_source_(time_system), restarter_(restarter), start_time_(time(nullptr)), - original_start_time_(start_time_), stats_store_(store), thread_local_(tls), - random_generator_(std::move(random_generator)), - api_(new Api::Impl(thread_factory, store, time_system, file_system, *random_generator_, - process_context ? ProcessContextOptRef(std::ref(*process_context)) - : absl::nullopt)), - dispatcher_(api_->allocateDispatcher("main_thread")), - singleton_manager_(new Singleton::ManagerImpl(api_->threadFactory())), - handler_(new ConnectionHandlerImpl(*dispatcher_, absl::nullopt)), - listener_component_factory_(*this), worker_factory_(thread_local_, *api_, hooks), - access_log_manager_(options.fileFlushIntervalMsec(), *api_, *dispatcher_, access_log_lock, - store), - terminated_(false), - mutex_tracer_(options.mutexTracingEnabled() ? &Envoy::MutexTracerImpl::getOrCreateTracer() - : nullptr), - grpc_context_(store.symbolTable()), http_context_(store.symbolTable()), - process_context_(std::move(process_context)), main_thread_id_(std::this_thread::get_id()), - server_contexts_(*this) { - try { - if (!options.logPath().empty()) { - try { - file_logger_ = std::make_unique( - options.logPath(), access_log_manager_, Logger::Registry::getSink()); - } catch (const EnvoyException& e) { - throw EnvoyException( - fmt::format("Failed to open log-file '{}'. e.what(): {}", options.logPath(), e.what())); - } - } - - restarter_.initialize(*dispatcher_, *this); - drain_manager_ = component_factory.createDrainManager(*this); - initialize(options, std::move(local_address), component_factory, hooks); - } catch (const EnvoyException& e) { - ENVOY_LOG(critical, "error initializing configuration '{}': {}", options.configPath(), - e.what()); - terminate(); - throw; - } catch (const std::exception& e) { - ENVOY_LOG(critical, "error initializing due to unexpected exception: {}", e.what()); - terminate(); - throw; - } catch (...) { - ENVOY_LOG(critical, "error initializing due to unknown exception"); - terminate(); - throw; - } -} - -InstanceImpl::~InstanceImpl() { - terminate(); - - // Stop logging to file before all the AccessLogManager and its dependencies are - // destructed to avoid crashing at shutdown. - file_logger_.reset(); - - // Destruct the ListenerManager explicitly, before InstanceImpl's local init_manager_ is - // destructed. - // - // The ListenerManager's DestinationPortsMap contains FilterChainSharedPtrs. There is a rare race - // condition where one of these FilterChains contains an HttpConnectionManager, which contains an - // RdsRouteConfigProvider, which contains an RdsRouteConfigSubscriptionSharedPtr. Since - // RdsRouteConfigSubscription is an Init::Target, ~RdsRouteConfigSubscription triggers a callback - // set at initialization, which goes to unregister it from the top-level InitManager, which has - // already been destructed (use-after-free) causing a segfault. - ENVOY_LOG(debug, "destroying listener manager"); - listener_manager_.reset(); - ENVOY_LOG(debug, "destroyed listener manager"); -} - -Upstream::ClusterManager& InstanceImpl::clusterManager() { return *config_.clusterManager(); } - -void InstanceImpl::drainListeners() { - ENVOY_LOG(info, "closing and draining listeners"); - listener_manager_->stopListeners(ListenerManager::StopListenersType::All); - drain_manager_->startDrainSequence([] {}); -} - -void InstanceImpl::failHealthcheck(bool fail) { - live_.store(!fail); - server_stats_->live_.set(live_.load()); -} - -MetricSnapshotImpl::MetricSnapshotImpl(Stats::Store& store, TimeSource& time_source) { - snapped_counters_ = store.counters(); - counters_.reserve(snapped_counters_.size()); - for (const auto& counter : snapped_counters_) { - counters_.push_back({counter->latch(), *counter}); - } - - snapped_gauges_ = store.gauges(); - gauges_.reserve(snapped_gauges_.size()); - for (const auto& gauge : snapped_gauges_) { - ASSERT(gauge->importMode() != Stats::Gauge::ImportMode::Uninitialized); - gauges_.push_back(*gauge); - } - - snapped_histograms_ = store.histograms(); - histograms_.reserve(snapped_histograms_.size()); - for (const auto& histogram : snapped_histograms_) { - histograms_.push_back(*histogram); - } - - snapped_text_readouts_ = store.textReadouts(); - text_readouts_.reserve(snapped_text_readouts_.size()); - for (const auto& text_readout : snapped_text_readouts_) { - text_readouts_.push_back(*text_readout); - } - - snapshot_time_ = time_source.systemTime(); -} - -void InstanceUtil::flushMetricsToSinks(const std::list& sinks, Stats::Store& store, - TimeSource& time_source) { - // Create a snapshot and flush to all sinks. - // NOTE: Even if there are no sinks, creating the snapshot has the important property that it - // latches all counters on a periodic basis. The hot restart code assumes this is being - // done so this should not be removed. - MetricSnapshotImpl snapshot(store, time_source); - for (const auto& sink : sinks) { - sink->flush(snapshot); - } -} - -void InstanceImpl::flushStats() { - ENVOY_LOG(debug, "flushing stats"); - // If Envoy is not fully initialized, workers will not be started and mergeHistograms - // completion callback is not called immediately. As a result of this server stats will - // not be updated and flushed to stat sinks. So skip mergeHistograms call if workers are - // not started yet. - if (initManager().state() == Init::Manager::State::Initialized) { - // A shutdown initiated before this callback may prevent this from being called as per - // the semantics documented in ThreadLocal's runOnAllThreads method. - stats_store_.mergeHistograms([this]() -> void { flushStatsInternal(); }); - } else { - ENVOY_LOG(debug, "Envoy is not fully initialized, skipping histogram merge and flushing stats"); - flushStatsInternal(); - } -} - -void InstanceImpl::updateServerStats() { - // mergeParentStatsIfAny() does nothing and returns a struct of 0s if there is no parent. - HotRestart::ServerStatsFromParent parent_stats = restarter_.mergeParentStatsIfAny(stats_store_); - - server_stats_->uptime_.set(time(nullptr) - original_start_time_); - server_stats_->memory_allocated_.set(Memory::Stats::totalCurrentlyAllocated() + - parent_stats.parent_memory_allocated_); - server_stats_->memory_heap_size_.set(Memory::Stats::totalCurrentlyReserved()); - server_stats_->memory_physical_size_.set(Memory::Stats::totalPhysicalBytes()); - server_stats_->parent_connections_.set(parent_stats.parent_connections_); - server_stats_->total_connections_.set(listener_manager_->numConnections() + - parent_stats.parent_connections_); - server_stats_->days_until_first_cert_expiring_.set( - sslContextManager().daysUntilFirstCertExpires()); - - auto secs_until_ocsp_response_expires = - sslContextManager().secondsUntilFirstOcspResponseExpires(); - if (secs_until_ocsp_response_expires) { - server_stats_->seconds_until_first_ocsp_response_expiring_.set( - secs_until_ocsp_response_expires.value()); - } - server_stats_->state_.set( - enumToInt(Utility::serverState(initManager().state(), healthCheckFailed()))); - server_stats_->stats_recent_lookups_.set( - stats_store_.symbolTable().getRecentLookups([](absl::string_view, uint64_t) {})); -} - -void InstanceImpl::flushStatsInternal() { - updateServerStats(); - InstanceUtil::flushMetricsToSinks(config_.statsSinks(), stats_store_, timeSource()); - // TODO(ramaraochavali): consider adding different flush interval for histograms. - if (stat_flush_timer_ != nullptr) { - stat_flush_timer_->enableTimer(config_.statsFlushInterval()); - } -} - -bool InstanceImpl::healthCheckFailed() { return !live_.load(); } - -ProcessContextOptRef InstanceImpl::processContext() { - if (process_context_ == nullptr) { - return absl::nullopt; - } - - return *process_context_; -} - -namespace { -// Loads a bootstrap object, potentially at a specific version (upgrading if necessary). -void loadBootstrap(absl::optional bootstrap_version, - envoy::config::bootstrap::v3::Bootstrap& bootstrap, - std::function load_function) { - - if (!bootstrap_version.has_value()) { - load_function(bootstrap, true); - } else if (*bootstrap_version == 3) { - load_function(bootstrap, false); - } else if (*bootstrap_version == 2) { - envoy::config::bootstrap::v2::Bootstrap bootstrap_v2; - load_function(bootstrap_v2, false); - Config::VersionConverter::upgrade(bootstrap_v2, bootstrap); - MessageUtil::onVersionUpgradeDeprecation("v2 bootstrap", false); - } else { - throw EnvoyException(fmt::format("Unknown bootstrap version {}.", *bootstrap_version)); - } -} -} // namespace - -void InstanceUtil::loadBootstrapConfig(envoy::config::bootstrap::v3::Bootstrap& bootstrap, - const Options& options, - ProtobufMessage::ValidationVisitor& validation_visitor, - Api::Api& api) { - const std::string& config_path = options.configPath(); - const std::string& config_yaml = options.configYaml(); - const envoy::config::bootstrap::v3::Bootstrap& config_proto = options.configProto(); - - // Exactly one of config_path and config_yaml should be specified. - if (config_path.empty() && config_yaml.empty() && config_proto.ByteSize() == 0) { - throw EnvoyException("At least one of --config-path or --config-yaml or Options::configProto() " - "should be non-empty"); - } - - if (!config_path.empty()) { - loadBootstrap( - options.bootstrapVersion(), bootstrap, - [&config_path, &validation_visitor, &api](Protobuf::Message& message, bool do_boosting) { - MessageUtil::loadFromFile(config_path, message, validation_visitor, api, do_boosting); - }); - } - if (!config_yaml.empty()) { - envoy::config::bootstrap::v3::Bootstrap bootstrap_override; - loadBootstrap( - options.bootstrapVersion(), bootstrap_override, - [&config_yaml, &validation_visitor](Protobuf::Message& message, bool do_boosting) { - MessageUtil::loadFromYaml(config_yaml, message, validation_visitor, do_boosting); - }); - // TODO(snowp): The fact that we do a merge here doesn't seem to be covered under test. - bootstrap.MergeFrom(bootstrap_override); - } - if (config_proto.ByteSize() != 0) { - bootstrap.MergeFrom(config_proto); - } - MessageUtil::validate(bootstrap, validation_visitor); -} - -void InstanceImpl::initialize(const Options& options, - Network::Address::InstanceConstSharedPtr local_address, - ComponentFactory& component_factory, ListenerHooks& hooks) { - ENVOY_LOG(info, "initializing epoch {} (base id={}, hot restart version={})", - options.restartEpoch(), restarter_.baseId(), restarter_.version()); - - ENVOY_LOG(info, "statically linked extensions:"); - for (const auto& ext : Envoy::Registry::FactoryCategoryRegistry::registeredFactories()) { - ENVOY_LOG(info, " {}: {}", ext.first, absl::StrJoin(ext.second->registeredNames(), ", ")); - } - - // Handle configuration that needs to take place prior to the main configuration load. - InstanceUtil::loadBootstrapConfig(bootstrap_, options, - messageValidationContext().staticValidationVisitor(), *api_); - bootstrap_config_update_time_ = time_source_.systemTime(); - - // Immediate after the bootstrap has been loaded, override the header prefix, if configured to - // do so. This must be set before any other code block references the HeaderValues ConstSingleton. - if (!bootstrap_.header_prefix().empty()) { - // setPrefix has a release assert verifying that setPrefix() is not called after prefix() - ThreadSafeSingleton::get().setPrefix(bootstrap_.header_prefix().c_str()); - } - // TODO(mattklein123): Custom O(1) headers can be registered at this point for creating/finalizing - // any header maps. - ENVOY_LOG(info, "HTTP header map info:"); - for (const auto& info : Http::HeaderMapImplUtility::getAllHeaderMapImplInfo()) { - ENVOY_LOG(info, " {}: {} bytes: {}", info.name_, info.size_, - absl::StrJoin(info.registered_headers_, ",")); - } - - // Needs to happen as early as possible in the instantiation to preempt the objects that require - // stats. - stats_store_.setTagProducer(Config::Utility::createTagProducer(bootstrap_)); - stats_store_.setStatsMatcher(Config::Utility::createStatsMatcher(bootstrap_)); - stats_store_.setHistogramSettings(Config::Utility::createHistogramSettings(bootstrap_)); - - const std::string server_stats_prefix = "server."; - server_stats_ = std::make_unique( - ServerStats{ALL_SERVER_STATS(POOL_COUNTER_PREFIX(stats_store_, server_stats_prefix), - POOL_GAUGE_PREFIX(stats_store_, server_stats_prefix), - POOL_HISTOGRAM_PREFIX(stats_store_, server_stats_prefix))}); - validation_context_.staticWarningValidationVisitor().setUnknownCounter( - server_stats_->static_unknown_fields_); - validation_context_.dynamicWarningValidationVisitor().setUnknownCounter( - server_stats_->dynamic_unknown_fields_); - - initialization_timer_ = std::make_unique( - server_stats_->initialization_time_ms_, timeSource()); - server_stats_->concurrency_.set(options_.concurrency()); - server_stats_->hot_restart_epoch_.set(options_.restartEpoch()); - - assert_action_registration_ = Assert::setDebugAssertionFailureRecordAction( - [this]() { server_stats_->debug_assertion_failures_.inc(); }); - envoy_bug_action_registration_ = Assert::setEnvoyBugFailureRecordAction( - [this]() { server_stats_->envoy_bug_failures_.inc(); }); - - InstanceImpl::failHealthcheck(false); - - // Check if bootstrap has server version override set, if yes, we should use that as - // 'server.version' stat. - uint64_t version_int; - if (bootstrap_.stats_server_version_override().value() > 0) { - version_int = bootstrap_.stats_server_version_override().value(); - } else { - if (!StringUtil::atoull(VersionInfo::revision().substr(0, 6).c_str(), version_int, 16)) { - throw EnvoyException("compiled GIT SHA is invalid. Invalid build."); - } - } - server_stats_->version_.set(version_int); - - bootstrap_.mutable_node()->set_hidden_envoy_deprecated_build_version(VersionInfo::version()); - bootstrap_.mutable_node()->set_user_agent_name("envoy"); - *bootstrap_.mutable_node()->mutable_user_agent_build_version() = VersionInfo::buildVersion(); - for (const auto& ext : Envoy::Registry::FactoryCategoryRegistry::registeredFactories()) { - for (const auto& name : ext.second->allRegisteredNames()) { - auto* extension = bootstrap_.mutable_node()->add_extensions(); - extension->set_name(std::string(name)); - extension->set_category(ext.first); - auto const version = ext.second->getFactoryVersion(name); - if (version) { - *extension->mutable_version() = version.value(); - } - extension->set_disabled(ext.second->isFactoryDisabled(name)); - } - } - - local_info_ = std::make_unique( - bootstrap_.node(), local_address, options.serviceZone(), options.serviceClusterName(), - options.serviceNodeName()); - - Configuration::InitialImpl initial_config(bootstrap_); - - // Learn original_start_time_ if our parent is still around to inform us of it. - restarter_.sendParentAdminShutdownRequest(original_start_time_); - admin_ = std::make_unique(initial_config.admin().profilePath(), *this); - - loadServerFlags(initial_config.flagsPath()); - - secret_manager_ = std::make_unique(admin_->getConfigTracker()); - - // Initialize the overload manager early so other modules can register for actions. - overload_manager_ = std::make_unique( - *dispatcher_, stats_store_, thread_local_, bootstrap_.overload_manager(), - messageValidationContext().staticValidationVisitor(), *api_); - - heap_shrinker_ = - std::make_unique(*dispatcher_, *overload_manager_, stats_store_); - - for (const auto& bootstrap_extension : bootstrap_.bootstrap_extensions()) { - auto& factory = Config::Utility::getAndCheckFactory( - bootstrap_extension); - auto config = Config::Utility::translateAnyToFactoryConfig( - bootstrap_extension.typed_config(), messageValidationContext().staticValidationVisitor(), - factory); - bootstrap_extensions_.push_back( - factory.createBootstrapExtension(*config, serverFactoryContext())); - } - - // Register the fatal actions. - { - FatalAction::FatalActionPtrList safe_actions; - FatalAction::FatalActionPtrList unsafe_actions; - for (const auto& action_config : bootstrap_.fatal_actions()) { - auto& factory = - Config::Utility::getAndCheckFactory( - action_config.config()); - auto action = factory.createFatalActionFromProto(action_config, this); - - if (action->isAsyncSignalSafe()) { - safe_actions.push_back(std::move(action)); - } else { - unsafe_actions.push_back(std::move(action)); - } - } - Envoy::FatalErrorHandler::registerFatalActions( - std::move(safe_actions), std::move(unsafe_actions), api_->threadFactory()); - } - - if (!bootstrap_.default_socket_interface().empty()) { - auto& sock_name = bootstrap_.default_socket_interface(); - auto sock = const_cast(Network::socketInterface(sock_name)); - if (sock != nullptr) { - Network::SocketInterfaceSingleton::clear(); - Network::SocketInterfaceSingleton::initialize(sock); - } - } - - // Workers get created first so they register for thread local updates. - listener_manager_ = std::make_unique( - *this, listener_component_factory_, worker_factory_, bootstrap_.enable_dispatcher_stats()); - - // The main thread is also registered for thread local updates so that code that does not care - // whether it runs on the main thread or on workers can still use TLS. - thread_local_.registerThread(*dispatcher_, true); - - // We can now initialize stats for threading. - stats_store_.initializeThreading(*dispatcher_, thread_local_); - - // It's now safe to start writing stats from the main thread's dispatcher. - if (bootstrap_.enable_dispatcher_stats()) { - dispatcher_->initializeStats(stats_store_, "server."); - } - - if (initial_config.admin().address()) { - if (initial_config.admin().accessLogPath().empty()) { - throw EnvoyException("An admin access log path is required for a listening server."); - } - ENVOY_LOG(info, "admin address: {}", initial_config.admin().address()->asString()); - admin_->startHttpListener(initial_config.admin().accessLogPath(), options.adminAddressPath(), - initial_config.admin().address(), - initial_config.admin().socketOptions(), - stats_store_.createScope("listener.admin.")); - } else { - ENVOY_LOG(warn, "No admin address given, so no admin HTTP server started."); - } - config_tracker_entry_ = - admin_->getConfigTracker().add("bootstrap", [this] { return dumpBootstrapConfig(); }); - if (initial_config.admin().address()) { - admin_->addListenerToHandler(handler_.get()); - } - - // The broad order of initialization from this point on is the following: - // 1. Statically provisioned configuration (bootstrap) are loaded. - // 2. Cluster manager is created and all primary clusters (i.e. with endpoint assignments - // provisioned statically in bootstrap, discovered through DNS or file based CDS) are - // initialized. - // 3. Various services are initialized and configured using the bootstrap config. - // 4. RTDS is initialized using primary clusters. This allows runtime overrides to be fully - // configured before the rest of xDS configuration is provisioned. - // 5. Secondary clusters (with endpoint assignments provisioned by xDS servers) are initialized. - // 6. The rest of the dynamic configuration is provisioned. - // - // Please note: this order requires that RTDS is provisioned using a primary cluster. If RTDS is - // provisioned through ADS then ADS must use primary cluster as well. This invariant is enforced - // during RTDS initialization and invalid configuration will be rejected. - - // Runtime gets initialized before the main configuration since during main configuration - // load things may grab a reference to the loader for later use. - runtime_singleton_ = std::make_unique( - component_factory.createRuntime(*this, initial_config)); - hooks.onRuntimeCreated(); - - // Once we have runtime we can initialize the SSL context manager. - ssl_context_manager_ = createContextManager("ssl_context_manager", time_source_); - - const bool use_tcp_for_dns_lookups = bootstrap_.use_tcp_for_dns_lookups(); - dns_resolver_ = dispatcher_->createDnsResolver({}, use_tcp_for_dns_lookups); - - cluster_manager_factory_ = std::make_unique( - *admin_, Runtime::LoaderSingleton::get(), stats_store_, thread_local_, dns_resolver_, - *ssl_context_manager_, *dispatcher_, *local_info_, *secret_manager_, - messageValidationContext(), *api_, http_context_, grpc_context_, access_log_manager_, - *singleton_manager_); - - // Now the configuration gets parsed. The configuration may start setting - // thread local data per above. See MainImpl::initialize() for why ConfigImpl - // is constructed as part of the InstanceImpl and then populated once - // cluster_manager_factory_ is available. - config_.initialize(bootstrap_, *this, *cluster_manager_factory_); - - // Instruct the listener manager to create the LDS provider if needed. This must be done later - // because various items do not yet exist when the listener manager is created. - if (bootstrap_.dynamic_resources().has_lds_config() || - bootstrap_.dynamic_resources().has_lds_resources_locator()) { - listener_manager_->createLdsApi(bootstrap_.dynamic_resources().lds_config(), - bootstrap_.dynamic_resources().has_lds_resources_locator() - ? &bootstrap_.dynamic_resources().lds_resources_locator() - : nullptr); - } - - // We have to defer RTDS initialization until after the cluster manager is - // instantiated (which in turn relies on runtime...). - Runtime::LoaderSingleton::get().initialize(clusterManager()); - - clusterManager().setPrimaryClustersInitializedCb( - [this]() { onClusterManagerPrimaryInitializationComplete(); }); - - for (Stats::SinkPtr& sink : config_.statsSinks()) { - stats_store_.addSink(*sink); - } - - // Some of the stat sinks may need dispatcher support so don't flush until the main loop starts. - // Just setup the timer. - stat_flush_timer_ = dispatcher_->createTimer([this]() -> void { flushStats(); }); - stat_flush_timer_->enableTimer(config_.statsFlushInterval()); - - // GuardDog (deadlock detection) object and thread setup before workers are - // started and before our own run() loop runs. - main_thread_guard_dog_ = std::make_unique( - stats_store_, config_.mainThreadWatchdogConfig(), *api_, "main_thread"); - worker_guard_dog_ = std::make_unique( - stats_store_, config_.workerWatchdogConfig(), *api_, "workers"); -} - -void InstanceImpl::onClusterManagerPrimaryInitializationComplete() { - // If RTDS was not configured the `onRuntimeReady` callback is immediately invoked. - Runtime::LoaderSingleton::get().startRtdsSubscriptions([this]() { onRuntimeReady(); }); -} - -void InstanceImpl::onRuntimeReady() { - // Begin initializing secondary clusters after RTDS configuration has been applied. - clusterManager().initializeSecondaryClusters(bootstrap_); - - if (bootstrap_.has_hds_config()) { - const auto& hds_config = bootstrap_.hds_config(); - async_client_manager_ = std::make_unique( - *config_.clusterManager(), thread_local_, time_source_, *api_, grpc_context_.statNames()); - hds_delegate_ = std::make_unique( - stats_store_, - Config::Utility::factoryForGrpcApiConfigSource(*async_client_manager_, hds_config, - stats_store_, false) - ->create(), - hds_config.transport_api_version(), *dispatcher_, Runtime::LoaderSingleton::get(), - stats_store_, *ssl_context_manager_, info_factory_, access_log_manager_, - *config_.clusterManager(), *local_info_, *admin_, *singleton_manager_, thread_local_, - messageValidationContext().dynamicValidationVisitor(), *api_); - } - - // If there is no global limit to the number of active connections, warn on startup. - // TODO (tonya11en): Move this functionality into the overload manager. - if (!runtime().snapshot().get(Network::TcpListenerImpl::GlobalMaxCxRuntimeKey)) { - ENVOY_LOG(warn, - "there is no configured limit to the number of allowed active connections. Set a " - "limit via the runtime key {}", - Network::TcpListenerImpl::GlobalMaxCxRuntimeKey); - } -} - -void InstanceImpl::startWorkers() { - listener_manager_->startWorkers(*worker_guard_dog_); - initialization_timer_->complete(); - // Update server stats as soon as initialization is done. - updateServerStats(); - workers_started_ = true; - // At this point we are ready to take traffic and all listening ports are up. Notify our parent - // if applicable that they can stop listening and drain. - restarter_.drainParentListeners(); - drain_manager_->startParentShutdownSequence(); -} - -Runtime::LoaderPtr InstanceUtil::createRuntime(Instance& server, - Server::Configuration::Initial& config) { - ENVOY_LOG(info, "runtime: {}", MessageUtil::getYamlStringFromMessage(config.runtime())); - return std::make_unique( - server.dispatcher(), server.threadLocal(), config.runtime(), server.localInfo(), - server.stats(), server.api().randomGenerator(), - server.messageValidationContext().dynamicValidationVisitor(), server.api()); -} - -void InstanceImpl::loadServerFlags(const absl::optional& flags_path) { - if (!flags_path) { - return; - } - - ENVOY_LOG(info, "server flags path: {}", flags_path.value()); - if (api_->fileSystem().fileExists(flags_path.value() + "/drain")) { - ENVOY_LOG(info, "starting server in drain mode"); - InstanceImpl::failHealthcheck(true); - } -} - -RunHelper::RunHelper(Instance& instance, const Options& options, Event::Dispatcher& dispatcher, - Upstream::ClusterManager& cm, AccessLog::AccessLogManager& access_log_manager, - Init::Manager& init_manager, OverloadManager& overload_manager, - std::function post_init_cb) - : init_watcher_("RunHelper", [&instance, post_init_cb]() { - if (!instance.isShutdown()) { - post_init_cb(); - } - }) { - // Setup signals. - // Since signals are not supported on Windows we have an internal definition for `SIGTERM` - // On POSIX it resolves as expected to SIGTERM - // On Windows we use it internaly for all the console events that indicate that we should - // terminate the process. - if (options.signalHandlingEnabled()) { - sigterm_ = dispatcher.listenForSignal(ENVOY_SIGTERM, [&instance]() { - ENVOY_LOG(warn, "caught ENVOY_SIGTERM"); - instance.shutdown(); - }); -#ifndef WIN32 - sigint_ = dispatcher.listenForSignal(SIGINT, [&instance]() { - ENVOY_LOG(warn, "caught SIGINT"); - instance.shutdown(); - }); - - sig_usr_1_ = dispatcher.listenForSignal(SIGUSR1, [&access_log_manager]() { - ENVOY_LOG(info, "caught SIGUSR1. Reopening access logs."); - access_log_manager.reopen(); - }); - - sig_hup_ = dispatcher.listenForSignal(SIGHUP, []() { - ENVOY_LOG(warn, "caught and eating SIGHUP. See documentation for how to hot restart."); - }); -#endif - } - - // Start overload manager before workers. - overload_manager.start(); - - // Register for cluster manager init notification. We don't start serving worker traffic until - // upstream clusters are initialized which may involve running the event loop. Note however that - // this can fire immediately if all clusters have already initialized. Also note that we need - // to guard against shutdown at two different levels since SIGTERM can come in once the run loop - // starts. - cm.setInitializedCb([&instance, &init_manager, &cm, this]() { - if (instance.isShutdown()) { - return; - } - - const auto type_urls = - Config::getAllVersionTypeUrls(); - // Pause RDS to ensure that we don't send any requests until we've - // subscribed to all the RDS resources. The subscriptions happen in the init callbacks, - // so we pause RDS until we've completed all the callbacks. - Config::ScopedResume maybe_resume_rds; - if (cm.adsMux()) { - maybe_resume_rds = cm.adsMux()->pause(type_urls); - } - - ENVOY_LOG(info, "all clusters initialized. initializing init manager"); - init_manager.initialize(init_watcher_); - - // Now that we're execute all the init callbacks we can resume RDS - // as we've subscribed to all the statically defined RDS resources. - // This is done by tearing down the maybe_resume_rds Cleanup object. - }); -} - -void InstanceImpl::run() { - // RunHelper exists primarily to facilitate testing of how we respond to early shutdown during - // startup (see RunHelperTest in server_test.cc). - const auto run_helper = RunHelper(*this, options_, *dispatcher_, clusterManager(), - access_log_manager_, init_manager_, overloadManager(), [this] { - notifyCallbacksForStage(Stage::PostInit); - startWorkers(); - }); - - // Run the main dispatch loop waiting to exit. - ENVOY_LOG(info, "starting main dispatch loop"); - auto watchdog = main_thread_guard_dog_->createWatchDog(api_->threadFactory().currentThreadId(), - "main_thread", *dispatcher_); - dispatcher_->post([this] { notifyCallbacksForStage(Stage::Startup); }); - dispatcher_->run(Event::Dispatcher::RunType::Block); - ENVOY_LOG(info, "main dispatch loop exited"); - main_thread_guard_dog_->stopWatching(watchdog); - watchdog.reset(); - - terminate(); -} - -void InstanceImpl::terminate() { - if (terminated_) { - return; - } - terminated_ = true; - - // Before starting to shutdown anything else, stop slot destruction updates. - thread_local_.shutdownGlobalThreading(); - - // Before the workers start exiting we should disable stat threading. - stats_store_.shutdownThreading(); - - if (overload_manager_) { - overload_manager_->stop(); - } - - // Shutdown all the workers now that the main dispatch loop is done. - if (listener_manager_ != nullptr) { - // Also shutdown the listener manager's ApiListener, if there is one, which runs on the main - // thread. This needs to happen ahead of calling thread_local_.shutdown() below to prevent any - // objects in the ApiListener destructor to reference any objects in thread local storage. - if (listener_manager_->apiListener().has_value()) { - listener_manager_->apiListener()->get().shutdown(); - } - - listener_manager_->stopWorkers(); - } - - // Only flush if we have not been hot restarted. - if (stat_flush_timer_) { - flushStats(); - } - - if (config_.clusterManager() != nullptr) { - config_.clusterManager()->shutdown(); - } - handler_.reset(); - thread_local_.shutdownThread(); - restarter_.shutdown(); - ENVOY_LOG(info, "exiting"); - ENVOY_FLUSH_LOG(); - FatalErrorHandler::clearFatalActionsOnTerminate(); -} - -Runtime::Loader& InstanceImpl::runtime() { return Runtime::LoaderSingleton::get(); } - -void InstanceImpl::shutdown() { - ENVOY_LOG(info, "shutting down server instance"); - shutdown_ = true; - restarter_.sendParentTerminateRequest(); - notifyCallbacksForStage(Stage::ShutdownExit, [this] { dispatcher_->exit(); }); -} - -void InstanceImpl::shutdownAdmin() { - ENVOY_LOG(warn, "shutting down admin due to child startup"); - stat_flush_timer_.reset(); - handler_->stopListeners(); - admin_->closeSocket(); - - // If we still have a parent, it should be terminated now that we have a child. - ENVOY_LOG(warn, "terminating parent process"); - restarter_.sendParentTerminateRequest(); -} - -ServerLifecycleNotifier::HandlePtr InstanceImpl::registerCallback(Stage stage, - StageCallback callback) { - auto& callbacks = stage_callbacks_[stage]; - return std::make_unique>(callbacks, callback); -} - -ServerLifecycleNotifier::HandlePtr -InstanceImpl::registerCallback(Stage stage, StageCallbackWithCompletion callback) { - ASSERT(stage == Stage::ShutdownExit); - auto& callbacks = stage_completable_callbacks_[stage]; - return std::make_unique>(callbacks, - callback); -} - -void InstanceImpl::notifyCallbacksForStage(Stage stage, Event::PostCb completion_cb) { - ASSERT(std::this_thread::get_id() == main_thread_id_); - const auto it = stage_callbacks_.find(stage); - if (it != stage_callbacks_.end()) { - for (const StageCallback& callback : it->second) { - callback(); - } - } - - // Wrap completion_cb so that it only gets invoked when all callbacks for this stage - // have finished their work. - std::shared_ptr cb_guard( - new Cleanup([this, completion_cb]() { dispatcher_->post(completion_cb); })); - - // Registrations which take a completion callback are typically implemented by executing a - // callback on all worker threads using Slot::runOnAllThreads which will hang indefinitely if - // worker threads have not been started so we need to skip notifications if envoy is shutdown - // early before workers have started. - if (workers_started_) { - const auto it2 = stage_completable_callbacks_.find(stage); - if (it2 != stage_completable_callbacks_.end()) { - ENVOY_LOG(info, "Notifying {} callback(s) with completion.", it2->second.size()); - for (const StageCallbackWithCompletion& callback : it2->second) { - callback([cb_guard] {}); - } - } - } -} - -ProtobufTypes::MessagePtr InstanceImpl::dumpBootstrapConfig() { - auto config_dump = std::make_unique(); - config_dump->mutable_bootstrap()->MergeFrom(bootstrap_); - TimestampUtil::systemClockToTimestamp(bootstrap_config_update_time_, - *(config_dump->mutable_last_updated())); - return config_dump; -} - -} // namespace Server -} // namespace Envoy +#include "server/server.h" + +#include +#include +#include +#include +#include +#include + +#include "envoy/admin/v3/config_dump.pb.h" +#include "envoy/common/exception.h" +#include "envoy/common/time.h" +#include "envoy/config/bootstrap/v2/bootstrap.pb.h" +#include "envoy/config/bootstrap/v2/bootstrap.pb.validate.h" +#include "envoy/config/bootstrap/v3/bootstrap.pb.h" +#include "envoy/config/bootstrap/v3/bootstrap.pb.validate.h" +#include "envoy/event/dispatcher.h" +#include "envoy/event/signal.h" +#include "envoy/event/timer.h" +#include "envoy/network/dns.h" +#include "envoy/registry/registry.h" +#include "envoy/server/bootstrap_extension_config.h" +#include "envoy/server/instance.h" +#include "envoy/server/options.h" +#include "envoy/upstream/cluster_manager.h" + +#include "common/api/api_impl.h" +#include "common/api/os_sys_calls_impl.h" +#include "common/common/enum_to_int.h" +#include "common/common/mutex_tracer_impl.h" +#include "common/common/utility.h" +#include "common/config/utility.h" +#include "common/config/version_converter.h" +#include "common/http/codes.h" +#include "common/local_info/local_info_impl.h" +#include "common/memory/stats.h" +#include "common/network/address_impl.h" +#include "common/network/socket_interface.h" +#include "common/network/socket_interface_impl.h" +#include "common/network/tcp_listener_impl.h" +#include "common/protobuf/utility.h" +#include "common/router/rds_impl.h" +#include "common/runtime/runtime_impl.h" +#include "common/signal/fatal_error_handler.h" +#include "common/singleton/manager_impl.h" +#include "common/stats/thread_local_store.h" +#include "common/stats/timespan_impl.h" +#include "common/upstream/cluster_manager_impl.h" +#include "common/version/version.h" + +#include "server/admin/utils.h" +#include "server/configuration_impl.h" +#include "server/connection_handler_impl.h" +#include "server/guarddog_impl.h" +#include "server/listener_hooks.h" +#include "server/ssl_context_manager.h" + +namespace Envoy { +namespace Server { + +InstanceImpl::InstanceImpl( + Init::Manager& init_manager, const Options& options, Event::TimeSystem& time_system, + Network::Address::InstanceConstSharedPtr local_address, ListenerHooks& hooks, + HotRestart& restarter, Stats::StoreRoot& store, Thread::BasicLockable& access_log_lock, + ComponentFactory& component_factory, Random::RandomGeneratorPtr&& random_generator, + ThreadLocal::Instance& tls, Thread::ThreadFactory& thread_factory, + Filesystem::Instance& file_system, std::unique_ptr process_context) + : init_manager_(init_manager), workers_started_(false), live_(false), shutdown_(false), + options_(options), validation_context_(options_.allowUnknownStaticFields(), + !options.rejectUnknownDynamicFields(), + options.ignoreUnknownDynamicFields()), + time_source_(time_system), restarter_(restarter), start_time_(time(nullptr)), + original_start_time_(start_time_), stats_store_(store), thread_local_(tls), + random_generator_(std::move(random_generator)), + api_(new Api::Impl(thread_factory, store, time_system, file_system, *random_generator_, + process_context ? ProcessContextOptRef(std::ref(*process_context)) + : absl::nullopt)), + dispatcher_(api_->allocateDispatcher("main_thread")), + singleton_manager_(new Singleton::ManagerImpl(api_->threadFactory())), + handler_(new ConnectionHandlerImpl(*dispatcher_, absl::nullopt)), + listener_component_factory_(*this), worker_factory_(thread_local_, *api_, hooks), + access_log_manager_(options.fileFlushIntervalMsec(), *api_, *dispatcher_, access_log_lock, + store), + terminated_(false), + mutex_tracer_(options.mutexTracingEnabled() ? &Envoy::MutexTracerImpl::getOrCreateTracer() + : nullptr), + grpc_context_(store.symbolTable()), http_context_(store.symbolTable()), + process_context_(std::move(process_context)), main_thread_id_(std::this_thread::get_id()), + server_contexts_(*this) { + try { + if (!options.logPath().empty()) { + try { + file_logger_ = std::make_unique( + options.logPath(), access_log_manager_, Logger::Registry::getSink()); + } catch (const EnvoyException& e) { + throw EnvoyException( + fmt::format("Failed to open log-file '{}'. e.what(): {}", options.logPath(), e.what())); + } + } + + restarter_.initialize(*dispatcher_, *this); + drain_manager_ = component_factory.createDrainManager(*this); + initialize(options, std::move(local_address), component_factory, hooks); + } catch (const EnvoyException& e) { + ENVOY_LOG(critical, "error initializing configuration '{}': {}", options.configPath(), + e.what()); + terminate(); + throw; + } catch (const std::exception& e) { + ENVOY_LOG(critical, "error initializing due to unexpected exception: {}", e.what()); + terminate(); + throw; + } catch (...) { + ENVOY_LOG(critical, "error initializing due to unknown exception"); + terminate(); + throw; + } +} + +InstanceImpl::~InstanceImpl() { + terminate(); + + // Stop logging to file before all the AccessLogManager and its dependencies are + // destructed to avoid crashing at shutdown. + file_logger_.reset(); + + // Destruct the ListenerManager explicitly, before InstanceImpl's local init_manager_ is + // destructed. + // + // The ListenerManager's DestinationPortsMap contains FilterChainSharedPtrs. There is a rare race + // condition where one of these FilterChains contains an HttpConnectionManager, which contains an + // RdsRouteConfigProvider, which contains an RdsRouteConfigSubscriptionSharedPtr. Since + // RdsRouteConfigSubscription is an Init::Target, ~RdsRouteConfigSubscription triggers a callback + // set at initialization, which goes to unregister it from the top-level InitManager, which has + // already been destructed (use-after-free) causing a segfault. + ENVOY_LOG(debug, "destroying listener manager"); + listener_manager_.reset(); + ENVOY_LOG(debug, "destroyed listener manager"); +} + +Upstream::ClusterManager& InstanceImpl::clusterManager() { return *config_.clusterManager(); } + +void InstanceImpl::drainListeners() { + ENVOY_LOG(info, "closing and draining listeners"); + listener_manager_->stopListeners(ListenerManager::StopListenersType::All); + drain_manager_->startDrainSequence([] {}); +} + +void InstanceImpl::failHealthcheck(bool fail) { + live_.store(!fail); + server_stats_->live_.set(live_.load()); +} + +MetricSnapshotImpl::MetricSnapshotImpl(Stats::Store& store, TimeSource& time_source) { + snapped_counters_ = store.counters(); + counters_.reserve(snapped_counters_.size()); + for (const auto& counter : snapped_counters_) { + counters_.push_back({counter->latch(), *counter}); + } + + snapped_gauges_ = store.gauges(); + gauges_.reserve(snapped_gauges_.size()); + for (const auto& gauge : snapped_gauges_) { + ASSERT(gauge->importMode() != Stats::Gauge::ImportMode::Uninitialized); + gauges_.push_back(*gauge); + } + + snapped_histograms_ = store.histograms(); + histograms_.reserve(snapped_histograms_.size()); + for (const auto& histogram : snapped_histograms_) { + histograms_.push_back(*histogram); + } + + snapped_text_readouts_ = store.textReadouts(); + text_readouts_.reserve(snapped_text_readouts_.size()); + for (const auto& text_readout : snapped_text_readouts_) { + text_readouts_.push_back(*text_readout); + } + + snapshot_time_ = time_source.systemTime(); +} + +void InstanceUtil::flushMetricsToSinks(const std::list& sinks, Stats::Store& store, + TimeSource& time_source) { + // Create a snapshot and flush to all sinks. + // NOTE: Even if there are no sinks, creating the snapshot has the important property that it + // latches all counters on a periodic basis. The hot restart code assumes this is being + // done so this should not be removed. + MetricSnapshotImpl snapshot(store, time_source); + for (const auto& sink : sinks) { + sink->flush(snapshot); + } +} + +void InstanceImpl::flushStats() { + ENVOY_LOG(debug, "flushing stats"); + // If Envoy is not fully initialized, workers will not be started and mergeHistograms + // completion callback is not called immediately. As a result of this server stats will + // not be updated and flushed to stat sinks. So skip mergeHistograms call if workers are + // not started yet. + if (initManager().state() == Init::Manager::State::Initialized) { + // A shutdown initiated before this callback may prevent this from being called as per + // the semantics documented in ThreadLocal's runOnAllThreads method. + stats_store_.mergeHistograms([this]() -> void { flushStatsInternal(); }); + } else { + ENVOY_LOG(debug, "Envoy is not fully initialized, skipping histogram merge and flushing stats"); + flushStatsInternal(); + } +} + +void InstanceImpl::updateServerStats() { + // mergeParentStatsIfAny() does nothing and returns a struct of 0s if there is no parent. + HotRestart::ServerStatsFromParent parent_stats = restarter_.mergeParentStatsIfAny(stats_store_); + + server_stats_->uptime_.set(time(nullptr) - original_start_time_); + server_stats_->memory_allocated_.set(Memory::Stats::totalCurrentlyAllocated() + + parent_stats.parent_memory_allocated_); + server_stats_->memory_heap_size_.set(Memory::Stats::totalCurrentlyReserved()); + server_stats_->memory_physical_size_.set(Memory::Stats::totalPhysicalBytes()); + server_stats_->parent_connections_.set(parent_stats.parent_connections_); + server_stats_->total_connections_.set(listener_manager_->numConnections() + + parent_stats.parent_connections_); + server_stats_->days_until_first_cert_expiring_.set( + sslContextManager().daysUntilFirstCertExpires()); + + auto secs_until_ocsp_response_expires = + sslContextManager().secondsUntilFirstOcspResponseExpires(); + if (secs_until_ocsp_response_expires) { + server_stats_->seconds_until_first_ocsp_response_expiring_.set( + secs_until_ocsp_response_expires.value()); + } + server_stats_->state_.set( + enumToInt(Utility::serverState(initManager().state(), healthCheckFailed()))); + server_stats_->stats_recent_lookups_.set( + stats_store_.symbolTable().getRecentLookups([](absl::string_view, uint64_t) {})); +} + +void InstanceImpl::flushStatsInternal() { + updateServerStats(); + InstanceUtil::flushMetricsToSinks(config_.statsSinks(), stats_store_, timeSource()); + // TODO(ramaraochavali): consider adding different flush interval for histograms. + if (stat_flush_timer_ != nullptr) { + stat_flush_timer_->enableTimer(config_.statsFlushInterval()); + } +} + +bool InstanceImpl::healthCheckFailed() { return !live_.load(); } + +ProcessContextOptRef InstanceImpl::processContext() { + if (process_context_ == nullptr) { + return absl::nullopt; + } + + return *process_context_; +} + +namespace { +// Loads a bootstrap object, potentially at a specific version (upgrading if necessary). +void loadBootstrap(absl::optional bootstrap_version, + envoy::config::bootstrap::v3::Bootstrap& bootstrap, + std::function load_function) { + + if (!bootstrap_version.has_value()) { + load_function(bootstrap, true); + } else if (*bootstrap_version == 3) { + load_function(bootstrap, false); + } else if (*bootstrap_version == 2) { + envoy::config::bootstrap::v2::Bootstrap bootstrap_v2; + load_function(bootstrap_v2, false); + Config::VersionConverter::upgrade(bootstrap_v2, bootstrap); + MessageUtil::onVersionUpgradeDeprecation("v2 bootstrap", false); + } else { + throw EnvoyException(fmt::format("Unknown bootstrap version {}.", *bootstrap_version)); + } +} +} // namespace + +void InstanceUtil::loadBootstrapConfig(envoy::config::bootstrap::v3::Bootstrap& bootstrap, + const Options& options, + ProtobufMessage::ValidationVisitor& validation_visitor, + Api::Api& api) { + const std::string& config_path = options.configPath(); + const std::string& config_yaml = options.configYaml(); + const envoy::config::bootstrap::v3::Bootstrap& config_proto = options.configProto(); + + // Exactly one of config_path and config_yaml should be specified. + if (config_path.empty() && config_yaml.empty() && config_proto.ByteSize() == 0) { + throw EnvoyException("At least one of --config-path or --config-yaml or Options::configProto() " + "should be non-empty"); + } + + if (!config_path.empty()) { + loadBootstrap( + options.bootstrapVersion(), bootstrap, + [&config_path, &validation_visitor, &api](Protobuf::Message& message, bool do_boosting) { + MessageUtil::loadFromFile(config_path, message, validation_visitor, api, do_boosting); + }); + } + if (!config_yaml.empty()) { + envoy::config::bootstrap::v3::Bootstrap bootstrap_override; + loadBootstrap( + options.bootstrapVersion(), bootstrap_override, + [&config_yaml, &validation_visitor](Protobuf::Message& message, bool do_boosting) { + MessageUtil::loadFromYaml(config_yaml, message, validation_visitor, do_boosting); + }); + // TODO(snowp): The fact that we do a merge here doesn't seem to be covered under test. + bootstrap.MergeFrom(bootstrap_override); + } + if (config_proto.ByteSize() != 0) { + bootstrap.MergeFrom(config_proto); + } + MessageUtil::validate(bootstrap, validation_visitor); +} + +void InstanceImpl::initialize(const Options& options, + Network::Address::InstanceConstSharedPtr local_address, + ComponentFactory& component_factory, ListenerHooks& hooks) { + ENVOY_LOG(info, "initializing epoch {} (base id={}, hot restart version={})", + options.restartEpoch(), restarter_.baseId(), restarter_.version()); + + ENVOY_LOG(info, "statically linked extensions:"); + for (const auto& ext : Envoy::Registry::FactoryCategoryRegistry::registeredFactories()) { + ENVOY_LOG(info, " {}: {}", ext.first, absl::StrJoin(ext.second->registeredNames(), ", ")); + } + + // Handle configuration that needs to take place prior to the main configuration load. + InstanceUtil::loadBootstrapConfig(bootstrap_, options, + messageValidationContext().staticValidationVisitor(), *api_); + bootstrap_config_update_time_ = time_source_.systemTime(); + + // Immediate after the bootstrap has been loaded, override the header prefix, if configured to + // do so. This must be set before any other code block references the HeaderValues ConstSingleton. + if (!bootstrap_.header_prefix().empty()) { + // setPrefix has a release assert verifying that setPrefix() is not called after prefix() + ThreadSafeSingleton::get().setPrefix(bootstrap_.header_prefix().c_str()); + } + // TODO(mattklein123): Custom O(1) headers can be registered at this point for creating/finalizing + // any header maps. + ENVOY_LOG(info, "HTTP header map info:"); + for (const auto& info : Http::HeaderMapImplUtility::getAllHeaderMapImplInfo()) { + ENVOY_LOG(info, " {}: {} bytes: {}", info.name_, info.size_, + absl::StrJoin(info.registered_headers_, ",")); + } + + // Needs to happen as early as possible in the instantiation to preempt the objects that require + // stats. + stats_store_.setTagProducer(Config::Utility::createTagProducer(bootstrap_)); + stats_store_.setStatsMatcher(Config::Utility::createStatsMatcher(bootstrap_)); + stats_store_.setHistogramSettings(Config::Utility::createHistogramSettings(bootstrap_)); + + const std::string server_stats_prefix = "server."; + server_stats_ = std::make_unique( + ServerStats{ALL_SERVER_STATS(POOL_COUNTER_PREFIX(stats_store_, server_stats_prefix), + POOL_GAUGE_PREFIX(stats_store_, server_stats_prefix), + POOL_HISTOGRAM_PREFIX(stats_store_, server_stats_prefix))}); + validation_context_.staticWarningValidationVisitor().setUnknownCounter( + server_stats_->static_unknown_fields_); + validation_context_.dynamicWarningValidationVisitor().setUnknownCounter( + server_stats_->dynamic_unknown_fields_); + + initialization_timer_ = std::make_unique( + server_stats_->initialization_time_ms_, timeSource()); + server_stats_->concurrency_.set(options_.concurrency()); + server_stats_->hot_restart_epoch_.set(options_.restartEpoch()); + + assert_action_registration_ = Assert::setDebugAssertionFailureRecordAction( + [this]() { server_stats_->debug_assertion_failures_.inc(); }); + envoy_bug_action_registration_ = Assert::setEnvoyBugFailureRecordAction( + [this]() { server_stats_->envoy_bug_failures_.inc(); }); + + InstanceImpl::failHealthcheck(false); + + // Check if bootstrap has server version override set, if yes, we should use that as + // 'server.version' stat. + uint64_t version_int; + if (bootstrap_.stats_server_version_override().value() > 0) { + version_int = bootstrap_.stats_server_version_override().value(); + } else { + if (!StringUtil::atoull(VersionInfo::revision().substr(0, 6).c_str(), version_int, 16)) { + throw EnvoyException("compiled GIT SHA is invalid. Invalid build."); + } + } + server_stats_->version_.set(version_int); + + bootstrap_.mutable_node()->set_hidden_envoy_deprecated_build_version(VersionInfo::version()); + bootstrap_.mutable_node()->set_user_agent_name("envoy"); + *bootstrap_.mutable_node()->mutable_user_agent_build_version() = VersionInfo::buildVersion(); + for (const auto& ext : Envoy::Registry::FactoryCategoryRegistry::registeredFactories()) { + for (const auto& name : ext.second->allRegisteredNames()) { + auto* extension = bootstrap_.mutable_node()->add_extensions(); + extension->set_name(std::string(name)); + extension->set_category(ext.first); + auto const version = ext.second->getFactoryVersion(name); + if (version) { + *extension->mutable_version() = version.value(); + } + extension->set_disabled(ext.second->isFactoryDisabled(name)); + } + } + + local_info_ = std::make_unique( + bootstrap_.node(), local_address, options.serviceZone(), options.serviceClusterName(), + options.serviceNodeName()); + + Configuration::InitialImpl initial_config(bootstrap_); + + // Learn original_start_time_ if our parent is still around to inform us of it. + restarter_.sendParentAdminShutdownRequest(original_start_time_); + admin_ = std::make_unique(initial_config.admin().profilePath(), *this); + + loadServerFlags(initial_config.flagsPath()); + + secret_manager_ = std::make_unique(admin_->getConfigTracker()); + + // Initialize the overload manager early so other modules can register for actions. + overload_manager_ = std::make_unique( + *dispatcher_, stats_store_, thread_local_, bootstrap_.overload_manager(), + messageValidationContext().staticValidationVisitor(), *api_); + + heap_shrinker_ = + std::make_unique(*dispatcher_, *overload_manager_, stats_store_); + + for (const auto& bootstrap_extension : bootstrap_.bootstrap_extensions()) { + auto& factory = Config::Utility::getAndCheckFactory( + bootstrap_extension); + auto config = Config::Utility::translateAnyToFactoryConfig( + bootstrap_extension.typed_config(), messageValidationContext().staticValidationVisitor(), + factory); + bootstrap_extensions_.push_back( + factory.createBootstrapExtension(*config, serverFactoryContext())); + } + + // Register the fatal actions. + { + FatalAction::FatalActionPtrList safe_actions; + FatalAction::FatalActionPtrList unsafe_actions; + for (const auto& action_config : bootstrap_.fatal_actions()) { + auto& factory = + Config::Utility::getAndCheckFactory( + action_config.config()); + auto action = factory.createFatalActionFromProto(action_config, this); + + if (action->isAsyncSignalSafe()) { + safe_actions.push_back(std::move(action)); + } else { + unsafe_actions.push_back(std::move(action)); + } + } + Envoy::FatalErrorHandler::registerFatalActions( + std::move(safe_actions), std::move(unsafe_actions), api_->threadFactory()); + } + + if (!bootstrap_.default_socket_interface().empty()) { + auto& sock_name = bootstrap_.default_socket_interface(); + auto sock = const_cast(Network::socketInterface(sock_name)); + if (sock != nullptr) { + Network::SocketInterfaceSingleton::clear(); + Network::SocketInterfaceSingleton::initialize(sock); + } + } + + // Workers get created first so they register for thread local updates. + listener_manager_ = std::make_unique( + *this, listener_component_factory_, worker_factory_, bootstrap_.enable_dispatcher_stats()); + + // The main thread is also registered for thread local updates so that code that does not care + // whether it runs on the main thread or on workers can still use TLS. + thread_local_.registerThread(*dispatcher_, true); + + // We can now initialize stats for threading. + stats_store_.initializeThreading(*dispatcher_, thread_local_); + + // It's now safe to start writing stats from the main thread's dispatcher. + if (bootstrap_.enable_dispatcher_stats()) { + dispatcher_->initializeStats(stats_store_, "server."); + } + + if (initial_config.admin().address()) { + if (initial_config.admin().accessLogPath().empty()) { + throw EnvoyException("An admin access log path is required for a listening server."); + } + ENVOY_LOG(info, "admin address: {}", initial_config.admin().address()->asString()); + admin_->startHttpListener(initial_config.admin().accessLogPath(), options.adminAddressPath(), + initial_config.admin().address(), + initial_config.admin().socketOptions(), + stats_store_.createScope("listener.admin.")); + } else { + ENVOY_LOG(warn, "No admin address given, so no admin HTTP server started."); + } + config_tracker_entry_ = + admin_->getConfigTracker().add("bootstrap", [this] { return dumpBootstrapConfig(); }); + if (initial_config.admin().address()) { + admin_->addListenerToHandler(handler_.get()); + } + + // The broad order of initialization from this point on is the following: + // 1. Statically provisioned configuration (bootstrap) are loaded. + // 2. Cluster manager is created and all primary clusters (i.e. with endpoint assignments + // provisioned statically in bootstrap, discovered through DNS or file based CDS) are + // initialized. + // 3. Various services are initialized and configured using the bootstrap config. + // 4. RTDS is initialized using primary clusters. This allows runtime overrides to be fully + // configured before the rest of xDS configuration is provisioned. + // 5. Secondary clusters (with endpoint assignments provisioned by xDS servers) are initialized. + // 6. The rest of the dynamic configuration is provisioned. + // + // Please note: this order requires that RTDS is provisioned using a primary cluster. If RTDS is + // provisioned through ADS then ADS must use primary cluster as well. This invariant is enforced + // during RTDS initialization and invalid configuration will be rejected. + + // Runtime gets initialized before the main configuration since during main configuration + // load things may grab a reference to the loader for later use. + runtime_singleton_ = std::make_unique( + component_factory.createRuntime(*this, initial_config)); + hooks.onRuntimeCreated(); + + // Once we have runtime we can initialize the SSL context manager. + ssl_context_manager_ = createContextManager("ssl_context_manager", time_source_); + + const bool use_tcp_for_dns_lookups = bootstrap_.use_tcp_for_dns_lookups(); + dns_resolver_ = dispatcher_->createDnsResolver({}, use_tcp_for_dns_lookups); + + cluster_manager_factory_ = std::make_unique( + *admin_, Runtime::LoaderSingleton::get(), stats_store_, thread_local_, dns_resolver_, + *ssl_context_manager_, *dispatcher_, *local_info_, *secret_manager_, + messageValidationContext(), *api_, http_context_, grpc_context_, access_log_manager_, + *singleton_manager_); + + // Now the configuration gets parsed. The configuration may start setting + // thread local data per above. See MainImpl::initialize() for why ConfigImpl + // is constructed as part of the InstanceImpl and then populated once + // cluster_manager_factory_ is available. + config_.initialize(bootstrap_, *this, *cluster_manager_factory_); + + // Instruct the listener manager to create the LDS provider if needed. This must be done later + // because various items do not yet exist when the listener manager is created. + if (bootstrap_.dynamic_resources().has_lds_config() || + bootstrap_.dynamic_resources().has_lds_resources_locator()) { + listener_manager_->createLdsApi(bootstrap_.dynamic_resources().lds_config(), + bootstrap_.dynamic_resources().has_lds_resources_locator() + ? &bootstrap_.dynamic_resources().lds_resources_locator() + : nullptr); + } + + // We have to defer RTDS initialization until after the cluster manager is + // instantiated (which in turn relies on runtime...). + Runtime::LoaderSingleton::get().initialize(clusterManager()); + + clusterManager().setPrimaryClustersInitializedCb( + [this]() { onClusterManagerPrimaryInitializationComplete(); }); + + for (Stats::SinkPtr& sink : config_.statsSinks()) { + stats_store_.addSink(*sink); + } + + // Some of the stat sinks may need dispatcher support so don't flush until the main loop starts. + // Just setup the timer. + stat_flush_timer_ = dispatcher_->createTimer([this]() -> void { flushStats(); }); + stat_flush_timer_->enableTimer(config_.statsFlushInterval()); + + // GuardDog (deadlock detection) object and thread setup before workers are + // started and before our own run() loop runs. + main_thread_guard_dog_ = std::make_unique( + stats_store_, config_.mainThreadWatchdogConfig(), *api_, "main_thread"); + worker_guard_dog_ = std::make_unique( + stats_store_, config_.workerWatchdogConfig(), *api_, "workers"); +} + +void InstanceImpl::onClusterManagerPrimaryInitializationComplete() { + // If RTDS was not configured the `onRuntimeReady` callback is immediately invoked. + Runtime::LoaderSingleton::get().startRtdsSubscriptions([this]() { onRuntimeReady(); }); +} + +void InstanceImpl::onRuntimeReady() { + // Begin initializing secondary clusters after RTDS configuration has been applied. + clusterManager().initializeSecondaryClusters(bootstrap_); + + if (bootstrap_.has_hds_config()) { + const auto& hds_config = bootstrap_.hds_config(); + async_client_manager_ = std::make_unique( + *config_.clusterManager(), thread_local_, time_source_, *api_, grpc_context_.statNames()); + hds_delegate_ = std::make_unique( + stats_store_, + Config::Utility::factoryForGrpcApiConfigSource(*async_client_manager_, hds_config, + stats_store_, false) + ->create(), + hds_config.transport_api_version(), *dispatcher_, Runtime::LoaderSingleton::get(), + stats_store_, *ssl_context_manager_, info_factory_, access_log_manager_, + *config_.clusterManager(), *local_info_, *admin_, *singleton_manager_, thread_local_, + messageValidationContext().dynamicValidationVisitor(), *api_); + } + + // If there is no global limit to the number of active connections, warn on startup. + // TODO (tonya11en): Move this functionality into the overload manager. + if (!runtime().snapshot().get(Network::TcpListenerImpl::GlobalMaxCxRuntimeKey)) { + ENVOY_LOG(warn, + "there is no configured limit to the number of allowed active connections. Set a " + "limit via the runtime key {}", + Network::TcpListenerImpl::GlobalMaxCxRuntimeKey); + } +} + +void InstanceImpl::startWorkers() { + listener_manager_->startWorkers(*worker_guard_dog_); + initialization_timer_->complete(); + // Update server stats as soon as initialization is done. + updateServerStats(); + workers_started_ = true; + // At this point we are ready to take traffic and all listening ports are up. Notify our parent + // if applicable that they can stop listening and drain. + restarter_.drainParentListeners(); + drain_manager_->startParentShutdownSequence(); +} + +Runtime::LoaderPtr InstanceUtil::createRuntime(Instance& server, + Server::Configuration::Initial& config) { + ENVOY_LOG(info, "runtime: {}", MessageUtil::getYamlStringFromMessage(config.runtime())); + return std::make_unique( + server.dispatcher(), server.threadLocal(), config.runtime(), server.localInfo(), + server.stats(), server.api().randomGenerator(), + server.messageValidationContext().dynamicValidationVisitor(), server.api()); +} + +void InstanceImpl::loadServerFlags(const absl::optional& flags_path) { + if (!flags_path) { + return; + } + + ENVOY_LOG(info, "server flags path: {}", flags_path.value()); + if (api_->fileSystem().fileExists(flags_path.value() + "/drain")) { + ENVOY_LOG(info, "starting server in drain mode"); + InstanceImpl::failHealthcheck(true); + } +} + +RunHelper::RunHelper(Instance& instance, const Options& options, Event::Dispatcher& dispatcher, + Upstream::ClusterManager& cm, AccessLog::AccessLogManager& access_log_manager, + Init::Manager& init_manager, OverloadManager& overload_manager, + std::function post_init_cb) + : init_watcher_("RunHelper", [&instance, post_init_cb]() { + if (!instance.isShutdown()) { + post_init_cb(); + } + }) { + // Setup signals. + // Since signals are not supported on Windows we have an internal definition for `SIGTERM` + // On POSIX it resolves as expected to SIGTERM + // On Windows we use it internally for all the console events that indicate that we should + // terminate the process. + if (options.signalHandlingEnabled()) { + sigterm_ = dispatcher.listenForSignal(ENVOY_SIGTERM, [&instance]() { + ENVOY_LOG(warn, "caught ENVOY_SIGTERM"); + instance.shutdown(); + }); +#ifndef WIN32 + sigint_ = dispatcher.listenForSignal(SIGINT, [&instance]() { + ENVOY_LOG(warn, "caught SIGINT"); + instance.shutdown(); + }); + + sig_usr_1_ = dispatcher.listenForSignal(SIGUSR1, [&access_log_manager]() { + ENVOY_LOG(info, "caught SIGUSR1. Reopening access logs."); + access_log_manager.reopen(); + }); + + sig_hup_ = dispatcher.listenForSignal(SIGHUP, []() { + ENVOY_LOG(warn, "caught and eating SIGHUP. See documentation for how to hot restart."); + }); +#endif + } + + // Start overload manager before workers. + overload_manager.start(); + + // Register for cluster manager init notification. We don't start serving worker traffic until + // upstream clusters are initialized which may involve running the event loop. Note however that + // this can fire immediately if all clusters have already initialized. Also note that we need + // to guard against shutdown at two different levels since SIGTERM can come in once the run loop + // starts. + cm.setInitializedCb([&instance, &init_manager, &cm, this]() { + if (instance.isShutdown()) { + return; + } + + const auto type_urls = + Config::getAllVersionTypeUrls(); + // Pause RDS to ensure that we don't send any requests until we've + // subscribed to all the RDS resources. The subscriptions happen in the init callbacks, + // so we pause RDS until we've completed all the callbacks. + Config::ScopedResume maybe_resume_rds; + if (cm.adsMux()) { + maybe_resume_rds = cm.adsMux()->pause(type_urls); + } + + ENVOY_LOG(info, "all clusters initialized. initializing init manager"); + init_manager.initialize(init_watcher_); + + // Now that we're execute all the init callbacks we can resume RDS + // as we've subscribed to all the statically defined RDS resources. + // This is done by tearing down the maybe_resume_rds Cleanup object. + }); +} + +void InstanceImpl::run() { + // RunHelper exists primarily to facilitate testing of how we respond to early shutdown during + // startup (see RunHelperTest in server_test.cc). + const auto run_helper = RunHelper(*this, options_, *dispatcher_, clusterManager(), + access_log_manager_, init_manager_, overloadManager(), [this] { + notifyCallbacksForStage(Stage::PostInit); + startWorkers(); + }); + + // Run the main dispatch loop waiting to exit. + ENVOY_LOG(info, "starting main dispatch loop"); + auto watchdog = main_thread_guard_dog_->createWatchDog(api_->threadFactory().currentThreadId(), + "main_thread", *dispatcher_); + dispatcher_->post([this] { notifyCallbacksForStage(Stage::Startup); }); + dispatcher_->run(Event::Dispatcher::RunType::Block); + ENVOY_LOG(info, "main dispatch loop exited"); + main_thread_guard_dog_->stopWatching(watchdog); + watchdog.reset(); + + terminate(); +} + +void InstanceImpl::terminate() { + if (terminated_) { + return; + } + terminated_ = true; + + // Before starting to shutdown anything else, stop slot destruction updates. + thread_local_.shutdownGlobalThreading(); + + // Before the workers start exiting we should disable stat threading. + stats_store_.shutdownThreading(); + + if (overload_manager_) { + overload_manager_->stop(); + } + + // Shutdown all the workers now that the main dispatch loop is done. + if (listener_manager_ != nullptr) { + // Also shutdown the listener manager's ApiListener, if there is one, which runs on the main + // thread. This needs to happen ahead of calling thread_local_.shutdown() below to prevent any + // objects in the ApiListener destructor to reference any objects in thread local storage. + if (listener_manager_->apiListener().has_value()) { + listener_manager_->apiListener()->get().shutdown(); + } + + listener_manager_->stopWorkers(); + } + + // Only flush if we have not been hot restarted. + if (stat_flush_timer_) { + flushStats(); + } + + if (config_.clusterManager() != nullptr) { + config_.clusterManager()->shutdown(); + } + handler_.reset(); + thread_local_.shutdownThread(); + restarter_.shutdown(); + ENVOY_LOG(info, "exiting"); + ENVOY_FLUSH_LOG(); + FatalErrorHandler::clearFatalActionsOnTerminate(); +} + +Runtime::Loader& InstanceImpl::runtime() { return Runtime::LoaderSingleton::get(); } + +void InstanceImpl::shutdown() { + ENVOY_LOG(info, "shutting down server instance"); + shutdown_ = true; + restarter_.sendParentTerminateRequest(); + notifyCallbacksForStage(Stage::ShutdownExit, [this] { dispatcher_->exit(); }); +} + +void InstanceImpl::shutdownAdmin() { + ENVOY_LOG(warn, "shutting down admin due to child startup"); + stat_flush_timer_.reset(); + handler_->stopListeners(); + admin_->closeSocket(); + + // If we still have a parent, it should be terminated now that we have a child. + ENVOY_LOG(warn, "terminating parent process"); + restarter_.sendParentTerminateRequest(); +} + +ServerLifecycleNotifier::HandlePtr InstanceImpl::registerCallback(Stage stage, + StageCallback callback) { + auto& callbacks = stage_callbacks_[stage]; + return std::make_unique>(callbacks, callback); +} + +ServerLifecycleNotifier::HandlePtr +InstanceImpl::registerCallback(Stage stage, StageCallbackWithCompletion callback) { + ASSERT(stage == Stage::ShutdownExit); + auto& callbacks = stage_completable_callbacks_[stage]; + return std::make_unique>(callbacks, + callback); +} + +void InstanceImpl::notifyCallbacksForStage(Stage stage, Event::PostCb completion_cb) { + ASSERT(std::this_thread::get_id() == main_thread_id_); + const auto it = stage_callbacks_.find(stage); + if (it != stage_callbacks_.end()) { + for (const StageCallback& callback : it->second) { + callback(); + } + } + + // Wrap completion_cb so that it only gets invoked when all callbacks for this stage + // have finished their work. + std::shared_ptr cb_guard( + new Cleanup([this, completion_cb]() { dispatcher_->post(completion_cb); })); + + // Registrations which take a completion callback are typically implemented by executing a + // callback on all worker threads using Slot::runOnAllThreads which will hang indefinitely if + // worker threads have not been started so we need to skip notifications if envoy is shutdown + // early before workers have started. + if (workers_started_) { + const auto it2 = stage_completable_callbacks_.find(stage); + if (it2 != stage_completable_callbacks_.end()) { + ENVOY_LOG(info, "Notifying {} callback(s) with completion.", it2->second.size()); + for (const StageCallbackWithCompletion& callback : it2->second) { + callback([cb_guard] {}); + } + } + } +} + +ProtobufTypes::MessagePtr InstanceImpl::dumpBootstrapConfig() { + auto config_dump = std::make_unique(); + config_dump->mutable_bootstrap()->MergeFrom(bootstrap_); + TimestampUtil::systemClockToTimestamp(bootstrap_config_update_time_, + *(config_dump->mutable_last_updated())); + return config_dump; +} + +} // namespace Server +} // namespace Envoy From 63e7ced9050bf7ff20100b07b1ca3e9eceb24523 Mon Sep 17 00:00:00 2001 From: Sotiris Nanopoulos Date: Mon, 7 Dec 2020 11:10:21 -0800 Subject: [PATCH 32/33] fix allocation + add panic Signed-off-by: Sotiris Nanopoulos --- source/common/event/win32/signal_impl.cc | 5 ++--- source/exe/win32/platform_impl.cc | 7 +++---- source/server/server.cc | 10 ++++++++-- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/source/common/event/win32/signal_impl.cc b/source/common/event/win32/signal_impl.cc index 11e63e38a54d..4ffd62b73094 100644 --- a/source/common/event/win32/signal_impl.cc +++ b/source/common/event/win32/signal_impl.cc @@ -11,9 +11,8 @@ SignalEventImpl::SignalEventImpl(DispatcherImpl& dispatcher, signal_t signal_num : cb_(cb) { if (signal_num > eventBridgeHandlersSingleton::get().size()) { - ENVOY_BUG(false, "Attempting to create SignalEventImpl with a signal id that exceeds the " - "number of supported signals."); - return; + PANIC("Attempting to create SignalEventImpl with a signal id that exceeds the number of " + "supported signals."); } if (eventBridgeHandlersSingleton::get()[signal_num]) { diff --git a/source/exe/win32/platform_impl.cc b/source/exe/win32/platform_impl.cc index ee729b5a167b..52107ae546fe 100644 --- a/source/exe/win32/platform_impl.cc +++ b/source/exe/win32/platform_impl.cc @@ -28,10 +28,9 @@ BOOL WINAPI CtrlHandler(DWORD fdwCtrlType) { // managed by Windows console host. For that reason we want to avoid allocating // substantial amount of memory or taking locks. // This is why we write to a socket to wake up the signal handler. - Buffer::OwnedImpl buffer; - constexpr absl::string_view data{"a"}; - buffer.add(data); - auto result = handler->write(buffer); + char data[] = {'a'}; + Buffer::RawSlice buffer{data, 1}; + auto result = handler->writev(&buffer, 1); RELEASE_ASSERT(result.rc_ == 1, fmt::format("failed to write 1 byte: {}", result.err_->getErrorDetails())); diff --git a/source/server/server.cc b/source/server/server.cc index 9b9606b3f4e1..4ec8b52a2954 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -648,9 +648,15 @@ RunHelper::RunHelper(Instance& instance, const Options& options, Event::Dispatch } }) { // Setup signals. + // Since signals are not supported on Windows we have an internal definition for `SIGTERM` + // On POSIX it resolves as expected to SIGTERM + // On Windows we use it internally for all the console events that indicate that we should + // terminate the process. if (options.signalHandlingEnabled()) { -// TODO(Pivotal): Figure out solution to graceful shutdown on Windows. None of these signals exist -// on Windows. + sigterm_ = dispatcher.listenForSignal(ENVOY_SIGTERM, [&instance]() { + ENVOY_LOG(warn, "caught ENVOY_SIGTERM"); + instance.shutdown(); + }); #ifndef WIN32 sigterm_ = dispatcher.listenForSignal(SIGTERM, [&instance]() { ENVOY_LOG(warn, "caught SIGTERM"); From 1f1e2119e0091b0e42a1d9ab7d81f4f34b2a8924 Mon Sep 17 00:00:00 2001 From: Sotiris Nanopoulos Date: Mon, 7 Dec 2020 12:47:32 -0800 Subject: [PATCH 33/33] remove double signal entry Signed-off-by: Sotiris Nanopoulos --- source/server/server.cc | 5 ----- 1 file changed, 5 deletions(-) diff --git a/source/server/server.cc b/source/server/server.cc index 4ec8b52a2954..df6c430bea53 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -658,11 +658,6 @@ RunHelper::RunHelper(Instance& instance, const Options& options, Event::Dispatch instance.shutdown(); }); #ifndef WIN32 - sigterm_ = dispatcher.listenForSignal(SIGTERM, [&instance]() { - ENVOY_LOG(warn, "caught SIGTERM"); - instance.shutdown(); - }); - sigint_ = dispatcher.listenForSignal(SIGINT, [&instance]() { ENVOY_LOG(warn, "caught SIGINT"); instance.shutdown();