This is an automated email from the ASF dual-hosted git repository. pengzheng pushed a commit to branch feature/215-stop-launcher-by-signal in repository https://gitbox.apache.org/repos/asf/celix.git
commit 8f4fafcdfa4449d31eaa591404a8ad9e88c2b1b5 Author: PengZheng <[email protected]> AuthorDate: Wed Jun 12 15:26:51 2024 +0800 gh-215: Implement async-signal-safe launcher shutdown. --- libs/framework/gtest/src/CelixLauncherTestSuite.cc | 20 +++- .../framework/gtest/src/ScheduledEventTestSuite.cc | 14 +++ libs/framework/include/celix_launcher.h | 9 ++ libs/framework/src/celix_launcher.c | 106 +++++++++++++++------ libs/framework/src/celix_launcher_private.h | 9 -- libs/framework/src/framework.c | 7 ++ 6 files changed, 122 insertions(+), 43 deletions(-) diff --git a/libs/framework/gtest/src/CelixLauncherTestSuite.cc b/libs/framework/gtest/src/CelixLauncherTestSuite.cc index 55fc2cfbb..d422d577c 100644 --- a/libs/framework/gtest/src/CelixLauncherTestSuite.cc +++ b/libs/framework/gtest/src/CelixLauncherTestSuite.cc @@ -19,10 +19,12 @@ #include <gtest/gtest.h> +#include <csignal> #include <thread> #include <future> #include <vector> #include <string> +#include <pthread.h> #include "celix_constants.h" #include "celix_launcher.h" @@ -190,19 +192,31 @@ TEST_F(CelixLauncherTestSuite, LaunchWithInvalidConfigPropertiesTest) { //Then the launch will exit auto status = future.wait_for(std::chrono::milliseconds{LAUNCH_WAIT_TIMEOUT}); EXPECT_EQ(status, std::future_status::ready); + + //When launching the framework with a properties set with a negative shutdown period + auto* props = celix_properties_create(); + ASSERT_TRUE(props != nullptr); + celix_properties_setDouble(props, CELIX_LAUNCHER_SHUTDOWN_PERIOD_IN_SECONDS, -1.0); + future = launchInThread({"programName"}, props, 1); + //Then launch will exit + status = future.wait_for(std::chrono::milliseconds{LAUNCH_WAIT_TIMEOUT}); + EXPECT_EQ(status, std::future_status::ready); } + TEST_F(CelixLauncherTestSuite, StopLauncherWithSignalTest) { + auto* props = celix_properties_create(); // When launching the framework - auto future = launchInThread({"programName"}, nullptr, 0); + celix_properties_setDouble(props, CELIX_LAUNCHER_SHUTDOWN_PERIOD_IN_SECONDS, 0.01); + auto future = launchInThread({"programName"}, props, 0); // Then the launch will not exit, because the framework is running auto status = future.wait_for(std::chrono::milliseconds{LAUNCH_WAIT_TIMEOUT}); EXPECT_EQ(status, std::future_status::timeout); - // When I stop the framework by mimicking a SIGINT signal + // When I stop the framework by sending a SIGINT signal int signal = SIGINT; - celix_launcher_stopInternal(&signal); + EXPECT_EQ(0, pthread_kill(pthread_self(), signal)); // Then the launch will exit status = future.wait_for(std::chrono::milliseconds{LAUNCH_WAIT_TIMEOUT}); diff --git a/libs/framework/gtest/src/ScheduledEventTestSuite.cc b/libs/framework/gtest/src/ScheduledEventTestSuite.cc index 56949ff6d..9edbbb282 100644 --- a/libs/framework/gtest/src/ScheduledEventTestSuite.cc +++ b/libs/framework/gtest/src/ScheduledEventTestSuite.cc @@ -231,7 +231,21 @@ TEST_F(ScheduledEventTestSuite, InvalidOptionsAndArgumentsTest) { auto ctx = fw->getFrameworkBundleContext(); celix_scheduled_event_options_t opts{}; // no callback long scheduledEventId = celix_bundleContext_scheduleEvent(ctx->getCBundleContext(), &opts); + // Then I expect an error + EXPECT_LT(scheduledEventId, 0); + + // When I create a scheduled event with negative initial delay + opts.name = "Invalid scheduled event test"; + opts.initialDelayInSeconds = -1; + opts.callback = [](void* /*data*/) { FAIL() << "Scheduled event called, but should not be called"; }; + scheduledEventId = celix_bundleContext_scheduleEvent(ctx->getCBundleContext(), &opts); + // Then I expect an error + EXPECT_LT(scheduledEventId, 0); + // When I create a scheduled event with negative interval value + opts.initialDelayInSeconds = 0.1; + opts.intervalInSeconds = -1; + scheduledEventId = celix_bundleContext_scheduleEvent(ctx->getCBundleContext(), &opts); // Then I expect an error EXPECT_LT(scheduledEventId, 0); diff --git a/libs/framework/include/celix_launcher.h b/libs/framework/include/celix_launcher.h index 97c6fa870..0db497e9e 100644 --- a/libs/framework/include/celix_launcher.h +++ b/libs/framework/include/celix_launcher.h @@ -28,6 +28,15 @@ extern "C" { #endif +/** + * @brief Celix launcher environment property to specify interval of the periodic shutdown check performed by launcher. + * + * The launcher will perform a periodic check to see whether to perform a shutdown, and if so, the launcher will + * stop and destroy the framework. The interval of this check can be specified in seconds using this property. + */ +#define CELIX_LAUNCHER_SHUTDOWN_PERIOD_IN_SECONDS "CELIX_LAUNCHER_SHUTDOWN_PERIOD_IN_SECONDS" +#define CELIX_LAUNCHER_SHUTDOWN_PERIOD_IN_SECONDS_DEFAULT 1.0 + /** * @brief Launch a celix framework, wait (block) until the framework is stopped and destroy the framework when stopped. * diff --git a/libs/framework/src/celix_launcher.c b/libs/framework/src/celix_launcher.c index e0bb06c73..e706c4490 100644 --- a/libs/framework/src/celix_launcher.c +++ b/libs/framework/src/celix_launcher.c @@ -19,6 +19,7 @@ #include "celix_launcher.h" #include "celix_launcher_private.h" +#include "celix_compiler.h" #include <signal.h> #include <stdio.h> @@ -36,14 +37,21 @@ #include "celix_file_utils.h" #include "celix_framework_factory.h" #include "celix_framework_utils.h" +#include "celix_threads.h" #define DEFAULT_CONFIG_FILE "config.properties" #define CELIX_LAUNCHER_OK_EXIT_CODE 0 #define CELIX_LAUNCHER_ERROR_EXIT_CODE 1 -static bool g_framework_launched = false; -static framework_t* g_framework = NULL; +typedef struct { + celix_thread_mutex_t lock; // protect the following + framework_t* framework; + long shutdownEventId; + bool launched; + bool shutdown; + int signal; +} celix_launcher_t; typedef struct { bool showHelp; @@ -52,6 +60,8 @@ typedef struct { const char* configFile; } celix_launcher_options_t; +static celix_launcher_t g_launcher = { PTHREAD_MUTEX_INITIALIZER, NULL, -1L, false, false, -1 }; + /** * @brief SIGUSR1 SIGUSR2 no-op callback handler. */ @@ -120,7 +130,7 @@ static celix_status_t celix_launcher_loadRuntimeProperties(const char* configFil /** * @brief Set the global framework instance. */ -static void celix_launcher_setGlobalFramework(celix_framework_t* fw); +static celix_status_t celix_launcher_setGlobalFramework(celix_framework_t* fw); int celix_launcher_launchAndWait(int argc, char* argv[], const char* embeddedConfig) { celix_autoptr(celix_properties_t) embeddedProps = NULL; @@ -167,18 +177,18 @@ int celix_launcher_launchAndWait(int argc, char* argv[], const char* embeddedCon celix_framework_t* framework = NULL; status = celix_launcher_createFramework(celix_steal_ptr(embeddedProps), runtimeProps, &framework); + if (status != CELIX_SUCCESS) { + return CELIX_LAUNCHER_ERROR_EXIT_CODE; + } + status = celix_launcher_setGlobalFramework(framework); if (status == CELIX_SUCCESS) { - celix_launcher_setGlobalFramework(framework); celix_framework_waitForStop(framework); - celix_frameworkFactory_destroyFramework(framework); + } + celix_launcher_resetLauncher(); #ifndef CELIX_NO_CURLINIT - // Cleanup Curl - curl_global_cleanup(); + // Cleanup Curl + curl_global_cleanup(); #endif - celix_launcher_resetLauncher(); - } else { - celix_launcher_resetLauncher(); - } return status == CELIX_SUCCESS ? CELIX_LAUNCHER_OK_EXIT_CODE : CELIX_LAUNCHER_ERROR_EXIT_CODE; } @@ -364,40 +374,74 @@ static celix_status_t celix_launcher_loadRuntimeProperties(const char* configFil } static void celix_launcher_shutdownFrameworkSignalHandler(int signal) { - celix_launcher_stopInternal(&signal); + __atomic_store_n(&g_launcher.signal, signal, __ATOMIC_RELAXED); + __atomic_store_n(&g_launcher.shutdown, true, __ATOMIC_RELEASE); } void celix_launcher_triggerStop() { - celix_launcher_stopInternal(NULL); -} - -static void celix_launcher_setGlobalFramework(celix_framework_t* fw) { - __atomic_store_n(&g_framework, fw, __ATOMIC_SEQ_CST); + celix_auto(celix_mutex_lock_guard_t) lck = celixMutexLockGuard_init(&g_launcher.lock); + if (g_launcher.framework == NULL || g_launcher.shutdownEventId == -1) { + fprintf(stderr, "No global framework instance to stop\n"); + return; + } + __atomic_store_n(&g_launcher.shutdown, true, __ATOMIC_RELAXED); + celix_bundleContext_wakeupScheduledEvent(celix_framework_getFrameworkContext(g_launcher.framework), + g_launcher.shutdownEventId); } -void celix_launcher_stopInternal(const int* signal) { - celix_framework_t* fw = __atomic_exchange_n(&g_framework, NULL, __ATOMIC_SEQ_CST); - if (fw) { - if (signal) { - celix_bundle_context_t* ctx = celix_framework_getFrameworkContext(fw); +static void celix_launcher_shutdownCheck(void* data CELIX_UNUSED) { + // assert(data == &g_launcher) + celix_auto(celix_mutex_lock_guard_t) lck = celixMutexLockGuard_init(&g_launcher.lock); + if (__atomic_load_n(&g_launcher.shutdown, __ATOMIC_ACQUIRE)) { + int sig = __atomic_load_n(&g_launcher.signal, __ATOMIC_RELAXED); + if (sig != -1) { celix_bundleContext_log( - ctx, CELIX_LOG_LEVEL_INFO, "Stopping Celix framework due to signal %s", strsignal(*signal)); + celix_framework_getFrameworkContext(g_launcher.framework), CELIX_LOG_LEVEL_INFO, + "Stopping Celix framework due to signal %s", strsignal(sig)); } - celix_framework_stopBundle(fw, CELIX_FRAMEWORK_BUNDLE_ID); - } else { - fprintf(stderr, "No global framework instance to stop\n"); + celix_bundleContext_removeScheduledEventAsync(celix_framework_getFrameworkContext(g_launcher.framework), + g_launcher.shutdownEventId); + celix_framework_stopBundleAsync(g_launcher.framework, CELIX_FRAMEWORK_BUNDLE_ID); + g_launcher.shutdownEventId = -1; } } +static celix_status_t celix_launcher_setGlobalFramework(celix_framework_t* fw) { + celix_auto(celix_mutex_lock_guard_t) lck = celixMutexLockGuard_init(&g_launcher.lock); + celix_bundle_context_t *ctx = celix_framework_getFrameworkContext(fw); + g_launcher.framework = fw; + celix_scheduled_event_options_t opts = CELIX_EMPTY_SCHEDULED_EVENT_OPTIONS; + opts.name = "celix_shutdown_check"; + opts.callback = celix_launcher_shutdownCheck; + opts.callbackData = &g_launcher; + opts.initialDelayInSeconds = celix_bundleContext_getPropertyAsDouble(ctx, CELIX_LAUNCHER_SHUTDOWN_PERIOD_IN_SECONDS, + CELIX_LAUNCHER_SHUTDOWN_PERIOD_IN_SECONDS_DEFAULT); + opts.intervalInSeconds = celix_bundleContext_getPropertyAsDouble(ctx, CELIX_LAUNCHER_SHUTDOWN_PERIOD_IN_SECONDS, + CELIX_LAUNCHER_SHUTDOWN_PERIOD_IN_SECONDS_DEFAULT); + g_launcher.shutdownEventId = celix_bundleContext_scheduleEvent(ctx, &opts); + return g_launcher.shutdownEventId >= 0 ? CELIX_SUCCESS : CELIX_FRAMEWORK_EXCEPTION; +} + static bool celix_launcher_checkFrameworkLaunched() { - bool alreadyLaunched = __atomic_exchange_n(&g_framework_launched, true, __ATOMIC_SEQ_CST); - if (alreadyLaunched) { + celix_auto(celix_mutex_lock_guard_t) lck = celixMutexLockGuard_init(&g_launcher.lock); + if (g_launcher.launched) { fprintf(stderr, "Cannot launch framework, already launched\n"); + return false; } - return !alreadyLaunched; + g_launcher.launched = true; + return true; } static void celix_launcher_resetLauncher() { - __atomic_store_n(&g_framework_launched, false, __ATOMIC_SEQ_CST); - __atomic_store_n(&g_framework, NULL, __ATOMIC_SEQ_CST); + celix_auto(celix_mutex_lock_guard_t) lck = celixMutexLockGuard_init(&g_launcher.lock); + __atomic_store_n(&g_launcher.shutdown, false, __ATOMIC_RELAXED); + __atomic_store_n(&g_launcher.signal, -1, __ATOMIC_RELAXED); + if (g_launcher.framework) { + long schedId = g_launcher.shutdownEventId; + g_launcher.shutdownEventId = -1L; + celix_bundleContext_removeScheduledEvent(celix_framework_getFrameworkContext(g_launcher.framework), schedId); + celix_frameworkFactory_destroyFramework(g_launcher.framework); + g_launcher.framework = NULL; + } + g_launcher.launched = false; } diff --git a/libs/framework/src/celix_launcher_private.h b/libs/framework/src/celix_launcher_private.h index 8cb1d443f..d08fc7a95 100644 --- a/libs/framework/src/celix_launcher_private.h +++ b/libs/framework/src/celix_launcher_private.h @@ -24,15 +24,6 @@ extern "C" { #endif -/** - * @brief Stop the framework, by stopping the framework bundle. - * - * Also logs a message if the framework is stopped due to a signal. - * - * @param signal The optional signal that caused the framework to stop. - */ -void celix_launcher_stopInternal(const int* signal); - /** * @brief Create a combined configuration properties set by combining the embedded properties with the runtime * properties. diff --git a/libs/framework/src/framework.c b/libs/framework/src/framework.c index 797fce227..e3b0431df 100644 --- a/libs/framework/src/framework.c +++ b/libs/framework/src/framework.c @@ -2542,6 +2542,13 @@ long celix_framework_scheduleEvent(celix_framework_t* fw, bndId); return -1; } + if (initialDelayInSeconds < 0 || intervalInSeconds < 0) { + fw_log(fw->logger, + CELIX_LOG_LEVEL_ERROR, + "Cannot add scheduled event for bundle id %li. Invalid intervals: (%f,%f).", + bndId, initialDelayInSeconds, intervalInSeconds); + return -1; + } celix_bundle_entry_t* bndEntry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(fw, bndId); if (bndEntry == NULL) {
