bug core: revert "catch signals before component system is up and running"

1. Exceptions during startup are swallowed, causing the service to hang on start instead of shutting down
2. There is a potential data race in `manager.OnSignal`
commit_hash:091ba960c3fe23b6dc4d2c25a70e6e1388e3ea4e

Author: Anton3

.mapping.json

@@ -758,11 +758,6 @@
   "core/functional_tests/metrics/tests/conftest.py":"taxi/uservices/userver/core/functional_tests/metrics/tests/conftest.py",
   "core/functional_tests/metrics/tests/static/metrics_values.txt":"taxi/uservices/userver/core/functional_tests/metrics/tests/static/metrics_values.txt",
   "core/functional_tests/metrics/tests/test_metrics.py":"taxi/uservices/userver/core/functional_tests/metrics/tests/test_metrics.py",
-  "core/functional_tests/signal_during_boot/CMakeLists.txt":"taxi/uservices/userver/core/functional_tests/signal_during_boot/CMakeLists.txt",
-  "core/functional_tests/signal_during_boot/service.cpp":"taxi/uservices/userver/core/functional_tests/signal_during_boot/service.cpp",
-  "core/functional_tests/signal_during_boot/static_config.yaml":"taxi/uservices/userver/core/functional_tests/signal_during_boot/static_config.yaml",
-  "core/functional_tests/signal_during_boot/tests/conftest.py":"taxi/uservices/userver/core/functional_tests/signal_during_boot/tests/conftest.py",
-  "core/functional_tests/signal_during_boot/tests/test_rotate_logs.py":"taxi/uservices/userver/core/functional_tests/signal_during_boot/tests/test_rotate_logs.py",
   "core/functional_tests/slow_start/CMakeLists.txt":"taxi/uservices/userver/core/functional_tests/slow_start/CMakeLists.txt",
   "core/functional_tests/slow_start/config_vars.yaml":"taxi/uservices/userver/core/functional_tests/slow_start/config_vars.yaml",
   "core/functional_tests/slow_start/secure_data.json":"taxi/uservices/userver/core/functional_tests/slow_start/secure_data.json",

core/functional_tests/CMakeLists.txt

@@ -35,9 +35,6 @@ add_dependencies(${PROJECT_NAME} ${PROJECT_NAME}-static-service)
 add_subdirectory(slow_start)
 add_dependencies(${PROJECT_NAME} ${PROJECT_NAME}-slow-start)
 
-add_subdirectory(signal_during_boot)
-add_dependencies(${PROJECT_NAME} ${PROJECT_NAME}-signal-during-boot)
-
 add_subdirectory(tracing)
 add_dependencies(${PROJECT_NAME} ${PROJECT_NAME}-tracing)
 

core/functional_tests/signal_during_boot/CMakeLists.txt

@@ -152,7 +152,11 @@ void Manager::TaskProcessorsStorage::WaitForAllTasksBlocking() const noexcept {
     }
 }
 
-Manager::Manager(std::unique_ptr<ManagerConfig>&& config, std::chrono::steady_clock::time_point start_time)
+Manager::Manager(
+    std::unique_ptr<ManagerConfig>&& config,
+    std::chrono::steady_clock::time_point start_time,
+    const ComponentList& component_list
+)
     : config_(std::move(config)),
       task_processors_storage_(std::make_shared<
                                engine::impl::TaskProcessorPools>(config_->coro_pool, config_->event_thread_pool)),
@@ -215,18 +219,15 @@ Manager::Manager(std::unique_ptr<ManagerConfig>&& config, std::chrono::steady_cl
     }
 
     default_task_processor_ = default_task_processor_it->second.get();
-}
+    RunInCoro(*default_task_processor_, [this, &component_list]() { CreateComponentContext(component_list); });
 
-engine::TaskWithResult<void> Manager::StartComponentSystem(const ComponentList& component_list) {
-    return engine::CriticalAsyncNoSpan(*default_task_processor_, [this, &component_list]() {
-        CreateComponentContext(component_list);
-        if (!config_->disable_phdr_cache) {
-            engine::impl::InitPhdrCache();
-        }
-        LOG_INFO()
-            << "Started components manager. All the components have started "
-               "successfully.";
-    });
+    if (!config_->disable_phdr_cache) {
+        engine::impl::InitPhdrCache();
+    }
+
+    LOG_INFO()
+        << "Started components manager. All the components have started "
+           "successfully.";
 }
 
 Manager::~Manager() {
@@ -324,10 +325,6 @@ void Manager::CreateComponentContext(const ComponentList& component_list) {
     component_context_ = std::make_unique<impl::ComponentContextImpl>(*this, std::move(loading_components));
 
     AddComponents(component_list);
-
-    LOG_INFO()
-        << "Started components manager. All the components have started "
-           "successfully.";
 }
 
 components::ComponentConfigMap Manager::MakeComponentConfigMap(const ComponentList& component_list) {

core/functional_tests/signal_during_boot/service.cpp

@@ -14,14 +14,9 @@
 
 USERVER_NAMESPACE_BEGIN
 
-namespace engine {
-template <typename T>
-class TaskWithResult;
-
-namespace impl {
+namespace engine::impl {
 class TaskProcessorPools;
-}  // namespace impl
-}  // namespace engine
+}  // namespace engine::impl
 
 namespace os_signals {
 class ProcessorComponent;
@@ -43,10 +38,13 @@ using TaskProcessorsMap = utils::impl::TransparentMap<std::string, std::unique_p
 // to the outside world.
 class Manager final {
 public:
-    Manager(std::unique_ptr<ManagerConfig>&& config, std::chrono::steady_clock::time_point start_time);
+    Manager(
+        std::unique_ptr<ManagerConfig>&& config,
+        std::chrono::steady_clock::time_point start_time,
+        const ComponentList& component_list
+    );
     ~Manager();
 
-    engine::TaskWithResult<void> StartComponentSystem(const ComponentList& component_list);
     const ManagerConfig& GetConfig() const;
     const std::shared_ptr<engine::impl::TaskProcessorPools>& GetTaskProcessorPools() const;
     const TaskProcessorsMap& GetTaskProcessorsMap() const;

core/functional_tests/signal_during_boot/static_config.yaml

@@ -13,8 +13,6 @@
 #include <userver/crypto/openssl.hpp>
 #include <userver/dynamic_config/snapshot.hpp>
 #include <userver/dynamic_config/value.hpp>
-#include <userver/engine/exception.hpp>
-#include <userver/engine/task/task_with_result.hpp>
 #include <userver/formats/json/serialize.hpp>
 #include <userver/fs/blocking/read.hpp>
 #include <userver/logging/impl/mem_logger.hpp>
@@ -185,33 +183,6 @@ ManagerConfig ParseManagerConfigAndSetupLogging(
     }
 }
 
-void CatchSignalsLoop(impl::Manager& manager, RunMode run_mode, utils::SignalCatcher& signal_catcher) noexcept {
-    if (run_mode == RunMode::kOnce) {
-        return;
-    }
-
-    LOG_INFO() << "Starting to catch signals";
-    for (;;) {
-        auto signum = signal_catcher.Catch();
-        if (signum == SIGTERM || signum == SIGQUIT) {
-            break;
-        } else if (signum == SIGINT) {
-            if (IsTraced()) {
-                // SIGINT is masked and cannot be used
-                std::raise(SIGTRAP);
-            } else {
-                break;
-            }
-        } else if (signum == SIGUSR1 || signum == SIGUSR2) {
-            LOG_INFO() << "Signal caught: " << utils::strsignal(signum);
-            manager.OnSignal(signum);
-        } else {
-            LOG_WARNING() << "Got unexpected signal: " << signum << " (" << utils::strsignal(signum) << ')';
-            UASSERT_MSG(false, "unexpected signal");
-        }
-    }
-}
-
 void DoRun(
     const PathOrConfig& config,
     const std::optional<std::string>& config_vars_path,
@@ -246,29 +217,35 @@ void DoRun(
             PreheatStacktraceCollector();
         }
 
-        manager.emplace(std::make_unique<ManagerConfig>(std::move(manager_config)), start_time);
-
-        // Start component system in background.
-        // POSIX signals can be already handled while component system is loading.
-        auto start_components_task = manager->StartComponentSystem(component_list);
-
-        // The main event loop.
-        // Other threads handle coroutines.
-        CatchSignalsLoop(*manager, run_mode, signal_catcher);
-
-        if (run_mode == RunMode::kNormal) {
-            start_components_task.RequestCancel();
-        }
-
-        try {
-            start_components_task.BlockingWait();
-            start_components_task.Get();
-        } catch (const engine::WaitInterruptedException&) {
-        }
+        manager.emplace(std::make_unique<ManagerConfig>(std::move(manager_config)), start_time, component_list);
     } catch (const std::exception& ex) {
         LOG_ERROR() << "Loading failed: " << ex;
         throw;
     }
+
+    if (run_mode == RunMode::kOnce) {
+        return;
+    }
+
+    for (;;) {
+        auto signum = signal_catcher.Catch();
+        if (signum == SIGTERM || signum == SIGQUIT) {
+            break;
+        } else if (signum == SIGINT) {
+            if (IsTraced()) {
+                // SIGINT is masked and cannot be used
+                std::raise(SIGTRAP);
+            } else {
+                break;
+            }
+        } else if (signum == SIGUSR1 || signum == SIGUSR2) {
+            LOG_INFO() << "Signal caught: " << utils::strsignal(signum);
+            manager->OnSignal(signum);
+        } else {
+            LOG_WARNING() << "Got unexpected signal: " << signum << " (" << utils::strsignal(signum) << ')';
+            UASSERT_MSG(false, "unexpected signal");
+        }
+    }
 }
 
 }  // namespace