From 66ab9bc651f04894a27f6fc083d74b101ddce950 Mon Sep 17 00:00:00 2001 From: xuzhenbao Date: Wed, 5 Jun 2024 11:14:28 +0800 Subject: [PATCH 01/21] Resolve issue 749 --- .../event_admin/gtest/src/CelixEventAdminTestSuite.cc | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/bundles/event_admin/event_admin/gtest/src/CelixEventAdminTestSuite.cc b/bundles/event_admin/event_admin/gtest/src/CelixEventAdminTestSuite.cc index eb4d45af8..5bc39a039 100644 --- a/bundles/event_admin/event_admin/gtest/src/CelixEventAdminTestSuite.cc +++ b/bundles/event_admin/event_admin/gtest/src/CelixEventAdminTestSuite.cc @@ -606,21 +606,26 @@ TEST_F(CelixEventAdminTestSuite, PostEventWithInvalidArgumentsTest) { } static bool g_blockingHandlerCalled = false; +static bool g_handlingEvent = false; TEST_F(CelixEventAdminTestSuite, AsyncEventQueueFullTest) { g_blockingHandlerCalled = true; + g_handlingEvent = false; TestPublishEvent("org/celix/test", nullptr, [](celix_event_admin_t *ea) { - for (int i = 0; i < 512 + 1/*handling event*/; ++i) { + for (int i = 0; i < 512; ++i) { auto status = celix_eventAdmin_postEvent(ea, "org/celix/test", nullptr); EXPECT_EQ(CELIX_SUCCESS, status); } - usleep(30000); + while (!g_handlingEvent) usleep(1000); auto status = celix_eventAdmin_postEvent(ea, "org/celix/test", nullptr); + EXPECT_EQ(CELIX_SUCCESS, status); + status = celix_eventAdmin_postEvent(ea, "org/celix/test", nullptr); EXPECT_EQ(CELIX_ILLEGAL_STATE, status); g_blockingHandlerCalled = false; }, [](void *handle, const char *topic, const celix_properties_t *props) { (void)handle; (void)props; (void)topic; + g_handlingEvent = true; while (g_blockingHandlerCalled) usleep(1000); return CELIX_SUCCESS; }); From a01495644a0e2423bf5ae2186e01d7850cb828b0 Mon Sep 17 00:00:00 2001 From: PengZheng Date: Fri, 7 Jun 2024 12:20:30 +0800 Subject: [PATCH 02/21] Make various lock guard deinit explicitly callable so that locks can be released manually. --- libs/utils/include/celix_threads.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/libs/utils/include/celix_threads.h b/libs/utils/include/celix_threads.h index 612c1869e..45c3111f1 100644 --- a/libs/utils/include/celix_threads.h +++ b/libs/utils/include/celix_threads.h @@ -22,6 +22,7 @@ #include #include +#include #include "celix_cleanup.h" #include "celix_errno.h" @@ -141,8 +142,8 @@ static CELIX_UNUSED inline celix_mutex_lock_guard_t celixMutexLockGuard_init(cel static CELIX_UNUSED inline void celixMutexLockGuard_deinit(celix_mutex_lock_guard_t* guard) { if (guard->mutex) { celixThreadMutex_unlock(guard->mutex); + guard->mutex = NULL; } - guard->mutex = NULL; } CELIX_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(celix_mutex_lock_guard_t, celixMutexLockGuard_deinit) @@ -216,6 +217,7 @@ static CELIX_UNUSED inline celix_rwlock_wlock_guard_t celixRwlockWlockGuard_init static CELIX_UNUSED inline void celixRwlockWlockGuard_deinit(celix_rwlock_wlock_guard_t* guard) { if (guard->lock) { celixThreadRwlock_unlock(guard->lock); + guard->lock = NULL; } } @@ -263,6 +265,7 @@ static CELIX_UNUSED inline celix_rwlock_rlock_guard_t celixRwlockRlockGuard_init static CELIX_UNUSED inline void celixRwlockRlockGuard_deinit(celix_rwlock_rlock_guard_t* guard) { if (guard->lock) { celixThreadRwlock_unlock(guard->lock); + guard->lock = NULL; } } From 21172764c6599cfb683713056ae28ee39dfc80be Mon Sep 17 00:00:00 2001 From: xuzhenbao Date: Fri, 7 Jun 2024 21:12:54 +0800 Subject: [PATCH 03/21] Use feature/promise to synchronize thread in unit test --- .../gtest/src/CelixEventAdminTestSuite.cc | 40 ++++++++++++------- .../src/CelixEventAdminTestSuiteBaseClass.h | 17 +++++--- 2 files changed, 37 insertions(+), 20 deletions(-) diff --git a/bundles/event_admin/event_admin/gtest/src/CelixEventAdminTestSuite.cc b/bundles/event_admin/event_admin/gtest/src/CelixEventAdminTestSuite.cc index 5bc39a039..e05cc19df 100644 --- a/bundles/event_admin/event_admin/gtest/src/CelixEventAdminTestSuite.cc +++ b/bundles/event_admin/event_admin/gtest/src/CelixEventAdminTestSuite.cc @@ -19,6 +19,7 @@ #include #include #include +#include #include #include "CelixEventAdminTestSuiteBaseClass.h" @@ -605,41 +606,50 @@ TEST_F(CelixEventAdminTestSuite, PostEventWithInvalidArgumentsTest) { }); } -static bool g_blockingHandlerCalled = false; -static bool g_handlingEvent = false; TEST_F(CelixEventAdminTestSuite, AsyncEventQueueFullTest) { - g_blockingHandlerCalled = true; - g_handlingEvent = false; - TestPublishEvent("org/celix/test", nullptr, [](celix_event_admin_t *ea) { + std::promise promise1; + auto future1 = promise1.get_future(); + std::promise promise2; + auto future2 = promise2.get_future(); + TestPublishEvent("org/celix/test", nullptr, [&promise2, &future1](celix_event_admin_t *ea) { for (int i = 0; i < 512; ++i) { auto status = celix_eventAdmin_postEvent(ea, "org/celix/test", nullptr); EXPECT_EQ(CELIX_SUCCESS, status); } - while (!g_handlingEvent) usleep(1000); + future1.get(); auto status = celix_eventAdmin_postEvent(ea, "org/celix/test", nullptr); EXPECT_EQ(CELIX_SUCCESS, status); status = celix_eventAdmin_postEvent(ea, "org/celix/test", nullptr); EXPECT_EQ(CELIX_ILLEGAL_STATE, status); - g_blockingHandlerCalled = false; - }, [](void *handle, const char *topic, const celix_properties_t *props) { + promise2.set_value(); + }, [&promise1, &future2](void *handle, const char *topic, const celix_properties_t *props) { (void)handle; (void)props; (void)topic; - g_handlingEvent = true; - while (g_blockingHandlerCalled) usleep(1000); + static bool firstCalled{true}; + if (firstCalled) { + promise1.set_value(); + future2.get(); + firstCalled = false; + } return CELIX_SUCCESS; }); } TEST_F(CelixEventAdminTestSuite, RemoveEventHandlerAfterEventAdminStopTest) { - g_blockingHandlerCalled = true; + std::promise promise; + auto future = promise.get_future(); celix_event_handler_service_t handler; - handler.handle = nullptr; + handler.handle = &future; handler.handleEvent = [](void *handle, const char *topic, const celix_properties_t *props) { - (void)handle; + auto feature = static_cast*>(handle); (void)topic; (void)props; - while (g_blockingHandlerCalled) usleep(1000); + static bool firstCalled{true}; + if (firstCalled) { + feature->get(); + firstCalled = false; + } return CELIX_SUCCESS; }; auto props = celix_properties_create(); @@ -674,7 +684,7 @@ TEST_F(CelixEventAdminTestSuite, RemoveEventHandlerAfterEventAdminStopTest) { EXPECT_EQ(CELIX_SUCCESS, status); } - g_blockingHandlerCalled = false; + promise.set_value(); status = celix_eventAdmin_stop(ea); EXPECT_EQ(CELIX_SUCCESS, status); diff --git a/bundles/event_admin/event_admin/gtest/src/CelixEventAdminTestSuiteBaseClass.h b/bundles/event_admin/event_admin/gtest/src/CelixEventAdminTestSuiteBaseClass.h index de0ad6092..7da83485e 100644 --- a/bundles/event_admin/event_admin/gtest/src/CelixEventAdminTestSuiteBaseClass.h +++ b/bundles/event_admin/event_admin/gtest/src/CelixEventAdminTestSuiteBaseClass.h @@ -20,6 +20,7 @@ #ifndef CELIX_CELIX_EVENT_ADMIN_TEST_SUITE_BASE_CLASS_H #define CELIX_CELIX_EVENT_ADMIN_TEST_SUITE_BASE_CLASS_H +#include #include "celix_event_admin.h" #include "celix_event_handler_service.h" @@ -137,16 +138,22 @@ class CelixEventAdminTestSuiteBaseClass : public ::testing::Test { celix_eventAdmin_destroy(ea); } - void TestPublishEvent(const char *handlerTopics, const char *eventFilter, void (*testBody)(celix_event_admin_t *ea), - celix_status_t (*onHandleEvent)(void *handle, const char *topic, const celix_properties_t *props), bool asyncUnordered = false) { + void TestPublishEvent(const char *handlerTopics, const char *eventFilter, std::function testBody, + std::function onHandleEvent, bool asyncUnordered = false) { auto ea = celix_eventAdmin_create(ctx.get()); EXPECT_TRUE(ea != nullptr); auto status = celix_eventAdmin_start(ea); EXPECT_EQ(CELIX_SUCCESS, status); - + struct celix_handle_event_callback_data { + std::function onHandleEvent; + void* handle; + } data{onHandleEvent, ea}; celix_event_handler_service_t handler; - handler.handle = ea; - handler.handleEvent = onHandleEvent; + handler.handle = &data; + handler.handleEvent = [](void *handle, const char *topic, const celix_properties_t *props) { + auto data = static_cast(handle); + return data->onHandleEvent(data->handle, topic, props); + }; auto props = celix_properties_create(); celix_properties_set(props, CELIX_FRAMEWORK_SERVICE_VERSION, CELIX_EVENT_HANDLER_SERVICE_VERSION); celix_properties_set(props, CELIX_EVENT_TOPIC, handlerTopics); From 8b0e0e0c4ec72ac7b4a8ff0ff200227b7144f32b Mon Sep 17 00:00:00 2001 From: Pepijn Noltes Date: Sun, 9 Jun 2024 14:48:19 +0200 Subject: [PATCH 04/21] gh-685: Add bundle entry guard Also improves several small issues based on review comments. --- ...undleArchiveWithErrorInjectionTestSuite.cc | 4 +- libs/framework/src/bundle_archive.c | 4 +- libs/framework/src/bundle_archive_private.h | 2 +- libs/framework/src/celix_bundle_cache.c | 6 +- libs/framework/src/celix_launcher.c | 4 +- libs/framework/src/celix_launcher_private.h | 2 +- libs/framework/src/framework.c | 182 +++++++++--------- .../src/framework_bundle_lifecycle_handler.c | 27 +-- libs/framework/src/framework_private.h | 62 ++++-- 9 files changed, 162 insertions(+), 131 deletions(-) diff --git a/libs/framework/gtest/src/BundleArchiveWithErrorInjectionTestSuite.cc b/libs/framework/gtest/src/BundleArchiveWithErrorInjectionTestSuite.cc index 2aeb425c3..47819d81e 100644 --- a/libs/framework/gtest/src/BundleArchiveWithErrorInjectionTestSuite.cc +++ b/libs/framework/gtest/src/BundleArchiveWithErrorInjectionTestSuite.cc @@ -210,7 +210,7 @@ TEST_F(CelixBundleArchiveErrorInjectionTestSuite, ArchiveCreateErrorTest) { EXPECT_EQ(CELIX_SUCCESS, celix_utils_extractZipFile(SIMPLE_TEST_BUNDLE1_LOCATION, testExtractDir, nullptr)); EXPECT_EQ(CELIX_SUCCESS, celix_bundleArchive_create(&fw, TEST_ARCHIVE_ROOT, 2, testExtractDir, &archive)); - bundleArchive_destroy(archive); + celix_bundleArchive_destroy(archive); archive = nullptr; std::this_thread::sleep_for(std::chrono::milliseconds(10)); celix_utils_touch(SIMPLE_TEST_BUNDLE1_LOCATION); @@ -223,7 +223,7 @@ TEST_F(CelixBundleArchiveErrorInjectionTestSuite, ArchiveCreateErrorTest) { EXPECT_EQ(CELIX_SUCCESS, celix_bundleArchive_create(&fw, TEST_ARCHIVE_ROOT, 1, SIMPLE_TEST_BUNDLE1_LOCATION, &archive)); - bundleArchive_destroy(archive); + celix_bundleArchive_destroy(archive); archive = nullptr; std::this_thread::sleep_for(std::chrono::milliseconds(10)); celix_utils_touch(SIMPLE_TEST_BUNDLE1_LOCATION); diff --git a/libs/framework/src/bundle_archive.c b/libs/framework/src/bundle_archive.c index 239ef9412..8867e8038 100644 --- a/libs/framework/src/bundle_archive.c +++ b/libs/framework/src/bundle_archive.c @@ -329,14 +329,14 @@ celix_status_t celix_bundleArchive_create(celix_framework_t* fw, const char *arc celix_utils_deleteDirectory(archive->archiveRoot, NULL); } init_failed: - bundleArchive_destroy(archive); + celix_bundleArchive_destroy(archive); calloc_failed: celix_framework_logTssErrors(fw->logger, CELIX_LOG_LEVEL_ERROR); framework_logIfError(fw->logger, status, error, "Could not create archive."); return status; } -void bundleArchive_destroy(bundle_archive_pt archive) { +void celix_bundleArchive_destroy(bundle_archive_pt archive) { if (archive != NULL) { free(archive->location); free(archive->savedBundleStatePropertiesPath); diff --git a/libs/framework/src/bundle_archive_private.h b/libs/framework/src/bundle_archive_private.h index 7a2c774a8..e0d86acb7 100644 --- a/libs/framework/src/bundle_archive_private.h +++ b/libs/framework/src/bundle_archive_private.h @@ -49,7 +49,7 @@ extern "C" { */ celix_status_t celix_bundleArchive_create(celix_framework_t* fw, const char *archiveRoot, long id, const char *location, bundle_archive_pt *bundle_archive); -void bundleArchive_destroy(bundle_archive_pt archive); +void celix_bundleArchive_destroy(bundle_archive_pt archive); /** * @brief Returns the bundle id of the bundle archive. diff --git a/libs/framework/src/celix_bundle_cache.c b/libs/framework/src/celix_bundle_cache.c index d99f64fc5..cb0260c64 100644 --- a/libs/framework/src/celix_bundle_cache.c +++ b/libs/framework/src/celix_bundle_cache.c @@ -212,7 +212,7 @@ void celix_bundleCache_destroyArchive(celix_bundle_cache_t* cache, bundle_archiv } (void)celix_bundleArchive_removeInvalidDirs(archive); celixThreadMutex_unlock(&cache->mutex); - bundleArchive_destroy(archive); + celix_bundleArchive_destroy(archive); } /** @@ -242,7 +242,7 @@ static celix_status_t celix_bundleCache_updateIdForLocationLookupMap(celix_bundl fw_logCode(cache->fw->logger, CELIX_LOG_LEVEL_ERROR, status, "Cannot load bundle state properties from %s", bundleStateProperties); celix_framework_logTssErrors(cache->fw->logger, CELIX_LOG_LEVEL_ERROR); - return CELIX_FILE_IO_EXCEPTION; + return status; } const char* visitLoc = celix_properties_get(props, CELIX_BUNDLE_ARCHIVE_LOCATION_PROPERTY_NAME, NULL); long bndId = celix_properties_getAsLong(props, CELIX_BUNDLE_ARCHIVE_BUNDLE_ID_PROPERTY_NAME, -1); @@ -309,7 +309,7 @@ celix_bundleCache_createBundleArchivesForSpaceSeparatedList(celix_framework_t* f fw_log(fw->logger, lvl, "Created bundle cache '%s' for bundle archive %s (bndId=%li).", celix_bundleArchive_getCurrentRevisionRoot(archive), celix_bundleArchive_getSymbolicName(archive), celix_bundleArchive_getId(archive)); - bundleArchive_destroy(archive); + celix_bundleArchive_destroy(archive); } location = strtok_r(NULL, delims, &savePtr); } diff --git a/libs/framework/src/celix_launcher.c b/libs/framework/src/celix_launcher.c index c4bd7188b..5e278ac24 100644 --- a/libs/framework/src/celix_launcher.c +++ b/libs/framework/src/celix_launcher.c @@ -151,7 +151,7 @@ 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 && framework) { + if (status == CELIX_SUCCESS) { status = celix_launcher_setGlobalFramework(framework); if (status != CELIX_SUCCESS) { return CELIX_LAUNCHER_ERROR_EXIT_CODE; @@ -235,7 +235,7 @@ static celix_status_t celix_launcher_createFramework(celix_properties_t* embedde sigaction(SIGUSR2, &sigact, NULL); #ifndef CELIX_NO_CURLINIT - // Before doing anything else, let's setup Curl + // Before doing anything else, lets setup Curl curl_global_init(CURL_GLOBAL_ALL); #endif diff --git a/libs/framework/src/celix_launcher_private.h b/libs/framework/src/celix_launcher_private.h index d35bd197d..8cb1d443f 100644 --- a/libs/framework/src/celix_launcher_private.h +++ b/libs/framework/src/celix_launcher_private.h @@ -37,7 +37,7 @@ void celix_launcher_stopInternal(const int* signal); * @brief Create a combined configuration properties set by combining the embedded properties with the runtime * properties. * @param[in,out] embeddedProps The embedded properties to use and extend with the runtime properties. - * @param[in] configFile The optional runtime properties file to load and use. + * @param[in] runtimeProps The optional runtime properties file to load and use. */ celix_status_t celix_launcher_combineProperties(celix_properties_t* embeddedProps, const celix_properties_t* runtimeProps); diff --git a/libs/framework/src/framework.c b/libs/framework/src/framework.c index 5d2044a8b..bd60e1837 100644 --- a/libs/framework/src/framework.c +++ b/libs/framework/src/framework.c @@ -60,10 +60,11 @@ struct celix_bundle_activator { }; static int celix_framework_eventQueueSize(celix_framework_t* fw); -static celix_status_t celix_framework_stopBundleEntryInternal(celix_framework_t* framework, celix_framework_bundle_entry_t* bndEntry); +static celix_status_t celix_framework_stopBundleEntryInternal(celix_framework_t* framework, + celix_bundle_entry_t* bndEntry); -static inline celix_framework_bundle_entry_t* fw_bundleEntry_create(celix_bundle_t *bnd) { - celix_framework_bundle_entry_t *entry = calloc(1, sizeof(*entry)); +static inline celix_bundle_entry_t* fw_bundleEntry_create(celix_bundle_t *bnd) { + celix_bundle_entry_t*entry = calloc(1, sizeof(*entry)); entry->bnd = bnd; celixThreadRwlock_create(&entry->fsmMutex, NULL); entry->bndId = celix_bundle_getId(bnd); @@ -73,7 +74,7 @@ static inline celix_framework_bundle_entry_t* fw_bundleEntry_create(celix_bundle return entry; } -static inline void fw_bundleEntry_destroy(celix_framework_bundle_entry_t *entry, bool wait) { +static inline void fw_bundleEntry_destroy(celix_bundle_entry_t*entry, bool wait) { celixThreadMutex_lock(&entry->useMutex); while (wait && entry->useCount != 0) { celixThreadCondition_wait(&entry->useCond, &entry->useMutex); @@ -87,14 +88,14 @@ static inline void fw_bundleEntry_destroy(celix_framework_bundle_entry_t *entry, free(entry); } -void celix_framework_bundleEntry_increaseUseCount(celix_framework_bundle_entry_t *entry) { +void celix_bundleEntry_increaseUseCount(celix_bundle_entry_t*entry) { assert(entry != NULL); celixThreadMutex_lock(&entry->useMutex); ++entry->useCount; celixThreadMutex_unlock(&entry->useMutex); } -void celix_framework_bundleEntry_decreaseUseCount(celix_framework_bundle_entry_t *entry) { +void celix_bundleEntry_decreaseUseCount(celix_bundle_entry_t*entry) { celixThreadMutex_lock(&entry->useMutex); assert(entry->useCount > 0); --entry->useCount; @@ -102,13 +103,13 @@ void celix_framework_bundleEntry_decreaseUseCount(celix_framework_bundle_entry_t celixThreadMutex_unlock(&entry->useMutex); } -celix_framework_bundle_entry_t* celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(celix_framework_t *fw, long bndId) { - celix_framework_bundle_entry_t* found = NULL; +celix_bundle_entry_t* celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(celix_framework_t *fw, long bndId) { + celix_bundle_entry_t* found = NULL; celixThreadMutex_lock(&fw->installedBundles.mutex); for (int i = 0; i < celix_arrayList_size(fw->installedBundles.entries); ++i) { - celix_framework_bundle_entry_t *entry = celix_arrayList_get(fw->installedBundles.entries, i); + celix_bundle_entry_t*entry = celix_arrayList_get(fw->installedBundles.entries, i); if (entry->bndId == bndId) { - celix_framework_bundleEntry_increaseUseCount(entry); + celix_bundleEntry_increaseUseCount(entry); found = entry; break; } @@ -121,7 +122,7 @@ bool celix_framework_isBundleIdAlreadyUsed(celix_framework_t *fw, long bndId) { bool found = false; celixThreadMutex_lock(&fw->installedBundles.mutex); for (int i = 0; i < celix_arrayList_size(fw->installedBundles.entries); ++i) { - celix_framework_bundle_entry_t *entry = celix_arrayList_get(fw->installedBundles.entries, i); + celix_bundle_entry_t*entry = celix_arrayList_get(fw->installedBundles.entries, i); if (entry->bndId == bndId) { found = true; break; @@ -131,11 +132,11 @@ bool celix_framework_isBundleIdAlreadyUsed(celix_framework_t *fw, long bndId) { return found; } -static inline bool fw_bundleEntry_removeBundleEntry(celix_framework_t *fw, celix_framework_bundle_entry_t* bndEntry) { +static inline bool fw_bundleEntry_removeBundleEntry(celix_framework_t *fw, celix_bundle_entry_t* bndEntry) { bool found = false; celixThreadMutex_lock(&fw->installedBundles.mutex); for (int i = 0; i < celix_arrayList_size(fw->installedBundles.entries); ++i) { - celix_framework_bundle_entry_t *entry = celix_arrayList_get(fw->installedBundles.entries, i); + celix_bundle_entry_t*entry = celix_arrayList_get(fw->installedBundles.entries, i); if (entry == bndEntry) { found = true; celix_arrayList_removeAt(fw->installedBundles.entries, i); @@ -152,7 +153,7 @@ long framework_getNextBundleId(framework_pt framework); celix_status_t fw_populateDependentGraph(framework_pt framework, bundle_pt exporter, hash_map_pt *map); -void fw_fireBundleEvent(framework_pt framework, bundle_event_type_e, celix_framework_bundle_entry_t* entry); +void fw_fireBundleEvent(framework_pt framework, bundle_event_type_e, celix_bundle_entry_t* entry); void fw_fireFrameworkEvent(framework_pt framework, framework_event_type_e eventType, celix_status_t errorCode); static void *fw_eventDispatcher(void *fw); @@ -265,7 +266,7 @@ celix_status_t framework_create(framework_pt *out, celix_properties_t* config) { status = CELIX_DO_IF(status, bundle_setContext(framework->bundle, context)); //create framework bundle entry - celix_framework_bundle_entry_t *entry = fw_bundleEntry_create(framework->bundle); + celix_bundle_entry_t*entry = fw_bundleEntry_create(framework->bundle); celixThreadMutex_lock(&framework->installedBundles.mutex); celix_arrayList_add(framework->installedBundles.entries, entry); celixThreadMutex_unlock(&framework->installedBundles.mutex); @@ -299,7 +300,7 @@ celix_status_t framework_destroy(framework_pt framework) { celix_serviceRegistry_destroy(framework->registry); for (int i = 0; i < celix_arrayList_size(framework->installedBundles.entries); ++i) { - celix_framework_bundle_entry_t *entry = celix_arrayList_get(framework->installedBundles.entries, i); + celix_bundle_entry_t*entry = celix_arrayList_get(framework->installedBundles.entries, i); celixThreadMutex_lock(&entry->useMutex); size_t count = entry->useCount; celixThreadMutex_unlock(&entry->useMutex); @@ -331,7 +332,7 @@ celix_status_t framework_destroy(framework_pt framework) { } if (archive) { - bundleArchive_destroy(archive); + celix_bundleArchive_destroy(archive); } bundle_destroy(bnd); @@ -453,10 +454,10 @@ celix_status_t framework_start(celix_framework_t* framework) { return status; } - celix_framework_bundle_entry_t* entry = + celix_bundle_entry_t* entry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(framework, framework->bundleId); fw_fireBundleEvent(framework, OSGI_FRAMEWORK_BUNDLE_EVENT_STARTED, entry); - celix_framework_bundleEntry_decreaseUseCount(entry); + celix_bundleEntry_decreaseUseCount(entry); bool allStarted = false; status = framework_autoStartConfiguredBundles(framework, &allStarted); @@ -654,8 +655,9 @@ celix_framework_installBundleInternalImpl(celix_framework_t* framework, const ch } // increase use count of framework bundle to prevent a stop. - celix_framework_bundle_entry_t* fwBundleEntry = + celix_bundle_entry_t* fwBundleEntry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(framework, framework->bundleId); + celix_auto(celix_bundle_entry_use_guard_t) fwEntryGuard = celix_bundleEntryUseGuard_init(fwBundleEntry); celix_bundle_state_e bndState = celix_bundle_getState(framework->bundle); if (bndState == CELIX_BUNDLE_STATE_STOPPING || bndState == CELIX_BUNDLE_STATE_UNINSTALLED) { @@ -667,7 +669,6 @@ celix_framework_installBundleInternalImpl(celix_framework_t* framework, const ch if (*bndId == -1L) { id = framework_getBundle(framework, bndLoc); if (id != -1L) { - celix_framework_bundleEntry_decreaseUseCount(fwBundleEntry); *bndId = id; return CELIX_SUCCESS; } @@ -675,7 +676,6 @@ celix_framework_installBundleInternalImpl(celix_framework_t* framework, const ch celix_status_t status = celix_bundleCache_findBundleIdForLocation(framework->cache, bndLoc, &alreadyExistingBndId); if (status != CELIX_SUCCESS) { - celix_framework_bundleEntry_decreaseUseCount(fwBundleEntry); fw_logCode(framework->logger, CELIX_LOG_LEVEL_ERROR, status, "Could not install bundle"); return status; } @@ -689,19 +689,18 @@ celix_framework_installBundleInternalImpl(celix_framework_t* framework, const ch celix_status_t status = celix_bundleCache_createArchive(framework->cache, id, bndLoc, &archive); status = CELIX_DO_IF(status, celix_bundle_createFromArchive(framework, archive, &bundle)); if (status != CELIX_SUCCESS) { - celix_framework_bundleEntry_decreaseUseCount(fwBundleEntry); - fw_logCode(framework->logger, CELIX_LOG_LEVEL_ERROR, status, "Could not install bundle"); + celix_bundleArchive_destroy(archive); + fw_logCode(framework->logger, CELIX_LOG_LEVEL_ERROR, status, "Cannot install bundle without archive"); return status; } - celix_framework_bundle_entry_t *bEntry = fw_bundleEntry_create(bundle); - celix_framework_bundleEntry_increaseUseCount(bEntry); + celix_bundle_entry_t*bEntry = fw_bundleEntry_create(bundle); + celix_bundleEntry_increaseUseCount(bEntry); celixThreadMutex_lock(&framework->installedBundles.mutex); celix_arrayList_add(framework->installedBundles.entries, bEntry); celixThreadMutex_unlock(&framework->installedBundles.mutex); fw_fireBundleEvent(framework, OSGI_FRAMEWORK_BUNDLE_EVENT_INSTALLED, bEntry); - celix_framework_bundleEntry_decreaseUseCount(bEntry); - celix_framework_bundleEntry_decreaseUseCount(fwBundleEntry); + celix_bundleEntry_decreaseUseCount(bEntry); *bndId = id; return CELIX_SUCCESS; } @@ -719,7 +718,7 @@ bool celix_framework_isBundleAlreadyInstalled(celix_framework_t* fw, const char* bool alreadyExists = false; celixThreadMutex_lock(&fw->installedBundles.mutex); for (int i = 0; i < celix_arrayList_size(fw->installedBundles.entries); ++i) { - celix_framework_bundle_entry_t *entry = celix_arrayList_get(fw->installedBundles.entries, i); + celix_bundle_entry_t*entry = celix_arrayList_get(fw->installedBundles.entries, i); if (celix_utils_stringEquals(entry->bnd->symbolicName, bundleSymbolicName)) { alreadyExists = true; break; @@ -793,10 +792,10 @@ celix_status_t fw_registerService(framework_pt framework, service_registration_p error = "ServiceName and SvcObj cannot be null"; } - celix_framework_bundle_entry_t *entry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(framework, + celix_bundle_entry_t*entry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(framework, bndId); status = CELIX_DO_IF(status, serviceRegistry_registerService(framework->registry, entry->bnd, serviceName, svcObj, properties, registration)); - celix_framework_bundleEntry_decreaseUseCount(entry); + celix_bundleEntry_decreaseUseCount(entry); framework_logIfError(framework->logger, status, error, "Cannot register service: %s", serviceName); return status; } @@ -809,12 +808,12 @@ celix_status_t fw_registerServiceFactory(framework_pt framework, service_registr error = "Service name and factory cannot be null"; } - celix_framework_bundle_entry_t *entry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(framework, + celix_bundle_entry_t*entry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(framework, bndId); status = CELIX_DO_IF(status, serviceRegistry_registerServiceFactory(framework->registry, entry->bnd, serviceName, factory, properties, registration)); - celix_framework_bundleEntry_decreaseUseCount(entry); + celix_bundleEntry_decreaseUseCount(entry); framework_logIfError(framework->logger, status, error, "Cannot register service factory: %s", serviceName); @@ -879,7 +878,7 @@ void fw_removeServiceListener(framework_pt framework, bundle_pt bundle CELIX_UNU celix_status_t fw_addBundleListener(framework_pt framework, bundle_pt bundle, bundle_listener_pt listener) { typedef struct { - celix_framework_bundle_entry_t* entry; + celix_bundle_entry_t* entry; celix_bundle_state_e currentState; } installed_bundle_entry_t; @@ -896,8 +895,8 @@ celix_status_t fw_addBundleListener(framework_pt framework, bundle_pt bundle, bu celixThreadMutex_lock(&framework->installedBundles.mutex); int size = celix_arrayList_size(framework->installedBundles.entries); for (int i = 0; i < size; ++i) { - celix_framework_bundle_entry_t *entry = celix_arrayList_get(framework->installedBundles.entries, i); - celix_framework_bundleEntry_increaseUseCount(entry); + celix_bundle_entry_t*entry = celix_arrayList_get(framework->installedBundles.entries, i); + celix_bundleEntry_increaseUseCount(entry); installed_bundle_entry_t* installedEntry = calloc(1, sizeof(*installedEntry)); installedEntry->entry = entry; installedEntry->currentState = celix_bundle_getState(entry->bnd); @@ -930,7 +929,7 @@ celix_status_t fw_addBundleListener(framework_pt framework, bundle_pt bundle, bu event.type = OSGI_FRAMEWORK_BUNDLE_EVENT_STARTED; listener->bundleChanged(listener, &event); } - celix_framework_bundleEntry_decreaseUseCount(installedEntry->entry); + celix_bundleEntry_decreaseUseCount(installedEntry->entry); free(installedEntry); } fw_bundleListener_decreaseUseCount(bundleListener); @@ -1019,7 +1018,7 @@ static celix_status_t framework_markBundleResolved(framework_pt framework, modul if (bundle != NULL) { long bndId = celix_bundle_getId(bundle); - celix_framework_bundle_entry_t *entry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(framework,bndId); + celix_bundle_entry_t*entry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(framework,bndId); bundle_getState(bundle, &state); if (state != CELIX_BUNDLE_STATE_INSTALLED) { @@ -1045,8 +1044,7 @@ static celix_status_t framework_markBundleResolved(framework_pt framework, modul fw_logCode(framework->logger, CELIX_LOG_LEVEL_ERROR, status, "Could not resolve bundle: %s [%ld]", symbolicName, id); } - - celix_framework_bundleEntry_decreaseUseCount(entry); + celix_bundleEntry_decreaseUseCount(entry); } return status; @@ -1060,7 +1058,7 @@ celix_array_list_t* framework_getBundles(framework_pt framework) { celixThreadMutex_lock(&framework->installedBundles.mutex); int size = celix_arrayList_size(framework->installedBundles.entries); for (int i = 0; i < size; ++i) { - celix_framework_bundle_entry_t *entry = celix_arrayList_get(framework->installedBundles.entries, i); + celix_bundle_entry_t*entry = celix_arrayList_get(framework->installedBundles.entries, i); celix_arrayList_add(bundles, entry->bnd); } celixThreadMutex_unlock(&framework->installedBundles.mutex); @@ -1074,7 +1072,7 @@ long framework_getBundle(framework_pt framework, const char* location) { celixThreadMutex_lock(&framework->installedBundles.mutex); int size = celix_arrayList_size(framework->installedBundles.entries); for (int i = 0; i < size; ++i) { - celix_framework_bundle_entry_t *entry = celix_arrayList_get(framework->installedBundles.entries, i); + celix_bundle_entry_t*entry = celix_arrayList_get(framework->installedBundles.entries, i); const char *loc = NULL; bundle_getBundleLocation(entry->bnd, &loc); if (loc != NULL && location != NULL && strncmp(loc, location, strlen(loc)) == 0) { @@ -1113,12 +1111,12 @@ static void* framework_shutdown(void *framework) { celix_framework_waitForBundleLifecycleHandlers(fw); celix_array_list_t *stopEntries = celix_arrayList_create(); - celix_framework_bundle_entry_t *fwEntry = NULL; + celix_bundle_entry_t*fwEntry = NULL; celixThreadMutex_lock(&fw->installedBundles.mutex); int size = celix_arrayList_size(fw->installedBundles.entries); for (int i = 0; i < size; ++i) { - celix_framework_bundle_entry_t *entry = celix_arrayList_get(fw->installedBundles.entries, i); - celix_framework_bundleEntry_increaseUseCount(entry); + celix_bundle_entry_t*entry = celix_arrayList_get(fw->installedBundles.entries, i); + celix_bundleEntry_increaseUseCount(entry); if (entry->bndId != 0) { //i.e. not framework bundle celix_arrayList_add(stopEntries, entry); } else { @@ -1129,7 +1127,7 @@ static void* framework_shutdown(void *framework) { size = celix_arrayList_size(stopEntries); for (int i = size-1; i >= 0; --i) { //note loop in reverse order -> uninstall later installed bundle first - celix_framework_bundle_entry_t *entry = celix_arrayList_get(stopEntries, i); + celix_bundle_entry_t*entry = celix_arrayList_get(stopEntries, i); celix_framework_uninstallBundleEntry(fw, entry, false); } celix_arrayList_destroy(stopEntries); @@ -1140,7 +1138,7 @@ static void* framework_shutdown(void *framework) { // Lock the mutex to make sure that `celix_framework_stopBundleEntryInternal` on the framework has finished. celixThreadRwlock_readLock(&fwEntry->fsmMutex); celixThreadRwlock_unlock(&fwEntry->fsmMutex); - celix_framework_bundleEntry_decreaseUseCount(fwEntry); + celix_bundleEntry_decreaseUseCount(fwEntry); } //Now that all bundled has been stopped, no more events will be sent, we can safely stop the event dispatcher. celix_framework_stopAndJoinEventQueue(fw); @@ -1189,7 +1187,7 @@ bundle_context_t* framework_getContext(const_framework_pt framework) { return result; } -void fw_fireBundleEvent(framework_pt framework, bundle_event_type_e eventType, celix_framework_bundle_entry_t* entry) { +void fw_fireBundleEvent(framework_pt framework, bundle_event_type_e eventType, celix_bundle_entry_t* entry) { if (eventType == OSGI_FRAMEWORK_BUNDLE_EVENT_STOPPING || eventType == OSGI_FRAMEWORK_BUNDLE_EVENT_UNINSTALLED || eventType == OSGI_FRAMEWORK_BUNDLE_EVENT_STOPPED) { if (entry->bndId == framework->bundleId) { //NOTE for framework bundle not triggering events while framework is stopped (and as result in use) @@ -1197,7 +1195,7 @@ void fw_fireBundleEvent(framework_pt framework, bundle_event_type_e eventType, c } } - celix_framework_bundleEntry_increaseUseCount(entry); + celix_bundleEntry_increaseUseCount(entry); celix_framework_event_t event; memset(&event, 0, sizeof(event)); @@ -1358,7 +1356,7 @@ static inline void fw_handleEvents(celix_framework_t* framework) { bool dynamicallyAllocatedEvent = fw_removeTopEventFromQueue(framework); if (topEvent->bndEntry != NULL) { - celix_framework_bundleEntry_decreaseUseCount(topEvent->bndEntry); + celix_bundleEntry_decreaseUseCount(topEvent->bndEntry); } free(topEvent->serviceName); if (dynamicallyAllocatedEvent) { @@ -1582,7 +1580,7 @@ static size_t celix_framework_useBundlesInternal(framework_t *fw, bool includeFr celixThreadMutex_lock(&fw->installedBundles.mutex); int size = celix_arrayList_size(fw->installedBundles.entries); for (int i = 0; i < size; ++i) { - celix_framework_bundle_entry_t *entry = celix_arrayList_get(fw->installedBundles.entries, i); + celix_bundle_entry_t*entry = celix_arrayList_get(fw->installedBundles.entries, i); if (entry->bndId > 0 || includeFrameworkBundle) { celix_arrayList_addLong(bundleIds, entry->bndId); } @@ -1617,7 +1615,7 @@ size_t celix_framework_useActiveBundles(framework_t *fw, bool includeFrameworkBu bool celix_framework_useBundle(framework_t *fw, bool onlyActive, long bundleId, void *callbackHandle, void(*use)(void *handle, const bundle_t *bnd)) { bool called = false; if (bundleId >= 0) { - celix_framework_bundle_entry_t *entry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(fw, + celix_bundle_entry_t*entry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(fw, bundleId); if (entry != NULL) { if (onlyActive) { @@ -1634,7 +1632,7 @@ bool celix_framework_useBundle(framework_t *fw, bool onlyActive, long bundleId, if (onlyActive) { celixThreadRwlock_unlock(&entry->fsmMutex); } - celix_framework_bundleEntry_decreaseUseCount(entry); + celix_bundleEntry_decreaseUseCount(entry); } else { framework_logIfError(fw->logger, CELIX_FRAMEWORK_EXCEPTION, NULL, "Bundle with id %li is not installed", bundleId); } @@ -1653,7 +1651,7 @@ long celix_framework_registerService(framework_t *fw, celix_bundle_t *bnd, const } long bndId = celix_bundle_getId(bnd); - celix_framework_bundle_entry_t *entry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(fw, bndId); + celix_bundle_entry_t*entry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(fw, bndId); if (factory != NULL) { @@ -1662,7 +1660,7 @@ long celix_framework_registerService(framework_t *fw, celix_bundle_t *bnd, const status = celix_serviceRegistry_registerService(fw->registry, bnd, serviceName, svc, properties, 0, ®); } - celix_framework_bundleEntry_decreaseUseCount(entry); + celix_bundleEntry_decreaseUseCount(entry); framework_logIfError(fw->logger, status, error, "Cannot register %s '%s'", factory == NULL ? "service" : "service factory", serviceName); @@ -1681,7 +1679,7 @@ long celix_framework_registerServiceAsync( void* eventDoneData, void (*eventDoneCallback)(void* eventDoneData)) { long bndId = celix_bundle_getId(bnd); - celix_framework_bundle_entry_t *entry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(fw, bndId); + celix_bundle_entry_t*entry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(fw, bndId); long svcId = celix_serviceRegistry_nextSvcId(fw->registry); @@ -1706,7 +1704,7 @@ long celix_framework_registerServiceAsync( void celix_framework_unregisterAsync(celix_framework_t* fw, celix_bundle_t* bnd, long serviceId, void *doneData, void (*doneCallback)(void*)) { long bndId = celix_bundle_getId(bnd); - celix_framework_bundle_entry_t *entry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(fw, bndId); + celix_bundle_entry_t*entry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(fw, bndId); celix_framework_event_t event; memset(&event, 0, sizeof(event)); @@ -1879,30 +1877,30 @@ celix_bundle_t* celix_framework_getFrameworkBundle(const celix_framework_t *fw) bundle_pt framework_getBundleById(framework_pt framework, long id) { celix_bundle_t *bnd = NULL; - celix_framework_bundle_entry_t *entry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(framework, id); + celix_bundle_entry_t*entry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(framework, id); if (entry != NULL) { bnd = entry->bnd; - celix_framework_bundleEntry_decreaseUseCount(entry); //NOTE returning bundle without increased use count -> FIXME make all getBundle api private (use bundle id instead) + celix_bundleEntry_decreaseUseCount(entry); //NOTE returning bundle without increased use count -> FIXME make all getBundle api private (use bundle id instead) } return bnd; } bool celix_framework_isBundleInstalled(celix_framework_t *fw, long bndId) { bool isInstalled = false; - celix_framework_bundle_entry_t *entry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(fw, bndId); + celix_bundle_entry_t*entry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(fw, bndId); if (entry != NULL) { isInstalled = true; - celix_framework_bundleEntry_decreaseUseCount(entry); + celix_bundleEntry_decreaseUseCount(entry); } return isInstalled; } bool celix_framework_isBundleActive(celix_framework_t *fw, long bndId) { bool isActive = false; - celix_framework_bundle_entry_t *entry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(fw, bndId); + celix_bundle_entry_t*entry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(fw, bndId); if (entry != NULL) { isActive = celix_bundle_getState(entry->bnd) == CELIX_BUNDLE_STATE_ACTIVE; - celix_framework_bundleEntry_decreaseUseCount(entry); + celix_bundleEntry_decreaseUseCount(entry); } return isActive; } @@ -1919,11 +1917,11 @@ static long celix_framework_installAndStartBundleInternal(celix_framework_t *fw, if (celix_framework_installBundleInternal(fw, bundleLoc, &bundleId) == CELIX_SUCCESS) { if (autoStart) { - celix_framework_bundle_entry_t* bndEntry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(fw, + celix_bundle_entry_t* bndEntry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(fw, bundleId); if (bndEntry != NULL) { status = celix_framework_startBundleOnANonCelixEventThread(fw, bndEntry, forcedAsync); - celix_framework_bundleEntry_decreaseUseCount(bndEntry); + celix_bundleEntry_decreaseUseCount(bndEntry); } else { status = CELIX_ILLEGAL_STATE; } @@ -1947,7 +1945,7 @@ long celix_framework_installBundleAsync(celix_framework_t *fw, const char *bundl static bool celix_framework_uninstallBundleInternal(celix_framework_t *fw, long bndId, bool forcedAsync, bool permanent) { bool uninstalled = false; - celix_framework_bundle_entry_t *bndEntry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(fw, bndId); + celix_bundle_entry_t*bndEntry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(fw, bndId); if (bndEntry != NULL) { celix_status_t status = celix_framework_uninstallBundleOnANonCelixEventThread(fw, bndEntry, forcedAsync, permanent); if (!forcedAsync) { @@ -1975,8 +1973,7 @@ void celix_framework_unloadBundleAsync(celix_framework_t *fw, long bndId) { celix_framework_uninstallBundleInternal(fw, bndId, true, false); } -static celix_status_t celix_framework_uninstallBundleEntryImpl(celix_framework_t* framework, - celix_framework_bundle_entry_t* bndEntry, +static celix_status_t celix_framework_uninstallBundleEntryImpl(celix_framework_t* framework, celix_bundle_entry_t* bndEntry, bool permanent) { assert(!celix_framework_isCurrentThreadTheEventLoop(framework)); @@ -1988,7 +1985,7 @@ static celix_status_t celix_framework_uninstallBundleEntryImpl(celix_framework_t if (!fw_bundleEntry_removeBundleEntry(framework, bndEntry)) { celixThreadRwlock_unlock(&bndEntry->fsmMutex); - celix_framework_bundleEntry_decreaseUseCount(bndEntry); + celix_bundleEntry_decreaseUseCount(bndEntry); return CELIX_ILLEGAL_STATE; } @@ -2014,7 +2011,7 @@ static celix_status_t celix_framework_uninstallBundleEntryImpl(celix_framework_t celixThreadRwlock_unlock(&bndEntry->fsmMutex); //NOTE wait outside installedBundles.mutex - celix_framework_bundleEntry_decreaseUseCount(bndEntry); + celix_bundleEntry_decreaseUseCount(bndEntry); fw_bundleEntry_destroy(bndEntry, true); //wait till use count is 0 -> e.g. not used if (status == CELIX_SUCCESS) { @@ -2030,7 +2027,7 @@ static celix_status_t celix_framework_uninstallBundleEntryImpl(celix_framework_t } -celix_status_t celix_framework_uninstallBundleEntry(celix_framework_t* framework, celix_framework_bundle_entry_t* bndEntry, bool permanent) { +celix_status_t celix_framework_uninstallBundleEntry(celix_framework_t* framework, celix_bundle_entry_t* bndEntry, bool permanent) { celix_status_t status = CELIX_SUCCESS; celixThreadMutex_lock(&framework->installLock); status = celix_framework_uninstallBundleEntryImpl(framework, bndEntry, permanent); @@ -2040,7 +2037,7 @@ celix_status_t celix_framework_uninstallBundleEntry(celix_framework_t* framework static bool celix_framework_stopBundleInternal(celix_framework_t* fw, long bndId, bool forcedAsync) { bool stopped = false; - celix_framework_bundle_entry_t *bndEntry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(fw, bndId); + celix_bundle_entry_t*bndEntry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(fw, bndId); if (bndEntry != NULL) { celix_bundle_state_e state = celix_bundle_getState(bndEntry->bnd); if (state == CELIX_BUNDLE_STATE_ACTIVE) { @@ -2051,7 +2048,7 @@ static bool celix_framework_stopBundleInternal(celix_framework_t* fw, long bndId } else { fw_log(fw->logger, CELIX_LOG_LEVEL_WARNING, "Cannot stop bundle, bundle state is %s", celix_bundleState_getName(state)); } - celix_framework_bundleEntry_decreaseUseCount(bndEntry); + celix_bundleEntry_decreaseUseCount(bndEntry); if (!forcedAsync) { celix_framework_waitForBundleEvents(fw, bndId); } @@ -2067,7 +2064,8 @@ void celix_framework_stopBundleAsync(celix_framework_t* fw, long bndId) { celix_framework_stopBundleInternal(fw, bndId, true); } -static celix_status_t celix_framework_stopBundleEntryInternal(celix_framework_t* framework, celix_framework_bundle_entry_t* bndEntry) { +static celix_status_t celix_framework_stopBundleEntryInternal(celix_framework_t* framework, + celix_bundle_entry_t* bndEntry) { celix_bundle_activator_t *activator = NULL; bundle_context_t *context = NULL; bool wasActive = false; @@ -2175,7 +2173,7 @@ static celix_status_t celix_framework_stopBundleEntryInternal(celix_framework_t* return status; } -celix_status_t celix_framework_stopBundleEntry(celix_framework_t* framework, celix_framework_bundle_entry_t* bndEntry) { +celix_status_t celix_framework_stopBundleEntry(celix_framework_t* framework, celix_bundle_entry_t* bndEntry) { celix_status_t status = CELIX_SUCCESS; assert(!celix_framework_isCurrentThreadTheEventLoop(framework)); celixThreadRwlock_writeLock(&bndEntry->fsmMutex); @@ -2186,14 +2184,14 @@ celix_status_t celix_framework_stopBundleEntry(celix_framework_t* framework, cel bool celix_framework_startBundleInternal(celix_framework_t *fw, long bndId, bool forcedAsync) { bool started = false; - celix_framework_bundle_entry_t *bndEntry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(fw, bndId); + celix_bundle_entry_t*bndEntry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(fw, bndId); if (bndEntry != NULL) { celix_bundle_state_e state = celix_bundle_getState(bndEntry->bnd); if (state == CELIX_BUNDLE_STATE_INSTALLED || state == CELIX_BUNDLE_STATE_RESOLVED) { celix_status_t rc = celix_framework_startBundleOnANonCelixEventThread(fw, bndEntry, forcedAsync); started = rc == CELIX_SUCCESS; } - celix_framework_bundleEntry_decreaseUseCount(bndEntry); + celix_bundleEntry_decreaseUseCount(bndEntry); if (!forcedAsync) { celix_framework_waitForBundleEvents(fw, bndId); } @@ -2209,8 +2207,7 @@ void celix_framework_startBundleAsync(celix_framework_t *fw, long bndId) { celix_framework_startBundleInternal(fw, bndId, true); } -static void celix_framework_printCelixErrForBundleEntry(celix_framework_t* framework, - celix_framework_bundle_entry_t* bndEntry) { +static void celix_framework_printCelixErrForBundleEntry(celix_framework_t* framework, celix_bundle_entry_t* bndEntry) { if (celix_err_getErrorCount() > 0) { celix_framework_log(framework->logger, CELIX_LOG_LEVEL_WARNING, NULL, NULL, 0, "Found unprocessed celix err messages for bundle %s [bndId=%li]. Unprocessed celix err messages:", @@ -2225,7 +2222,7 @@ static void celix_framework_printCelixErrForBundleEntry(celix_framework_t* frame } } -celix_status_t celix_framework_startBundleEntry(celix_framework_t* framework, celix_framework_bundle_entry_t* bndEntry) { +celix_status_t celix_framework_startBundleEntry(celix_framework_t* framework, celix_bundle_entry_t* bndEntry) { assert(!celix_framework_isCurrentThreadTheEventLoop(framework)); celix_status_t status = CELIX_SUCCESS; const char* error = ""; @@ -2354,7 +2351,7 @@ static bool celix_framework_updateBundleInternal(celix_framework_t *fw, long bnd return true; } bool updated = false; - celix_framework_bundle_entry_t *bndEntry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(fw, bndId); + celix_bundle_entry_t*bndEntry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(fw, bndId); if (bndEntry != NULL) { celix_status_t rc = celix_framework_updateBundleOnANonCelixEventThread(fw, bndEntry, updatedBundleUrl, forcedAsync); //note not decreasing bndEntry, because this entry should now be deleted (uninstalled) @@ -2367,7 +2364,7 @@ static bool celix_framework_updateBundleInternal(celix_framework_t *fw, long bnd } celix_status_t celix_framework_updateBundleEntry(celix_framework_t* framework, - celix_framework_bundle_entry_t* bndEntry, + celix_bundle_entry_t* bndEntry, const char* updatedBundleUrl) { celix_status_t status = CELIX_SUCCESS; long bundleId = bndEntry->bndId; @@ -2393,17 +2390,14 @@ celix_status_t celix_framework_updateBundleEntry(celix_framework_t* framework, CELIX_LOG_LEVEL_ERROR, "Specified bundle location %s exists in cache with a different id. Update failed.", updatedBundleUrl); - celix_framework_bundleEntry_decreaseUseCount(bndEntry); + celix_bundleEntry_decreaseUseCount(bndEntry); return CELIX_ILLEGAL_STATE; } celix_bundleArchive_invalidateCache(celix_bundle_getArchive(bndEntry->bnd)); } status = celix_framework_uninstallBundleEntryImpl(framework, bndEntry, false); if (status != CELIX_SUCCESS) { - fw_log(framework->logger, - CELIX_LOG_LEVEL_ERROR, - "Failed to uninstall bundle %s", - celix_bundle_getSymbolicName(bndEntry->bnd)); + fw_log(framework->logger, CELIX_LOG_LEVEL_ERROR, "Failed to uninstall bundle id %li", bundleId); return status; } @@ -2423,11 +2417,11 @@ celix_status_t celix_framework_updateBundleEntry(celix_framework_t* framework, return CELIX_SUCCESS; } - celix_framework_bundle_entry_t* entry = + celix_bundle_entry_t* entry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(framework, bundleId); celixMutexLockGuard_deinit(&lck); status = celix_framework_startBundleEntry(framework, entry); - celix_framework_bundleEntry_decreaseUseCount(entry); + celix_bundleEntry_decreaseUseCount(entry); if (status != CELIX_SUCCESS) { fw_log(framework->logger, CELIX_LOG_LEVEL_ERROR, @@ -2451,7 +2445,7 @@ static celix_array_list_t* celix_framework_listBundlesInternal(celix_framework_t celix_array_list_t* result = celix_arrayList_create(); celix_auto(celix_mutex_lock_guard_t) lock = celixMutexLockGuard_init(&framework->installedBundles.mutex); for (int i = 0; i < celix_arrayList_size(framework->installedBundles.entries); ++i) { - celix_framework_bundle_entry_t* entry = celix_arrayList_get(framework->installedBundles.entries, i); + celix_bundle_entry_t* entry = celix_arrayList_get(framework->installedBundles.entries, i); if (entry->bndId == CELIX_FRAMEWORK_BUNDLE_ID) { continue; } @@ -2554,7 +2548,7 @@ long celix_framework_scheduleEvent(celix_framework_t* fw, return -1; } - celix_framework_bundle_entry_t* bndEntry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(fw, bndId); + celix_bundle_entry_t* bndEntry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(fw, bndId); if (bndEntry == NULL) { fw_log(fw->logger, CELIX_LOG_LEVEL_ERROR, "Cannot add scheduled event for non existing bundle id %li.", bndId); return -1; @@ -2571,7 +2565,7 @@ long celix_framework_scheduleEvent(celix_framework_t* fw, removeCallback); if (event == NULL) { - celix_framework_bundleEntry_decreaseUseCount(bndEntry); + celix_bundleEntry_decreaseUseCount(bndEntry); return -1L; //error logged by celix_scheduledEvent_create } @@ -2583,7 +2577,7 @@ long celix_framework_scheduleEvent(celix_framework_t* fw, id, celix_bundle_getSymbolicName(bndEntry->bnd), bndId); - celix_framework_bundleEntry_decreaseUseCount(bndEntry); + celix_bundleEntry_decreaseUseCount(bndEntry); celixThreadMutex_lock(&fw->dispatcher.mutex); celix_longHashMap_put(fw->dispatcher.scheduledEvents, id, event); @@ -2673,7 +2667,7 @@ void celix_framework_setLogCallback(celix_framework_t* fw, void* logHandle, void } long celix_framework_fireGenericEvent(framework_t* fw, long eventId, long bndId, const char *eventName, void* processData, void (*processCallback)(void *data), void* doneData, void (*doneCallback)(void* doneData)) { - celix_framework_bundle_entry_t* bndEntry = NULL; + celix_bundle_entry_t* bndEntry = NULL; if (bndId >=0) { bndEntry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(fw, bndId); if (bndEntry == NULL) { diff --git a/libs/framework/src/framework_bundle_lifecycle_handler.c b/libs/framework/src/framework_bundle_lifecycle_handler.c index 40caeb283..907b18ce0 100644 --- a/libs/framework/src/framework_bundle_lifecycle_handler.c +++ b/libs/framework/src/framework_bundle_lifecycle_handler.c @@ -41,22 +41,22 @@ static void* celix_framework_BundleLifecycleHandlingThread(void *data) { switch (handler->command) { case CELIX_BUNDLE_LIFECYCLE_START: celix_framework_startBundleEntry(handler->framework, handler->bndEntry); - celix_framework_bundleEntry_decreaseUseCount(handler->bndEntry); + celix_bundleEntry_decreaseUseCount(handler->bndEntry); break; case CELIX_BUNDLE_LIFECYCLE_STOP: celix_framework_stopBundleEntry(handler->framework, handler->bndEntry); - celix_framework_bundleEntry_decreaseUseCount(handler->bndEntry); + celix_bundleEntry_decreaseUseCount(handler->bndEntry); break; case CELIX_BUNDLE_LIFECYCLE_UNINSTALL: - celix_framework_bundleEntry_decreaseUseCount(handler->bndEntry); + celix_bundleEntry_decreaseUseCount(handler->bndEntry); celix_framework_uninstallBundleEntry(handler->framework, handler->bndEntry, true); break; case CELIX_BUNDLE_LIFECYCLE_UNLOAD: - celix_framework_bundleEntry_decreaseUseCount(handler->bndEntry); + celix_bundleEntry_decreaseUseCount(handler->bndEntry); celix_framework_uninstallBundleEntry(handler->framework, handler->bndEntry, false); break; default: //update - celix_framework_bundleEntry_decreaseUseCount(handler->bndEntry); + celix_bundleEntry_decreaseUseCount(handler->bndEntry); celix_framework_updateBundleEntry(handler->framework, handler->bndEntry, handler->updatedBundleUrl); break; } @@ -87,8 +87,9 @@ void celix_framework_waitForBundleLifecycleHandlers(celix_framework_t* fw) { celixThreadMutex_unlock(&fw->bundleLifecycleHandling.mutex); } -static void celix_framework_createAndStartBundleLifecycleHandler(celix_framework_t* fw, celix_framework_bundle_entry_t* bndEntry, enum celix_bundle_lifecycle_command bndCommand, const char* updatedBundleUrl) { - celix_framework_bundleEntry_increaseUseCount(bndEntry); +static void celix_framework_createAndStartBundleLifecycleHandler(celix_framework_t* fw, + celix_bundle_entry_t* bndEntry, enum celix_bundle_lifecycle_command bndCommand, const char* updatedBundleUrl) { + celix_bundleEntry_increaseUseCount(bndEntry); celixThreadMutex_lock(&fw->bundleLifecycleHandling.mutex); celix_framework_bundle_lifecycle_handler_t* handler = calloc(1, sizeof(*handler)); handler->command = bndCommand; @@ -106,7 +107,8 @@ static void celix_framework_createAndStartBundleLifecycleHandler(celix_framework celixThreadMutex_unlock(&fw->bundleLifecycleHandling.mutex); } -celix_status_t celix_framework_startBundleOnANonCelixEventThread(celix_framework_t* fw, celix_framework_bundle_entry_t* bndEntry, bool forceSpawnThread) { +celix_status_t celix_framework_startBundleOnANonCelixEventThread(celix_framework_t* fw, + celix_bundle_entry_t* bndEntry, bool forceSpawnThread) { if (forceSpawnThread) { fw_log(fw->logger, CELIX_LOG_LEVEL_TRACE, "start bundle from a separate thread"); celix_framework_createAndStartBundleLifecycleHandler(fw, bndEntry, CELIX_BUNDLE_LIFECYCLE_START, NULL); @@ -121,7 +123,8 @@ celix_status_t celix_framework_startBundleOnANonCelixEventThread(celix_framework } } -celix_status_t celix_framework_stopBundleOnANonCelixEventThread(celix_framework_t* fw, celix_framework_bundle_entry_t* bndEntry, bool forceSpawnThread) { +celix_status_t celix_framework_stopBundleOnANonCelixEventThread(celix_framework_t* fw, + celix_bundle_entry_t* bndEntry, bool forceSpawnThread) { if (forceSpawnThread) { fw_log(fw->logger, CELIX_LOG_LEVEL_TRACE, "stop bundle from a separate thread"); celix_framework_createAndStartBundleLifecycleHandler(fw, bndEntry, CELIX_BUNDLE_LIFECYCLE_STOP, NULL); @@ -136,7 +139,8 @@ celix_status_t celix_framework_stopBundleOnANonCelixEventThread(celix_framework_ } } -celix_status_t celix_framework_uninstallBundleOnANonCelixEventThread(celix_framework_t* fw, celix_framework_bundle_entry_t* bndEntry, bool forceSpawnThread, bool permanent) { +celix_status_t celix_framework_uninstallBundleOnANonCelixEventThread(celix_framework_t* fw, + celix_bundle_entry_t* bndEntry, bool forceSpawnThread, bool permanent) { if (forceSpawnThread) { fw_log(fw->logger, CELIX_LOG_LEVEL_TRACE, "uninstall bundle from a separate thread"); celix_framework_createAndStartBundleLifecycleHandler(fw, bndEntry, permanent ? CELIX_BUNDLE_LIFECYCLE_UNINSTALL : CELIX_BUNDLE_LIFECYCLE_UNLOAD, NULL); @@ -151,7 +155,8 @@ celix_status_t celix_framework_uninstallBundleOnANonCelixEventThread(celix_frame } } -celix_status_t celix_framework_updateBundleOnANonCelixEventThread(celix_framework_t* fw, celix_framework_bundle_entry_t* bndEntry, const char* updatedBundleUrl, bool forceSpawnThread) { +celix_status_t celix_framework_updateBundleOnANonCelixEventThread(celix_framework_t* fw, + celix_bundle_entry_t* bndEntry, const char* updatedBundleUrl, bool forceSpawnThread) { if (forceSpawnThread) { fw_log(fw->logger, CELIX_LOG_LEVEL_TRACE, "update bundle from a separate thread"); celix_framework_createAndStartBundleLifecycleHandler(fw, bndEntry, CELIX_BUNDLE_LIFECYCLE_UPDATE, updatedBundleUrl); diff --git a/libs/framework/src/framework_private.h b/libs/framework/src/framework_private.h index a00d6d8c6..788a8dc59 100644 --- a/libs/framework/src/framework_private.h +++ b/libs/framework/src/framework_private.h @@ -55,7 +55,7 @@ extern "C" { #define CELIX_FRAMEWORK_CACHE_USE_TMP_DIR_DEFAULT false #define CELIX_FRAMEWORK_CACHE_DIR_DEFAULT ".cache" -typedef struct celix_framework_bundle_entry { +typedef struct celix_bundle_entry { celix_bundle_t *bnd; celix_thread_rwlock_t fsmMutex; //protects bundle state transition long bndId; @@ -63,7 +63,7 @@ typedef struct celix_framework_bundle_entry { celix_thread_mutex_t useMutex; //protects useCount celix_thread_cond_t useCond; size_t useCount; -} celix_framework_bundle_entry_t; +} celix_bundle_entry_t; enum celix_framework_event_type { CELIX_FRAMEWORK_EVENT_TYPE = 0x01, @@ -77,7 +77,7 @@ typedef enum celix_framework_event_type celix_framework_event_type_e; struct celix_framework_event { celix_framework_event_type_e type; - celix_framework_bundle_entry_t* bndEntry; + celix_bundle_entry_t* bndEntry; void *doneData; void (*doneCallback)(void*); @@ -124,7 +124,7 @@ enum celix_bundle_lifecycle_command { typedef struct celix_framework_bundle_lifecycle_handler { celix_framework_t* framework; - celix_framework_bundle_entry_t* bndEntry; + celix_bundle_entry_t* bndEntry; long bndId; char* updatedBundleUrl; //only relevant and present for update command enum celix_bundle_lifecycle_command command; @@ -347,17 +347,45 @@ void celix_framework_waitForAsyncUnregistration(celix_framework_t *fw, long svcI /** * Increase the use count of a bundle and ensure that a bundle cannot be uninstalled. */ -void celix_framework_bundleEntry_increaseUseCount(celix_framework_bundle_entry_t *entry); +void celix_bundleEntry_increaseUseCount(celix_bundle_entry_t*entry); /** * Decrease the use count of a bundle. */ -void celix_framework_bundleEntry_decreaseUseCount(celix_framework_bundle_entry_t *entry); +void celix_bundleEntry_decreaseUseCount(celix_bundle_entry_t*entry); + +/** + * @brief Use guard for bundle entry usage. + */ +typedef struct celix_bundle_entry_use_guard { + celix_bundle_entry_t* entry; +} celix_bundle_entry_use_guard_t; + +/** + * @brief Initialize a bundle entry use guard. + */ +static CELIX_UNUSED inline celix_bundle_entry_use_guard_t celix_bundleEntryUseGuard_init(celix_bundle_entry_t* entry) { + celix_bundle_entry_use_guard_t guard; + guard.entry = entry; + return guard; +} + +/** + * @brief De-initialize a bundle entry use guard. + */ +static CELIX_UNUSED inline void celix_bundleEntryUseGuard_deinit(celix_bundle_entry_use_guard_t* guard) { + if (guard->entry) { + celix_bundleEntry_decreaseUseCount(guard->entry); + } + guard->entry = NULL; +} + +CELIX_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(celix_bundle_entry_use_guard_t, celix_bundleEntryUseGuard_deinit) /** * Find the bundle entry for the bnd id and increase use count */ -celix_framework_bundle_entry_t* celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(celix_framework_t *fw, long bndId); +celix_bundle_entry_t* celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(celix_framework_t *fw, long bndId); /** * @brief Check if the bundle id is already in use. @@ -381,7 +409,8 @@ bool celix_framework_isBundleAlreadyInstalled(celix_framework_t* fw, const char* * @param forceSpawnThread If the true, the start bundle will always be done on a spawn thread * @return CELIX_SUCCESS of the call went alright. */ -celix_status_t celix_framework_startBundleOnANonCelixEventThread(celix_framework_t* fw, celix_framework_bundle_entry_t* bndEntry, bool forceSpawnThread); +celix_status_t celix_framework_startBundleOnANonCelixEventThread(celix_framework_t* fw, + celix_bundle_entry_t* bndEntry, bool forceSpawnThread); /** * Stop a bundle and ensure that this is not done on the Celix event thread. @@ -391,7 +420,8 @@ celix_status_t celix_framework_startBundleOnANonCelixEventThread(celix_framework * @param forceSpawnThread If the true, the start bundle will always be done on a spawn thread * @return CELIX_SUCCESS of the call went alright. */ -celix_status_t celix_framework_stopBundleOnANonCelixEventThread(celix_framework_t* fw, celix_framework_bundle_entry_t* bndEntry, bool forceSpawnThread); +celix_status_t celix_framework_stopBundleOnANonCelixEventThread(celix_framework_t* fw, + celix_bundle_entry_t* bndEntry, bool forceSpawnThread); /** * Uninstall (and if needed stop) a bundle and ensure that this is not done on the Celix event thread. @@ -402,7 +432,8 @@ celix_status_t celix_framework_stopBundleOnANonCelixEventThread(celix_framework_ * @param permanent If true, the bundle will be permanently uninstalled (e.g. the bundle archive will be removed). * @return CELIX_SUCCESS of the call went alright. */ -celix_status_t celix_framework_uninstallBundleOnANonCelixEventThread(celix_framework_t* fw, celix_framework_bundle_entry_t* bndEntry, bool forceSpawnThread, bool permanent); +celix_status_t celix_framework_uninstallBundleOnANonCelixEventThread(celix_framework_t* fw, + celix_bundle_entry_t* bndEntry, bool forceSpawnThread, bool permanent); /** * Update (and if needed stop and start) a bundle and ensure that this is not done on the Celix event thread. @@ -411,7 +442,8 @@ celix_status_t celix_framework_uninstallBundleOnANonCelixEventThread(celix_frame * @param forceSpawnThread If the true, the start bundle will always be done on a spawn thread * @return CELIX_SUCCESS of the call went alright. */ -celix_status_t celix_framework_updateBundleOnANonCelixEventThread(celix_framework_t* fw, celix_framework_bundle_entry_t* bndEntry, const char* updatedBundleUrl, bool forceSpawnThread); +celix_status_t celix_framework_updateBundleOnANonCelixEventThread(celix_framework_t* fw, + celix_bundle_entry_t* bndEntry, const char* updatedBundleUrl, bool forceSpawnThread); /** * Wait for all bundle lifecycle handlers finishing their jobs. @@ -422,22 +454,22 @@ void celix_framework_waitForBundleLifecycleHandlers(celix_framework_t* fw); /** * Start a bundle. Cannot be called on the Celix event thread. */ -celix_status_t celix_framework_startBundleEntry(celix_framework_t* fw, celix_framework_bundle_entry_t* bndEntry); +celix_status_t celix_framework_startBundleEntry(celix_framework_t* fw, celix_bundle_entry_t* bndEntry); /** * Stop a bundle. Cannot be called on the Celix event thread. */ -celix_status_t celix_framework_stopBundleEntry(celix_framework_t* fw, celix_framework_bundle_entry_t* bndEntry); +celix_status_t celix_framework_stopBundleEntry(celix_framework_t* fw, celix_bundle_entry_t* bndEntry); /** * Uninstall a bundle. Cannot be called on the Celix event thread. */ -celix_status_t celix_framework_uninstallBundleEntry(celix_framework_t* fw, celix_framework_bundle_entry_t* bndEntry, bool permanent); +celix_status_t celix_framework_uninstallBundleEntry(celix_framework_t* fw, celix_bundle_entry_t* bndEntry, bool permanent); /** * Update a bundle. Cannot be called on the Celix event thread. */ -celix_status_t celix_framework_updateBundleEntry(celix_framework_t* fw, celix_framework_bundle_entry_t* bndEntry, const char* updatedBundleUrl); +celix_status_t celix_framework_updateBundleEntry(celix_framework_t* fw, celix_bundle_entry_t* bndEntry, const char* updatedBundleUrl); /** @brief Return the next scheduled event id. From f237f4b2004a8995a226f913d63adb8d8dfd918d Mon Sep 17 00:00:00 2001 From: Pepijn Noltes Date: Sun, 9 Jun 2024 17:04:10 +0200 Subject: [PATCH 05/21] gh-685: Add rsa shm test case And some small improvements based on review comments --- .../src/RsaShmClientServerUnitTestSuite.cc | 40 +++++++++++++++++++ .../rsa_shm/src/rsa_shm_client.c | 1 + cmake/cmake_celix/ContainerPackaging.cmake | 1 - libs/framework/src/framework.c | 6 +-- .../celix_properties/CMakeLists.txt | 1 + .../include/celix_properties_ei.h | 1 + .../src/celix_properties_ei.cc | 9 +++++ 7 files changed, 54 insertions(+), 5 deletions(-) diff --git a/bundles/remote_services/remote_service_admin_shm_v2/rsa_shm/gtest/src/RsaShmClientServerUnitTestSuite.cc b/bundles/remote_services/remote_service_admin_shm_v2/rsa_shm/gtest/src/RsaShmClientServerUnitTestSuite.cc index 44ae8d99e..77ab646d5 100644 --- a/bundles/remote_services/remote_service_admin_shm_v2/rsa_shm/gtest/src/RsaShmClientServerUnitTestSuite.cc +++ b/bundles/remote_services/remote_service_admin_shm_v2/rsa_shm/gtest/src/RsaShmClientServerUnitTestSuite.cc @@ -35,6 +35,7 @@ #include "socket_ei.h" #include "stdio_ei.h" #include "pthread_ei.h" +#include "celix_properties_ei.h" #include "thpool_ei.h" #include "celix_errno.h" #include @@ -80,6 +81,7 @@ class RsaShmClientServerUnitTestSuite : public ::testing::Test { celix_ei_expect_pthread_cond_timedwait(nullptr, 1, 0); celix_ei_expect_thpool_init(nullptr, 0, nullptr); celix_ei_expect_thpool_add_work(nullptr, 0, 0); + celix_ei_expect_celix_properties_saveToStream(nullptr, 0, CELIX_SUCCESS); } @@ -139,6 +141,44 @@ TEST_F(RsaShmClientServerUnitTestSuite, SendMsg) { rsaShmServer_destroy(server); } +TEST_F(RsaShmClientServerUnitTestSuite, SendMsgErrorEncodePropertiesTest) { + //Given a rsa shm server + rsa_shm_server_t *server = nullptr; + auto status = rsaShmServer_create(ctx.get(), "shm_test_server", logHelper.get(), ReceiveMsgCallback, nullptr, &server); + EXPECT_EQ(CELIX_SUCCESS, status); + EXPECT_NE(nullptr, server); + + //And a rsa shm client + rsa_shm_client_manager_t *clientManager = nullptr; + status = rsaShmClientManager_create(ctx.get(), logHelper.get(), &clientManager); + EXPECT_EQ(CELIX_SUCCESS, status); + EXPECT_NE(nullptr, clientManager); + + // And the client is attached to the server + long serverId = 100;//dummy id + status = rsaShmClientManager_createOrAttachClient(clientManager, "shm_test_server", serverId); + EXPECT_EQ(CELIX_SUCCESS, status); + + //When an error is prepared for saveToStream + celix_ei_expect_celix_properties_saveToStream((void*)rsaShmClientManager_sendMsgTo, 0, ENOMEM); + + //And a message is sent + celix_autoptr(celix_properties_t) metadata = celix_properties_create(); + celix_properties_set(metadata, "CustomKey", "test"); + struct iovec request = {.iov_base = (void*)"request", .iov_len = strlen("request")}; + struct iovec response = {.iov_base = nullptr, .iov_len = 0}; + status = rsaShmClientManager_sendMsgTo(clientManager, "shm_test_server", serverId, metadata, &request, &response); + + //Then the injected error is returned + EXPECT_EQ(ENOMEM, status); + + rsaShmClientManager_destroyOrDetachClient(clientManager, "shm_test_server", serverId); + + rsaShmClientManager_destroy(clientManager); + + rsaShmServer_destroy(server); +} + TEST_F(RsaShmClientServerUnitTestSuite, SendMsgWithNoServer) { rsa_shm_client_manager_t *clientManager = nullptr; auto status = rsaShmClientManager_create(ctx.get(), logHelper.get(), &clientManager); diff --git a/bundles/remote_services/remote_service_admin_shm_v2/rsa_shm/src/rsa_shm_client.c b/bundles/remote_services/remote_service_admin_shm_v2/rsa_shm/src/rsa_shm_client.c index aa5a480b5..70ffba0b7 100644 --- a/bundles/remote_services/remote_service_admin_shm_v2/rsa_shm/src/rsa_shm_client.c +++ b/bundles/remote_services/remote_service_admin_shm_v2/rsa_shm/src/rsa_shm_client.c @@ -280,6 +280,7 @@ celix_status_t rsaShmClientManager_sendMsgTo(rsa_shm_client_manager_t *clientMan if (metadata != NULL) { status = celix_properties_saveToStream(metadata, fp, 0); if (status != CELIX_SUCCESS) { + fclose(fp); celix_logHelper_error( clientManager->logHelper, "RsaShmClient: Error encoding metadata to memory stream. %d.", status); celix_logHelper_logTssErrors(clientManager->logHelper, CELIX_LOG_LEVEL_ERROR); diff --git a/cmake/cmake_celix/ContainerPackaging.cmake b/cmake/cmake_celix/ContainerPackaging.cmake index 77044884c..71c8c3c19 100644 --- a/cmake/cmake_celix/ContainerPackaging.cmake +++ b/cmake/cmake_celix/ContainerPackaging.cmake @@ -217,7 +217,6 @@ function(add_celix_container) file(GENERATE OUTPUT "${STAGE1_LAUNCHER_SRC}" CONTENT "#include -#include #define CELIX_MULTI_LINE_STRING(...) #__VA_ARGS__ diff --git a/libs/framework/src/framework.c b/libs/framework/src/framework.c index bd60e1837..287a59259 100644 --- a/libs/framework/src/framework.c +++ b/libs/framework/src/framework.c @@ -574,11 +574,9 @@ static bool framework_autoStartConfiguredBundlesForList(celix_framework_t* fw, } } else { fw_log(fw->logger, - CELIX_LOG_LEVEL_TRACE, - "Cannot start bundle %s (bnd id = %li), because it is already started\n", - bnd->symbolicName, + CELIX_LOG_LEVEL_WARNING, + "Cannot start bundle %s (bnd id = %li) again, already started\n", bnd->symbolicName, bndId); - allStarted = false; } } return allStarted; diff --git a/libs/utils/error_injector/celix_properties/CMakeLists.txt b/libs/utils/error_injector/celix_properties/CMakeLists.txt index aa177a63f..0155b75e7 100644 --- a/libs/utils/error_injector/celix_properties/CMakeLists.txt +++ b/libs/utils/error_injector/celix_properties/CMakeLists.txt @@ -28,6 +28,7 @@ target_link_options(properties_ei INTERFACE LINKER:--wrap,celix_properties_setVersion LINKER:--wrap,celix_properties_setEntry LINKER:--wrap,celix_properties_save + LINKER:--wrap,celix_properties_saveToStream LINKER:--wrap,celix_properties_load ) add_library(Celix::properties_ei ALIAS properties_ei) diff --git a/libs/utils/error_injector/celix_properties/include/celix_properties_ei.h b/libs/utils/error_injector/celix_properties/include/celix_properties_ei.h index 9291d0c5e..a98905c54 100644 --- a/libs/utils/error_injector/celix_properties/include/celix_properties_ei.h +++ b/libs/utils/error_injector/celix_properties/include/celix_properties_ei.h @@ -33,6 +33,7 @@ CELIX_EI_DECLARE(celix_properties_setLong, celix_status_t); CELIX_EI_DECLARE(celix_properties_setVersion, celix_status_t); CELIX_EI_DECLARE(celix_properties_setEntry, celix_status_t); CELIX_EI_DECLARE(celix_properties_save, celix_status_t); +CELIX_EI_DECLARE(celix_properties_saveToStream, celix_status_t); CELIX_EI_DECLARE(celix_properties_load, celix_status_t); #ifdef __cplusplus diff --git a/libs/utils/error_injector/celix_properties/src/celix_properties_ei.cc b/libs/utils/error_injector/celix_properties/src/celix_properties_ei.cc index 1ba0903ca..c0bfe60a4 100644 --- a/libs/utils/error_injector/celix_properties/src/celix_properties_ei.cc +++ b/libs/utils/error_injector/celix_properties/src/celix_properties_ei.cc @@ -75,6 +75,15 @@ __wrap_celix_properties_save(const celix_properties_t* properties, const char* f return __real_celix_properties_save(properties, filename, encodeFlags); } +celix_status_t +__real_celix_properties_saveToStream(const celix_properties_t* properties, FILE* stream, int encodeFlags); +CELIX_EI_DEFINE(celix_properties_saveToStream, celix_status_t) +celix_status_t +__wrap_celix_properties_saveToStream(const celix_properties_t* properties, FILE* stream, int encodeFlags) { + CELIX_EI_IMPL(celix_properties_saveToStream); + return __real_celix_properties_saveToStream(properties, stream, encodeFlags); +} + celix_status_t __real_celix_properties_load(const char* filename, int decodeFlags, From 17a5adac5014cb774860a99ccd01e2b4e85fbb7b Mon Sep 17 00:00:00 2001 From: PengZheng Date: Mon, 10 Jun 2024 14:50:21 +0800 Subject: [PATCH 06/21] Cleanup bundle cache on installation failure and fix use-after-free. --- libs/framework/gtest/CMakeLists.txt | 6 ++- .../src/CelixBundleContextBundlesTestSuite.cc | 9 ++++ libs/framework/src/bundle.c | 2 +- libs/framework/src/framework.c | 49 +++++++++---------- libs/framework/src/framework_private.h | 2 +- 5 files changed, 39 insertions(+), 29 deletions(-) diff --git a/libs/framework/gtest/CMakeLists.txt b/libs/framework/gtest/CMakeLists.txt index af3f60c0a..a6184dada 100644 --- a/libs/framework/gtest/CMakeLists.txt +++ b/libs/framework/gtest/CMakeLists.txt @@ -20,6 +20,8 @@ celix_bundle_name(simple_test_bundle1 "Simple Test Bundle") celix_bundle_group(simple_test_bundle1 "test/group") celix_bundle_description(simple_test_bundle1 "Test Description") +add_celix_bundle(dup_symbolic_name_bundle NO_ACTIVATOR VERSION 1.0.0 SYMBOLIC_NAME simple_test_bundle1) + add_celix_bundle(simple_test_bundle2 NO_ACTIVATOR VERSION 1.0.0) add_celix_bundle(simple_test_bundle3 NO_ACTIVATOR VERSION 1.0.0) add_celix_bundle(bundle_with_exception SOURCES src/nop_activator.c VERSION 1.0.0) @@ -80,11 +82,12 @@ add_celix_bundle_dependencies(test_framework simple_test_bundle2 simple_test_bundle3 simple_test_bundle4 simple_test_bundle5 bundle_with_exception bundle_with_bad_export unresolvable_bundle simple_cxx_bundle simple_cxx_dep_man_bundle cmp_test_bundle - celix_err_test_bundle) + celix_err_test_bundle dup_symbolic_name_bundle) target_include_directories(test_framework PRIVATE ../src) celix_deprecated_utils_headers(test_framework) celix_get_bundle_file(simple_test_bundle1 SIMPLE_TEST_BUNDLE1) +celix_get_bundle_file(dup_symbolic_name_bundle DUP_SYMBOLIC_NAME_BUNDLE) celix_get_bundle_file(simple_test_bundle2 SIMPLE_TEST_BUNDLE2) celix_get_bundle_file(simple_test_bundle3 SIMPLE_TEST_BUNDLE3) celix_get_bundle_file(simple_test_bundle4 SIMPLE_TEST_BUNDLE4) @@ -115,6 +118,7 @@ target_compile_definitions(test_framework PRIVATE SIMPLE_TEST_BUNDLE3_LOCATION="${SIMPLE_TEST_BUNDLE3}" SIMPLE_TEST_BUNDLE4_LOCATION="${SIMPLE_TEST_BUNDLE4_FILENAME}" SIMPLE_TEST_BUNDLE5_LOCATION="${SIMPLE_TEST_BUNDLE5_FILENAME}" + DUP_SYMBOLIC_NAME_BUNDLE_LOCATION="${DUP_SYMBOLIC_NAME_BUNDLE}" TEST_BUNDLE_WITH_EXCEPTION_LOCATION="${BUNDLE_WITH_EXCEPTION}" BUNDLE_WITH_BAD_EXPORT_LOCATION="${BUNDLE_WITH_BAD_EXPORT}" TEST_BUNDLE_UNRESOLVABLE_LOCATION="${UNRESOLVABLE_BUNDLE}" diff --git a/libs/framework/gtest/src/CelixBundleContextBundlesTestSuite.cc b/libs/framework/gtest/src/CelixBundleContextBundlesTestSuite.cc index 01616065b..8a1c89336 100644 --- a/libs/framework/gtest/src/CelixBundleContextBundlesTestSuite.cc +++ b/libs/framework/gtest/src/CelixBundleContextBundlesTestSuite.cc @@ -47,6 +47,7 @@ class CelixBundleContextBundlesTestSuite : public ::testing::Test { const char * const TEST_BND5_LOC = "" SIMPLE_TEST_BUNDLE5_LOCATION ""; const char * const TEST_BND_WITH_EXCEPTION_LOC = "" TEST_BUNDLE_WITH_EXCEPTION_LOCATION ""; const char * const TEST_BND_UNRESOLVABLE_LOC = "" TEST_BUNDLE_UNRESOLVABLE_LOCATION ""; + const char * const DUP_SYMBOLIC_NAME_BUNDLE_LOC = "" DUP_SYMBOLIC_NAME_BUNDLE_LOCATION ""; CelixBundleContextBundlesTestSuite() { properties = celix_properties_create(); @@ -79,6 +80,14 @@ TEST_F(CelixBundleContextBundlesTestSuite, InstallABundleTest) { ASSERT_TRUE(bndId >= 0); } +TEST_F(CelixBundleContextBundlesTestSuite, InstallBundleWithDuplicatedSymbolicNameTest) { + long bndId1 = celix_bundleContext_installBundle(ctx, TEST_BND1_LOC, true); + ASSERT_TRUE(bndId1 >= 0); + + long bndId2 = celix_bundleContext_installBundle(ctx, DUP_SYMBOLIC_NAME_BUNDLE_LOC, true); + ASSERT_TRUE(bndId2 < 0); +} + //TEST_F(CelixBundleContextBundlesTestSuite, InstallBundleWithBadExport) { // long bndId = celix_bundleContext_installBundle(ctx, BUNDLE_WITH_BAD_EXPORT_LOCATION, true); // ASSERT_TRUE(bndId >= 0); diff --git a/libs/framework/src/bundle.c b/libs/framework/src/bundle.c index 97a4f6e29..033a3541e 100644 --- a/libs/framework/src/bundle.c +++ b/libs/framework/src/bundle.c @@ -79,7 +79,7 @@ celix_status_t celix_bundle_createFromArchive(celix_framework_t *framework, bund } *bundleOut = bundle; - return status; + return status; } celix_status_t bundle_destroy(bundle_pt bundle) { diff --git a/libs/framework/src/framework.c b/libs/framework/src/framework.c index 287a59259..797fce227 100644 --- a/libs/framework/src/framework.c +++ b/libs/framework/src/framework.c @@ -685,10 +685,13 @@ celix_framework_installBundleInternalImpl(celix_framework_t* framework, const ch bundle_archive_t* archive = NULL; celix_bundle_t* bundle = NULL; celix_status_t status = celix_bundleCache_createArchive(framework->cache, id, bndLoc, &archive); - status = CELIX_DO_IF(status, celix_bundle_createFromArchive(framework, archive, &bundle)); if (status != CELIX_SUCCESS) { - celix_bundleArchive_destroy(archive); - fw_logCode(framework->logger, CELIX_LOG_LEVEL_ERROR, status, "Cannot install bundle without archive"); + return status; + } + status = celix_bundle_createFromArchive(framework, archive, &bundle); + if (status != CELIX_SUCCESS) { + celix_bundleArchive_invalidate(archive); + celix_bundleCache_destroyArchive(framework->cache, archive); return status; } @@ -783,19 +786,19 @@ celix_status_t fw_populateDependentGraph(framework_pt framework, bundle_pt expor } celix_status_t fw_registerService(framework_pt framework, service_registration_pt *registration, long bndId, const char* serviceName, const void* svcObj, celix_properties_t *properties) { - celix_status_t status = CELIX_SUCCESS; - char *error = NULL; - if (serviceName == NULL || svcObj == NULL) { - status = CELIX_ILLEGAL_ARGUMENT; - error = "ServiceName and SvcObj cannot be null"; - } + celix_status_t status = CELIX_SUCCESS; + char *error = NULL; + if (serviceName == NULL || svcObj == NULL) { + status = CELIX_ILLEGAL_ARGUMENT; + error = "ServiceName and SvcObj cannot be null"; + } - celix_bundle_entry_t*entry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(framework, - bndId); + celix_bundle_entry_t*entry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(framework, + bndId); status = CELIX_DO_IF(status, serviceRegistry_registerService(framework->registry, entry->bnd, serviceName, svcObj, properties, registration)); celix_bundleEntry_decreaseUseCount(entry); framework_logIfError(framework->logger, status, error, "Cannot register service: %s", serviceName); - return status; + return status; } celix_status_t fw_registerServiceFactory(framework_pt framework, service_registration_pt *registration, long bndId, const char* serviceName, service_factory_pt factory, celix_properties_t* properties) { @@ -1911,9 +1914,8 @@ static void celix_framework_waitForBundleEvents(celix_framework_t *fw, long bndI static long celix_framework_installAndStartBundleInternal(celix_framework_t *fw, const char *bundleLoc, bool autoStart, bool forcedAsync) { long bundleId = -1; - celix_status_t status = CELIX_SUCCESS; - - if (celix_framework_installBundleInternal(fw, bundleLoc, &bundleId) == CELIX_SUCCESS) { + celix_status_t status = celix_framework_installBundleInternal(fw, bundleLoc, &bundleId) ; + if (status == CELIX_SUCCESS) { if (autoStart) { celix_bundle_entry_t* bndEntry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(fw, bundleId); @@ -2375,15 +2377,10 @@ celix_status_t celix_framework_updateBundleEntry(celix_framework_t* framework, if (updatedBundleUrl == NULL) { updatedBundleUrl = location; } else if (strcmp(location, updatedBundleUrl) != 0) { - long existingBndId; - status = celix_bundleCache_findBundleIdForLocation(framework->cache, updatedBundleUrl, &existingBndId); - if (status != CELIX_SUCCESS) { - fw_log(framework->logger, - CELIX_LOG_LEVEL_ERROR, - "Could not read bundle cache to check if bundle location %s exists in cache. Update failed.", - updatedBundleUrl); - return status; - } else if (existingBndId != -1 && existingBndId != bundleId) { + long existingBndId = -1L; + // celix_bundleCache_findBundleIdForLocation can never fail since there is at lease bndEntry is installed + (void)celix_bundleCache_findBundleIdForLocation(framework->cache, updatedBundleUrl, &existingBndId); + if (existingBndId != -1 && existingBndId != bundleId) { fw_log(framework->logger, CELIX_LOG_LEVEL_ERROR, "Specified bundle location %s exists in cache with a different id. Update failed.", @@ -2417,14 +2414,14 @@ celix_status_t celix_framework_updateBundleEntry(celix_framework_t* framework, celix_bundle_entry_t* entry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(framework, bundleId); + celix_auto(celix_bundle_entry_use_guard_t) entryGuard = celix_bundleEntryUseGuard_init(entry); celixMutexLockGuard_deinit(&lck); status = celix_framework_startBundleEntry(framework, entry); - celix_bundleEntry_decreaseUseCount(entry); if (status != CELIX_SUCCESS) { fw_log(framework->logger, CELIX_LOG_LEVEL_ERROR, "Failed to start updated bundle %s", - celix_bundle_getSymbolicName(bndEntry->bnd)); + celix_bundle_getSymbolicName(entry->bnd)); return CELIX_BUNDLE_EXCEPTION; } diff --git a/libs/framework/src/framework_private.h b/libs/framework/src/framework_private.h index 788a8dc59..457e16944 100644 --- a/libs/framework/src/framework_private.h +++ b/libs/framework/src/framework_private.h @@ -376,8 +376,8 @@ static CELIX_UNUSED inline celix_bundle_entry_use_guard_t celix_bundleEntryUseGu static CELIX_UNUSED inline void celix_bundleEntryUseGuard_deinit(celix_bundle_entry_use_guard_t* guard) { if (guard->entry) { celix_bundleEntry_decreaseUseCount(guard->entry); + guard->entry = NULL; } - guard->entry = NULL; } CELIX_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(celix_bundle_entry_use_guard_t, celix_bundleEntryUseGuard_deinit) From ad9e542c1ef080ed809ca7502e2beaac8ad7c72a Mon Sep 17 00:00:00 2001 From: Pepijn Noltes Date: Mon, 10 Jun 2024 19:41:41 +0200 Subject: [PATCH 07/21] gh-685: Refactor launcher global framework handling --- libs/framework/include/celix_launcher.h | 9 +-- libs/framework/src/celix_launcher.c | 73 ++++++++++++++++--------- 2 files changed, 51 insertions(+), 31 deletions(-) diff --git a/libs/framework/include/celix_launcher.h b/libs/framework/include/celix_launcher.h index 78631db3d..97c6fa870 100644 --- a/libs/framework/include/celix_launcher.h +++ b/libs/framework/include/celix_launcher.h @@ -28,7 +28,6 @@ extern "C" { #endif - /** * @brief Launch a celix framework, wait (block) until the framework is stopped and destroy the framework when stopped. * @@ -36,8 +35,10 @@ extern "C" { * For SIGINT and SIGTERM the framework will be stopped. * SIGUSR1 and SIGUSR2 will be ignored. * - * Note that the Celix launcher uses a global variable to store the framework instance and as such only one framework - * can be launched at a time using this function. + * The Celix launcher can only controls a single framework instance. If multiple frameworks are needed, + * `celix_frameworkFactory_createFramework` or `celix::createFramework` should be used. If the celix launcher is called + * while a framework is already running or being launched, the launcher will print an error message to stderr and + * return 1. * * @param argc argc as provided in a main function. * @param argv argv as provided in a main function. @@ -45,7 +46,7 @@ extern "C" { * @return 0 if successful and return 1 if the framework could not be launched. Reason for not launching the framework * will be logged. */ -CELIX_FRAMEWORK_EXPORT int celix_launcher_launchAndWait(int argc, char *argv[], const char* embeddedConfig); +CELIX_FRAMEWORK_EXPORT int celix_launcher_launchAndWait(int argc, char* argv[], const char* embeddedConfig); /** * @brief Trigger the stop of the Celix framework. diff --git a/libs/framework/src/celix_launcher.c b/libs/framework/src/celix_launcher.c index 5e278ac24..35ea7460b 100644 --- a/libs/framework/src/celix_launcher.c +++ b/libs/framework/src/celix_launcher.c @@ -42,7 +42,8 @@ #define CELIX_LAUNCHER_OK_EXIT_CODE 0 #define CELIX_LAUNCHER_ERROR_EXIT_CODE 1 -static framework_t* g_fw = NULL; +static bool g_framework_launched = false; +static framework_t* g_framework = NULL; typedef struct { bool showHelp; @@ -61,6 +62,17 @@ static void celix_launcher_noopSignalHandler(int signal); */ static void celix_launcher_shutdownFrameworkSignalHandler(int signal); +/** + * @brief Check and set if a framework can be launched. + * @return true if the framework can be launched. + */ +static bool celix_launcher_checkFrameworkLaunched(); + +/** + * @brief Reset the launcher state. + */ +static void celix_launcher_resetLauncher(); + /** * @brief Create a Celix framework instance with the given embedded and runtime properties. * @@ -107,11 +119,8 @@ static celix_status_t celix_launcher_loadRuntimeProperties(const char* configFil /** * @brief Set the global framework instance. - * - * If a global framework instance is already set, the new framework instance will be destroyed and a - * error code will be returned. */ -static celix_status_t celix_launcher_setGlobalFramework(celix_framework_t* fw); +static void 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; @@ -144,42 +153,35 @@ int celix_launcher_launchAndWait(int argc, char* argv[], const char* embeddedCon } else if (options.showProps) { celix_launcher_printProperties(embeddedProps, runtimeProps, options.configFile); return CELIX_LAUNCHER_OK_EXIT_CODE; - } else if (options.createCache) { + } + + if (!celix_launcher_checkFrameworkLaunched()) { + return CELIX_LAUNCHER_ERROR_EXIT_CODE; + } + + if (options.createCache) { status = celix_launcher_createBundleCache(celix_steal_ptr(embeddedProps), runtimeProps); + celix_launcher_resetLauncher(); return status == CELIX_SUCCESS ? CELIX_LAUNCHER_OK_EXIT_CODE : CELIX_LAUNCHER_ERROR_EXIT_CODE; } celix_framework_t* framework = NULL; status = celix_launcher_createFramework(celix_steal_ptr(embeddedProps), runtimeProps, &framework); if (status == CELIX_SUCCESS) { - status = celix_launcher_setGlobalFramework(framework); - if (status != CELIX_SUCCESS) { - return CELIX_LAUNCHER_ERROR_EXIT_CODE; - } + celix_launcher_setGlobalFramework(framework); celix_framework_waitForStop(framework); celix_frameworkFactory_destroyFramework(framework); #ifndef CELIX_NO_CURLINIT // 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; } -static celix_status_t celix_launcher_setGlobalFramework(celix_framework_t* fw) { - //try to set the global framework instance, but fail if global instance is already set. - celix_framework_t* expected = NULL; - bool swapped = __atomic_compare_exchange_n(&g_fw, &expected, fw, false, __ATOMIC_SEQ_CST, __ATOMIC_RELAXED); - if (!swapped) { - celix_bundle_context_t* ctx = celix_framework_getFrameworkContext(fw); - celix_bundleContext_log( - ctx, CELIX_LOG_LEVEL_ERROR, "Failed to set global framework instance, already set. Destroying framework"); - celix_frameworkFactory_destroyFramework(fw); - return CELIX_ILLEGAL_STATE; - } - return CELIX_SUCCESS; -} - static celix_status_t celix_launcher_parseOptions(int argc, char* argv[], celix_launcher_options_t* opts) { // Perform some minimal command-line option parsing... int optCount = 0; @@ -366,8 +368,16 @@ static void celix_launcher_shutdownFrameworkSignalHandler(int signal) { celix_launcher_stopInternal(&signal); } +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); +} + void celix_launcher_stopInternal(const int* signal) { - celix_framework_t* fw = __atomic_exchange_n(&g_fw, NULL, __ATOMIC_SEQ_CST); + 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); @@ -380,6 +390,15 @@ void celix_launcher_stopInternal(const int* signal) { } } -void celix_launcher_triggerStop() { - celix_launcher_stopInternal(NULL); +static bool celix_launcher_checkFrameworkLaunched() { + bool alreadyLaunched = __atomic_exchange_n(&g_framework_launched, true, __ATOMIC_SEQ_CST); + if (alreadyLaunched) { + fprintf(stderr, "Cannot launch framework, already launched\n"); + } + return !alreadyLaunched; +} + +static void celix_launcher_resetLauncher() { + __atomic_store_n(&g_framework_launched, false, __ATOMIC_SEQ_CST); + __atomic_store_n(&g_framework, NULL, __ATOMIC_SEQ_CST); } From 621c2be4b38c068984d8757c50ac9e0944f051d8 Mon Sep 17 00:00:00 2001 From: PengZheng Date: Tue, 11 Jun 2024 16:40:40 +0800 Subject: [PATCH 08/21] gh-685: Improve error handling of launcher. 1. Avoid discarding valid configuration on properties creation error. 2. Fix leak on cache creation error. --- libs/framework/src/celix_launcher.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/libs/framework/src/celix_launcher.c b/libs/framework/src/celix_launcher.c index 35ea7460b..e0bb06c73 100644 --- a/libs/framework/src/celix_launcher.c +++ b/libs/framework/src/celix_launcher.c @@ -86,7 +86,7 @@ static celix_status_t celix_launcher_createFramework(celix_properties_t* embedde celix_framework_t** frameworkOut); /** - * @brief Stop the global framework instance (if set). + * @brief Parse launcher options from the given command line arguments. */ static celix_status_t celix_launcher_parseOptions(int argc, char* argv[], celix_launcher_options_t* opts); @@ -125,14 +125,14 @@ static void 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; if (embeddedConfig) { - celix_status_t status = celix_properties_loadFromString(embeddedConfig, 0, &embeddedProps); - if (status != CELIX_SUCCESS) { - celix_err_printErrors(stderr, "Error creating embedded properties: ", NULL); - return CELIX_LAUNCHER_ERROR_EXIT_CODE; - } + (void)celix_properties_loadFromString(embeddedConfig, 0, &embeddedProps); } else { embeddedProps = celix_properties_create(); } + if (embeddedProps == NULL) { + celix_err_printErrors(stderr, "Error creating embedded properties: ", NULL); + return CELIX_LAUNCHER_ERROR_EXIT_CODE; + } celix_launcher_options_t options; memset(&options, 0, sizeof(options)); @@ -253,7 +253,7 @@ static void celix_launcher_noopSignalHandler(int signal __attribute__((unused))) } static void celix_launcher_printUsage(char* progName) { - printf("Usage:\n %s [-h|-p] [path/to/runtime/config.properties]\n", basename(progName)); + printf("Usage:\n %s [-h|-p|-c] [path/to/runtime/config.properties]\n", basename(progName)); printf("Options:\n"); printf("\t-h | --help: Show this message.\n"); printf("\t-p | --props: Show the embedded and runtime properties for this Celix container and exit.\n"); @@ -323,10 +323,9 @@ static celix_status_t celix_launcher_createBundleCache(celix_properties_t* embed if (status != CELIX_SUCCESS) { celix_bundle_context_t* ctx = celix_framework_getFrameworkContext(fw); celix_bundleContext_log(ctx, CELIX_LOG_LEVEL_ERROR, "Failed to create bundle cache"); - return status; } celix_frameworkFactory_destroyFramework(fw); - return CELIX_SUCCESS; + return status; } celix_status_t celix_launcher_combineProperties(celix_properties_t* embeddedProps, From 8f4fafcdfa4449d31eaa591404a8ad9e88c2b1b5 Mon Sep 17 00:00:00 2001 From: PengZheng Date: Wed, 12 Jun 2024 15:26:51 +0800 Subject: [PATCH 09/21] gh-215: Implement async-signal-safe launcher shutdown. --- .../gtest/src/CelixLauncherTestSuite.cc | 20 +++- .../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 +#include #include #include #include #include +#include #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 #include @@ -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) { From b5214cf9910b597e542ff1022d1d6f306d9da9ab Mon Sep 17 00:00:00 2001 From: PengZheng Date: Wed, 12 Jun 2024 16:19:14 +0800 Subject: [PATCH 10/21] gh-215: Reset launcher on error. --- libs/framework/src/celix_launcher.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libs/framework/src/celix_launcher.c b/libs/framework/src/celix_launcher.c index e706c4490..9c94d76d7 100644 --- a/libs/framework/src/celix_launcher.c +++ b/libs/framework/src/celix_launcher.c @@ -178,6 +178,7 @@ 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) { + celix_launcher_resetLauncher(); return CELIX_LAUNCHER_ERROR_EXIT_CODE; } status = celix_launcher_setGlobalFramework(framework); @@ -255,12 +256,14 @@ static celix_status_t celix_launcher_createFramework(celix_properties_t* embedde return *frameworkOut != NULL ? CELIX_SUCCESS : CELIX_FRAMEWORK_EXCEPTION; } +// LCOV_EXCL_START /** * @brief SIGUSR1 SIGUSR2 no-op callback */ static void celix_launcher_noopSignalHandler(int signal __attribute__((unused))) { // ignoring signal SIGUSR1, SIGUSR2. Can be used on threads } +// LCOV_EXCL_STOP static void celix_launcher_printUsage(char* progName) { printf("Usage:\n %s [-h|-p|-c] [path/to/runtime/config.properties]\n", basename(progName)); From e07b2e812d2e3dc812104f0e64baf6ca0744131d Mon Sep 17 00:00:00 2001 From: PengZheng Date: Wed, 12 Jun 2024 17:14:49 +0800 Subject: [PATCH 11/21] gh-215: Remove all remaining scheduled events for an inactive dispatcher. --- libs/framework/src/framework.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libs/framework/src/framework.c b/libs/framework/src/framework.c index e3b0431df..0c73f8afb 100644 --- a/libs/framework/src/framework.c +++ b/libs/framework/src/framework.c @@ -1383,7 +1383,7 @@ static void celix_framework_processScheduledEvents(celix_framework_t* fw) { celixThreadMutex_lock(&fw->dispatcher.mutex); CELIX_LONG_HASH_MAP_ITERATE(fw->dispatcher.scheduledEvents, entry) { celix_scheduled_event_t* visit = entry.value.ptrValue; - if (celix_scheduledEvent_isMarkedForRemoval(visit)) { + if (!fw->dispatcher.active || celix_scheduledEvent_isMarkedForRemoval(visit)) { removeEvent = visit; celix_longHashMap_remove(fw->dispatcher.scheduledEvents, celix_scheduledEvent_getId(visit)); break; @@ -1536,6 +1536,7 @@ static void *fw_eventDispatcher(void *fw) { celixThreadMutex_unlock(&framework->dispatcher.mutex); while (needExtraRun) { fw_handleEvents(framework); + celix_framework_processScheduledEvents(framework); celixThreadMutex_lock(&framework->dispatcher.mutex); needExtraRun = celix_framework_eventQueueSize(fw) > 0; celixThreadMutex_unlock(&framework->dispatcher.mutex); From d384c8653570e54be521b24ec0902950288201bf Mon Sep 17 00:00:00 2001 From: PengZheng Date: Wed, 12 Jun 2024 17:32:27 +0800 Subject: [PATCH 12/21] gh-215: Remove all remaining scheduled events for an inactive dispatcher. --- libs/framework/src/framework.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/framework/src/framework.c b/libs/framework/src/framework.c index 0c73f8afb..0b317c91a 100644 --- a/libs/framework/src/framework.c +++ b/libs/framework/src/framework.c @@ -1531,12 +1531,12 @@ static void *fw_eventDispatcher(void *fw) { } //not active anymore, extra runs for possible request leftovers + celix_framework_processScheduledEvents(framework); celixThreadMutex_lock(&framework->dispatcher.mutex); bool needExtraRun = celix_framework_eventQueueSize(fw) > 0; celixThreadMutex_unlock(&framework->dispatcher.mutex); while (needExtraRun) { fw_handleEvents(framework); - celix_framework_processScheduledEvents(framework); celixThreadMutex_lock(&framework->dispatcher.mutex); needExtraRun = celix_framework_eventQueueSize(fw) > 0; celixThreadMutex_unlock(&framework->dispatcher.mutex); From 61d0f6900346d08409cb8808102f91649a2803dc Mon Sep 17 00:00:00 2001 From: PengZheng Date: Sun, 16 Jun 2024 11:12:39 +0800 Subject: [PATCH 13/21] gh-215: Minor documentation improvement. --- libs/framework/src/celix_launcher.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/libs/framework/src/celix_launcher.c b/libs/framework/src/celix_launcher.c index 9c94d76d7..edf04d412 100644 --- a/libs/framework/src/celix_launcher.c +++ b/libs/framework/src/celix_launcher.c @@ -49,8 +49,8 @@ typedef struct { framework_t* framework; long shutdownEventId; bool launched; - bool shutdown; - int signal; + bool shutdown; // accessed through atomic operations + int signal; // accessed through atomic operations } celix_launcher_t; typedef struct { @@ -393,7 +393,6 @@ void celix_launcher_triggerStop() { } 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); From 2378bac0e31fbd346492123e9dd43bb2855960f4 Mon Sep 17 00:00:00 2001 From: PengZheng Date: Sun, 16 Jun 2024 12:34:59 +0800 Subject: [PATCH 14/21] [CID 349739]: Release a lock guard manually. --- libs/framework/src/bundle_context.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/framework/src/bundle_context.c b/libs/framework/src/bundle_context.c index 261e7b36b..4f126101d 100644 --- a/libs/framework/src/bundle_context.c +++ b/libs/framework/src/bundle_context.c @@ -579,7 +579,7 @@ celix_dependency_manager_t* celix_bundleContext_getDependencyManager(bundle_cont if (ctx->mng) { return ctx->mng; } - celixThreadRwlock_unlock(celix_steal_ptr(rlockGuard.lock)); + celixRwlockRlockGuard_deinit(&rlockGuard); celix_auto(celix_rwlock_wlock_guard_t) wlockGuard = celixRwlockWlockGuard_init(&ctx->lock); if (ctx->mng == NULL) { From 049efdd541fc6122a72c287d91e43b4f25dfdb3b Mon Sep 17 00:00:00 2001 From: PengZheng Date: Sun, 23 Jun 2024 12:29:26 +0800 Subject: [PATCH 15/21] [CID 392574]: Destroy held lock. --- bundles/shell/remote_shell/src/shell_mediator.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/bundles/shell/remote_shell/src/shell_mediator.c b/bundles/shell/remote_shell/src/shell_mediator.c index 636296031..5ad3e6212 100644 --- a/bundles/shell/remote_shell/src/shell_mediator.c +++ b/bundles/shell/remote_shell/src/shell_mediator.c @@ -85,8 +85,6 @@ celix_status_t shellMediator_stop(shell_mediator_pt instance) { celix_status_t shellMediator_destroy(shell_mediator_pt instance) { celix_status_t status = CELIX_SUCCESS; - celixThreadMutex_lock(&instance->mutex); - instance->shellService = NULL; serviceTracker_destroy(instance->tracker); celix_logHelper_destroy(instance->loghelper); From 63445a3fa18109b20f8168c66a2ce536a6dbcd55 Mon Sep 17 00:00:00 2001 From: PengZheng Date: Sun, 23 Jun 2024 18:13:37 +0800 Subject: [PATCH 16/21] Avoid schedule event to an inactive framework. --- .../gtest/src/ScheduledEventTestSuite.cc | 23 +++++++++++++++++++ libs/framework/src/framework.c | 9 ++++++-- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/libs/framework/gtest/src/ScheduledEventTestSuite.cc b/libs/framework/gtest/src/ScheduledEventTestSuite.cc index 9edbbb282..f2dd53f55 100644 --- a/libs/framework/gtest/src/ScheduledEventTestSuite.cc +++ b/libs/framework/gtest/src/ScheduledEventTestSuite.cc @@ -21,6 +21,7 @@ #include "celix/FrameworkFactory.h" #include "celix_bundle_context.h" +#include "celix_framework.h" #include "celix_scheduled_event.h" class ScheduledEventTestSuite : public ::testing::Test { @@ -761,3 +762,25 @@ TEST_F(ScheduledEventTestSuite, ScheduledEventTimeoutLogTest) { EXPECT_GE(logCount.load(), 2); } #endif + +TEST_F(ScheduledEventTestSuite, ScheduledEventForInvactiveFramework) { + // Given a framework that is stopped + celix_framework_stopBundle(fw->getCFramework(), CELIX_FRAMEWORK_BUNDLE_ID); + celix_framework_waitForStop(fw->getCFramework()); + // When a scheduled event is added + std::atomic count{0}; + auto callback = [](void* data) { + auto* count = static_cast*>(data); + count->fetch_add(1); + }; + + celix_scheduled_event_options_t opts{}; + opts.initialDelayInSeconds = 0.01; + opts.callbackData = &count; + opts.callback = callback; + long eventId = celix_bundleContext_scheduleEvent(fw->getFrameworkBundleContext()->getCBundleContext(), &opts); + EXPECT_LT(eventId, 0); + + // Then the event is not added + EXPECT_EQ(0, count.load()); +} diff --git a/libs/framework/src/framework.c b/libs/framework/src/framework.c index 0b317c91a..4706a4ae6 100644 --- a/libs/framework/src/framework.c +++ b/libs/framework/src/framework.c @@ -2583,8 +2583,13 @@ long celix_framework_scheduleEvent(celix_framework_t* fw, celix_bundleEntry_decreaseUseCount(bndEntry); celixThreadMutex_lock(&fw->dispatcher.mutex); - celix_longHashMap_put(fw->dispatcher.scheduledEvents, id, event); - celixThreadCondition_broadcast(&fw->dispatcher.cond); //notify dispatcher thread for newly added scheduled event + if (fw->dispatcher.active) { + celix_longHashMap_put(fw->dispatcher.scheduledEvents, id, event); + celixThreadCondition_broadcast(&fw->dispatcher.cond); //notify dispatcher thread for newly added scheduled event + } else { + celix_scheduledEvent_release(event); + id = -1L; + } celixThreadMutex_unlock(&fw->dispatcher.mutex); return id; From 5712dabe2269f631c99d772487ccee556e6c5fa8 Mon Sep 17 00:00:00 2001 From: PengZheng Date: Mon, 24 Jun 2024 17:56:09 +0800 Subject: [PATCH 17/21] Fix test_package error. --- libs/framework/gtest/src/CelixLauncherTestSuite.cc | 9 --------- libs/framework/src/celix_launcher.c | 7 +++++-- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/libs/framework/gtest/src/CelixLauncherTestSuite.cc b/libs/framework/gtest/src/CelixLauncherTestSuite.cc index d422d577c..fa1d7fc8b 100644 --- a/libs/framework/gtest/src/CelixLauncherTestSuite.cc +++ b/libs/framework/gtest/src/CelixLauncherTestSuite.cc @@ -192,15 +192,6 @@ 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); } diff --git a/libs/framework/src/celix_launcher.c b/libs/framework/src/celix_launcher.c index edf04d412..34abbfdb2 100644 --- a/libs/framework/src/celix_launcher.c +++ b/libs/framework/src/celix_launcher.c @@ -182,9 +182,12 @@ int celix_launcher_launchAndWait(int argc, char* argv[], const char* embeddedCon return CELIX_LAUNCHER_ERROR_EXIT_CODE; } status = celix_launcher_setGlobalFramework(framework); - if (status == CELIX_SUCCESS) { - celix_framework_waitForStop(framework); + if (status != CELIX_SUCCESS) { + celix_bundleContext_log(celix_framework_getFrameworkContext(framework), CELIX_LOG_LEVEL_WARNING, + "Failed to schedule celix_shutdown_check"); + status = CELIX_SUCCESS; } + celix_framework_waitForStop(framework); celix_launcher_resetLauncher(); #ifndef CELIX_NO_CURLINIT // Cleanup Curl From 1606b4993d3aa090b25312b7c269976786fa9b6f Mon Sep 17 00:00:00 2001 From: PengZheng Date: Mon, 24 Jun 2024 16:25:54 +0800 Subject: [PATCH 18/21] Update to macOS-12. --- .github/workflows/conan_create.yml | 2 +- .github/workflows/macos.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/conan_create.yml b/.github/workflows/conan_create.yml index 575cab429..8754f9d1f 100644 --- a/.github/workflows/conan_create.yml +++ b/.github/workflows/conan_create.yml @@ -76,7 +76,7 @@ jobs: conan remove -c celix/* mac-build: - runs-on: macOS-11 + runs-on: macOS-12 timeout-minutes: 120 steps: - name: Checkout source code diff --git a/.github/workflows/macos.yml b/.github/workflows/macos.yml index 54872e504..84814a086 100644 --- a/.github/workflows/macos.yml +++ b/.github/workflows/macos.yml @@ -16,7 +16,7 @@ env: jobs: macos-build-conan: - runs-on: macOS-11 + runs-on: macOS-12 timeout-minutes: 120 steps: - name: Checkout source code From f82223d6eac01c118c62e5ae53eac814748342aa Mon Sep 17 00:00:00 2001 From: PengZheng Date: Mon, 24 Jun 2024 19:21:58 +0800 Subject: [PATCH 19/21] Add test for failure to schedule the celix_shutdown_check event. --- libs/framework/gtest/CMakeLists.txt | 6 +++- .../gtest/src/CelixLauncherTestSuite.cc | 13 +++++++ libs/framework/gtest/src/activator_stop.c | 36 +++++++++++++++++++ libs/framework/src/celix_launcher.c | 3 +- 4 files changed, 55 insertions(+), 3 deletions(-) create mode 100644 libs/framework/gtest/src/activator_stop.c diff --git a/libs/framework/gtest/CMakeLists.txt b/libs/framework/gtest/CMakeLists.txt index a6184dada..3efab0dac 100644 --- a/libs/framework/gtest/CMakeLists.txt +++ b/libs/framework/gtest/CMakeLists.txt @@ -34,6 +34,7 @@ add_celix_bundle(cmp_test_bundle SOURCES src/CmpTestBundleActivator.cc VERSION 1 add_celix_bundle(cond_test_bundle SOURCES src/CondTestBundleActivator.cc VERSION 1.0.0) add_subdirectory(subdir) #simple_test_bundle4, simple_test_bundle5 and sublib add_celix_bundle(celix_err_test_bundle SOURCES src/activator_with_celix_err.c VERSION 1.0.0) +add_celix_bundle(immediate_stop_bundle SOURCES src/activator_stop.c VERSION 1.0.0) add_celix_bundle(unresolvable_bundle SOURCES src/nop_activator.c VERSION 1.0.0) if (CMAKE_BUILD_TYPE STREQUAL "Debug") @@ -82,7 +83,7 @@ add_celix_bundle_dependencies(test_framework simple_test_bundle2 simple_test_bundle3 simple_test_bundle4 simple_test_bundle5 bundle_with_exception bundle_with_bad_export unresolvable_bundle simple_cxx_bundle simple_cxx_dep_man_bundle cmp_test_bundle - celix_err_test_bundle dup_symbolic_name_bundle) + celix_err_test_bundle dup_symbolic_name_bundle immediate_stop_bundle) target_include_directories(test_framework PRIVATE ../src) celix_deprecated_utils_headers(test_framework) @@ -94,6 +95,7 @@ celix_get_bundle_file(simple_test_bundle4 SIMPLE_TEST_BUNDLE4) celix_get_bundle_file(simple_test_bundle5 SIMPLE_TEST_BUNDLE5) celix_get_bundle_filename(simple_test_bundle4 SIMPLE_TEST_BUNDLE4_FILENAME) celix_get_bundle_filename(simple_test_bundle5 SIMPLE_TEST_BUNDLE5_FILENAME) +celix_get_bundle_file(immediate_stop_bundle IMMEDIATE_STOP_BUNDLE) celix_get_bundle_filename(bundle_with_exception BUNDLE_WITH_EXCEPTION) celix_get_bundle_file(bundle_with_bad_export BUNDLE_WITH_BAD_EXPORT) @@ -128,7 +130,9 @@ target_compile_definitions(test_framework PRIVATE CMP_TEST_BUNDLE_LOC="${CMP_TEST_BUNDLE_LOC}" COND_TEST_BUNDLE_LOC="${COND_TEST_BUNDLE_LOC}" INSTALL_AND_START_BUNDLES_CONFIG_PROPERTIES_FILE="${CMAKE_CURRENT_BINARY_DIR}/install_and_start_bundles.properties" + IMMEDIATE_STOP_BUNDLE_LOCATION="${IMMEDIATE_STOP_BUNDLE}" ) + if (ENABLE_TESTING_ON_CI) target_compile_definitions(test_framework PRIVATE TESTING_ON_CI=1) endif () diff --git a/libs/framework/gtest/src/CelixLauncherTestSuite.cc b/libs/framework/gtest/src/CelixLauncherTestSuite.cc index fa1d7fc8b..4d3a69c81 100644 --- a/libs/framework/gtest/src/CelixLauncherTestSuite.cc +++ b/libs/framework/gtest/src/CelixLauncherTestSuite.cc @@ -194,6 +194,19 @@ TEST_F(CelixLauncherTestSuite, LaunchWithInvalidConfigPropertiesTest) { EXPECT_EQ(status, std::future_status::ready); } +TEST_F(CelixLauncherTestSuite, LaunchWithInvalidConfigProperties2Test) { + + //When launching the framework with a properties set with a negative shutdown period + auto* props = celix_properties_create(); + ASSERT_TRUE(props != nullptr); + celix_properties_set(props, CELIX_AUTO_START_0, IMMEDIATE_STOP_BUNDLE_LOCATION); + celix_properties_setDouble(props, CELIX_LAUNCHER_SHUTDOWN_PERIOD_IN_SECONDS, -1.0); + auto future = launchInThread({"programName"}, props, 0); + //Then launch will exit + auto 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(); diff --git a/libs/framework/gtest/src/activator_stop.c b/libs/framework/gtest/src/activator_stop.c new file mode 100644 index 000000000..2842e03bb --- /dev/null +++ b/libs/framework/gtest/src/activator_stop.c @@ -0,0 +1,36 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "celix_bundle_activator.h" +#include "celix_framework.h" + +typedef struct bundle_activator { + //empty +} bundle_activator_t; + +static celix_status_t act_start(bundle_activator_t *act CELIX_UNUSED, celix_bundle_context_t *ctx CELIX_UNUSED) { + celix_framework_stopBundleAsync(celix_bundleContext_getFramework(ctx), CELIX_FRAMEWORK_BUNDLE_ID); // to make container quit immediately + return CELIX_SUCCESS; +} + +static celix_status_t act_stop(bundle_activator_t *act CELIX_UNUSED, celix_bundle_context_t *ctx CELIX_UNUSED) { + return CELIX_SUCCESS; +} + +CELIX_GEN_BUNDLE_ACTIVATOR(bundle_activator_t, act_start, act_stop); diff --git a/libs/framework/src/celix_launcher.c b/libs/framework/src/celix_launcher.c index 34abbfdb2..ab6832943 100644 --- a/libs/framework/src/celix_launcher.c +++ b/libs/framework/src/celix_launcher.c @@ -185,7 +185,6 @@ int celix_launcher_launchAndWait(int argc, char* argv[], const char* embeddedCon if (status != CELIX_SUCCESS) { celix_bundleContext_log(celix_framework_getFrameworkContext(framework), CELIX_LOG_LEVEL_WARNING, "Failed to schedule celix_shutdown_check"); - status = CELIX_SUCCESS; } celix_framework_waitForStop(framework); celix_launcher_resetLauncher(); @@ -193,7 +192,7 @@ int celix_launcher_launchAndWait(int argc, char* argv[], const char* embeddedCon // Cleanup Curl curl_global_cleanup(); #endif - return status == CELIX_SUCCESS ? CELIX_LAUNCHER_OK_EXIT_CODE : CELIX_LAUNCHER_ERROR_EXIT_CODE; + return CELIX_LAUNCHER_OK_EXIT_CODE; } static celix_status_t celix_launcher_parseOptions(int argc, char* argv[], celix_launcher_options_t* opts) { From bfccf0e44b7f328afbf9a2ce9484d55688da2486 Mon Sep 17 00:00:00 2001 From: PengZheng Date: Thu, 11 Jul 2024 10:14:08 +0800 Subject: [PATCH 20/21] Update linux-build-apt to use ninja. --- .github/workflows/ubuntu.yml | 4 +++- CMakeLists.txt | 5 ----- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ubuntu.yml b/.github/workflows/ubuntu.yml index d5b87a93f..6a1079536 100644 --- a/.github/workflows/ubuntu.yml +++ b/.github/workflows/ubuntu.yml @@ -110,6 +110,7 @@ jobs: sudo apt-get update sudo apt-get install -yq --no-install-recommends \ build-essential \ + ninja-build \ curl \ uuid-dev \ libzip-dev \ @@ -145,11 +146,12 @@ jobs: -DENABLE_TESTING_ON_CI=ON -DCMAKE_BUILD_TYPE=${{ matrix.type }} -DENABLE_CCACHE=ON + -G Ninja run: | mkdir build install cd build cmake ${BUILD_OPTIONS} -DCMAKE_INSTALL_PREFIX=../install .. - make -j $(nproc) && make install + ninja && ninja install - name: Test run: | cd $GITHUB_WORKSPACE/build diff --git a/CMakeLists.txt b/CMakeLists.txt index f0aa5aa5c..d72ce4d76 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -31,11 +31,6 @@ list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake") set(THREADS_PREFER_PTHREAD_FLAG ON) find_package(Threads REQUIRED) -# see https://public.kitware.com/Bug/view.php?id=15696 -IF (${CMAKE_MAJOR_VERSION}.${CMAKE_MINOR_VERSION} EQUAL 3.3 AND ${CMAKE_GENERATOR} STREQUAL "Unix Makefiles") - message( FATAL_ERROR "Building Celix using CMake 3.3 and makefiles is not supported due to a bug in the Makefile Generator (see Bug 15696). Please change the used CMake version - both, CMake 3.2 and CMake 3.4 are working fine. Or use a different generator (e.g. Ninja)." ) -ENDIF() - # Options option(ENABLE_TESTING "Enables unit/bundle testing" FALSE) if (ENABLE_TESTING) From a62656d8d181faed36963dc3279bb7a2faa82f37 Mon Sep 17 00:00:00 2001 From: PengZheng Date: Wed, 17 Jul 2024 19:39:12 +0800 Subject: [PATCH 21/21] Fix header includes. --- .../remote_service_admin_shm_v2/rsa_shm/src/rsa_shm_client.c | 1 + .../remote_service_admin_shm_v2/rsa_shm/src/rsa_shm_server.c | 1 + libs/framework/src/celix_bundle_cache.c | 2 +- 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/bundles/remote_services/remote_service_admin_shm_v2/rsa_shm/src/rsa_shm_client.c b/bundles/remote_services/remote_service_admin_shm_v2/rsa_shm/src/rsa_shm_client.c index 70ffba0b7..2b304dcc7 100644 --- a/bundles/remote_services/remote_service_admin_shm_v2/rsa_shm/src/rsa_shm_client.c +++ b/bundles/remote_services/remote_service_admin_shm_v2/rsa_shm/src/rsa_shm_client.c @@ -36,6 +36,7 @@ #include #include #include +#include #include diff --git a/bundles/remote_services/remote_service_admin_shm_v2/rsa_shm/src/rsa_shm_server.c b/bundles/remote_services/remote_service_admin_shm_v2/rsa_shm/src/rsa_shm_server.c index aaebbb368..6e40962b7 100644 --- a/bundles/remote_services/remote_service_admin_shm_v2/rsa_shm/src/rsa_shm_server.c +++ b/bundles/remote_services/remote_service_admin_shm_v2/rsa_shm/src/rsa_shm_server.c @@ -36,6 +36,7 @@ #include #include #include +#include #include #define MAX_RSA_SHM_SERVER_HANDLE_MSG_THREADS_NUM 5 diff --git a/libs/framework/src/celix_bundle_cache.c b/libs/framework/src/celix_bundle_cache.c index cb0260c64..d401ac432 100644 --- a/libs/framework/src/celix_bundle_cache.c +++ b/libs/framework/src/celix_bundle_cache.c @@ -23,7 +23,7 @@ #include #include #include -#include +#include #include "celix_constants.h" #include "celix_log.h"