This is an automated email from the ASF dual-hosted git repository. pnoltes pushed a commit to branch feature/685-update-container-config-properties-usage in repository https://gitbox.apache.org/repos/asf/celix.git
commit 0a0709a2d65d98e34ce6a8730bbc1333d21f3759 Author: Pepijn Noltes <[email protected]> AuthorDate: Thu May 30 18:30:12 2024 +0200 gh-685: Improve error handling for properties save/load --- documents/containers.md | 44 +++-- .../BundleArchiveWithErrorInjectionTestSuite.cc | 13 +- .../src/CelixBundleCacheErrorInjectionTestSuite.cc | 17 +- .../gtest/src/CelixBundleCacheTestSuite.cc | 23 ++- libs/framework/src/bundle_archive.c | 2 - libs/framework/src/celix_bundle_cache.c | 36 ++-- libs/framework/src/celix_bundle_cache.h | 10 +- libs/framework/src/celix_log.c | 4 +- libs/framework/src/framework.c | 199 +++++++++++---------- .../gtest/src/CelixUtilsAutoCleanupTestSuite.cc | 4 + libs/utils/include/celix_stdio_cleanup.h | 3 + libs/utils/include/celix_threads.h | 1 + 12 files changed, 201 insertions(+), 155 deletions(-) diff --git a/documents/containers.md b/documents/containers.md index 77e673e89..37e366b37 100644 --- a/documents/containers.md +++ b/documents/containers.md @@ -45,12 +45,21 @@ will create the following main source file (note: reformatted for display purpos ```C++ //${CMAKE_BINARY_DIR}/celix/gen/containers/my_empty_container/main.cc #include <celix_launcher.h> +#include <celix_err.h> +#define CELIX_MULTI_LINE_STRING(...) #__VA_ARGS__ int main(int argc, char *argv[]) { - const char * config = "\ -CELIX_CONTAINER_NAME=my_empty_container\n\ -CELIX_BUNDLES_PATH=bundles\n\ -"; - celix_properties_t *embeddedProps = celix_properties_loadFromString(config); + const char * config = CELIX_MULTI_LINE_STRING( +{ + "CELIX_BUNDLES_PATH":"bundles", + "CELIX_CONTAINER_NAME":"my_empty_container" +}); + + celix_properties_t *embeddedProps; + celix_status_t status = celix_properties_loadFromString2(config, 0, &embeddedProps); + if (status != CELIX_SUCCESS) { + celix_err_printErrors(stderr, "Error creating embedded properties.", NULL); + return -1; + } return celixLauncher_launchAndWaitForShutdown(argc, argv, embeddedProps); } ``` @@ -84,15 +93,24 @@ add_celix_container(my_web_shell_container will create the following main source file (note: reformatted for display purpose): ```C++ #include <celix_launcher.h> +#include <celix_err.h> +#define CELIX_MULTI_LINE_STRING(...) #__VA_ARGS__ int main(int argc, char *argv[]) { - const char * config = "\ -CELIX_CONTAINER_NAME=my_web_shell_container\n\ -CELIX_BUNDLES_PATH=bundles\n\ -CELIX_AUTO_START_3=celix_http_admin-Debug.zip celix_shell-Debug.zip celix_shell_wui-Debug.zip\n\ -CELIX_LOGGING_DEFAULT_ACTIVE_LOG_LEVEL=debug\n\ -CELIX_HTTP_ADMIN_LISTENING_PORTS=8888"; - - celix_properties_t *embeddedProps = celix_properties_loadFromString(config); + const char * config = CELIX_MULTI_LINE_STRING( +{ + "CELIX_AUTO_START_3":"celix_http_admin-Debug.zip celix_shell-Debug.zip celix_shell_wui-Debug.zip", + "CELIX_LOGGING_DEFAULT_ACTIVE_LOG_LEVEL":"debug", + "CELIX_HTTP_ADMIN_LISTENING_PORTS":"8888", + "CELIX_BUNDLES_PATH":"bundles", + "CELIX_CONTAINER_NAME":"my_web_shell_container" +}); + + celix_properties_t *embeddedProps; + celix_status_t status = celix_properties_loadFromString2(config, 0, &embeddedProps); + if (status != CELIX_SUCCESS) { + celix_err_printErrors(stderr, "Error creating embedded properties.", NULL); + return -1; + } return celixLauncher_launchAndWaitForShutdown(argc, argv, embeddedProps); } ``` diff --git a/libs/framework/gtest/src/BundleArchiveWithErrorInjectionTestSuite.cc b/libs/framework/gtest/src/BundleArchiveWithErrorInjectionTestSuite.cc index b7d1e5568..2aeb425c3 100644 --- a/libs/framework/gtest/src/BundleArchiveWithErrorInjectionTestSuite.cc +++ b/libs/framework/gtest/src/BundleArchiveWithErrorInjectionTestSuite.cc @@ -268,18 +268,9 @@ TEST_F(CelixBundleArchiveErrorInjectionTestSuite, StoreBundleStatePropertiesErro auto ctx = fw->getFrameworkBundleContext(); // When an error is prepped for celix_properties_save - celix_ei_expect_celix_properties_save((void*)celix_bundleCache_findBundleIdForLocation, 1, CELIX_FILE_IO_EXCEPTION); + celix_ei_expect_celix_properties_save((void*)celix_bundleArchive_create, 1, CELIX_FILE_IO_EXCEPTION); // When the bundle install, a bundle id is returned (async bundle install) long bndId = ctx->installBundle(SIMPLE_TEST_BUNDLE1_LOCATION); - EXPECT_GE(bndId, 0); - - // Then the bundle is not successfully installed - - celix_bundleContext_useBundle( - ctx->getCBundleContext(), bndId, nullptr, [](void* /*handle*/, const celix_bundle_t* bnd) { - auto status = celix_bundle_getState(bnd); - // TODO fixme, bundle is installed and active, this is not correct - EXPECT_EQ(CELIX_BUNDLE_EVENT_INSTALLED, status); - }); + EXPECT_LT(bndId, 0); } diff --git a/libs/framework/gtest/src/CelixBundleCacheErrorInjectionTestSuite.cc b/libs/framework/gtest/src/CelixBundleCacheErrorInjectionTestSuite.cc index 732119441..84b8a0464 100644 --- a/libs/framework/gtest/src/CelixBundleCacheErrorInjectionTestSuite.cc +++ b/libs/framework/gtest/src/CelixBundleCacheErrorInjectionTestSuite.cc @@ -110,13 +110,18 @@ TEST_F(CelixBundleCacheErrorInjectionTestSuite, ArchiveCreateErrorTest) { celix_ei_expect_celix_utils_writeOrCreateString((void*)celix_bundleCache_createArchive, 0, nullptr); EXPECT_EQ(CELIX_ENOMEM, celix_bundleCache_createArchive(cache, 1, SIMPLE_TEST_BUNDLE1_LOCATION, &archive)); EXPECT_EQ(nullptr, archive); - EXPECT_EQ(-1, celix_bundleCache_findBundleIdForLocation(cache, SIMPLE_TEST_BUNDLE1_LOCATION)); + long bndId; + auto status = celix_bundleCache_findBundleIdForLocation(cache, SIMPLE_TEST_BUNDLE1_LOCATION, &bndId); + EXPECT_EQ(CELIX_SUCCESS, status); + EXPECT_EQ(-1, bndId); EXPECT_FALSE(celix_bundleCache_isBundleIdAlreadyUsed(cache, 1)); celix_ei_expect_calloc((void*)celix_bundleArchive_create, 0, nullptr); EXPECT_EQ(CELIX_ENOMEM, celix_bundleCache_createArchive(cache, 1, SIMPLE_TEST_BUNDLE1_LOCATION, &archive)); EXPECT_EQ(nullptr, archive); - EXPECT_EQ(-1, celix_bundleCache_findBundleIdForLocation(cache, SIMPLE_TEST_BUNDLE1_LOCATION)); + status = celix_bundleCache_findBundleIdForLocation(cache, SIMPLE_TEST_BUNDLE1_LOCATION, &bndId); + EXPECT_EQ(CELIX_SUCCESS, status); + EXPECT_EQ(-1, bndId); EXPECT_FALSE(celix_bundleCache_isBundleIdAlreadyUsed(cache, 1)); EXPECT_EQ(CELIX_SUCCESS, celix_bundleCache_destroy(cache)); @@ -196,11 +201,5 @@ TEST_F(CelixBundleCacheErrorInjectionTestSuite, LoadBundleStatePropertiesErrorTe // Then installing the bundle will fail bndId = ctx->installBundle(SIMPLE_TEST_BUNDLE1_LOCATION); - EXPECT_GT(bndId, -1); //async install, so bundle id is returned - celix_bundleContext_useBundle( - ctx->getCBundleContext(), bndId, nullptr, [](void* /*handle*/, const celix_bundle_t* bnd) { - auto status = celix_bundle_getState(bnd); - // TODO fixme, bundle is installed and active, this is not correct - EXPECT_EQ(CELIX_BUNDLE_EVENT_INSTALLED, status); - }); + EXPECT_EQ(bndId, -1); //async install, so bundle id is returned } diff --git a/libs/framework/gtest/src/CelixBundleCacheTestSuite.cc b/libs/framework/gtest/src/CelixBundleCacheTestSuite.cc index 42e397c53..a9393adfd 100644 --- a/libs/framework/gtest/src/CelixBundleCacheTestSuite.cc +++ b/libs/framework/gtest/src/CelixBundleCacheTestSuite.cc @@ -54,13 +54,18 @@ TEST_F(CelixBundleCacheTestSuite, ArchiveCreateDestroyTest) { auto location = celix_bundleArchive_getLocation(archive); EXPECT_STREQ(SIMPLE_TEST_BUNDLE1_LOCATION, location); free(location); - EXPECT_EQ(1, celix_bundleCache_findBundleIdForLocation(fw.cache, SIMPLE_TEST_BUNDLE1_LOCATION)); + long bndId; + auto status = celix_bundleCache_findBundleIdForLocation(fw.cache, SIMPLE_TEST_BUNDLE1_LOCATION, &bndId); + EXPECT_EQ(CELIX_SUCCESS, status); + EXPECT_EQ(1, bndId); EXPECT_TRUE(celix_bundleCache_isBundleIdAlreadyUsed(fw.cache, 1)); std::string loc = celix_bundleArchive_getPersistentStoreRoot(archive); EXPECT_TRUE(celix_utils_directoryExists(loc.c_str())); celix_bundleArchive_invalidate(archive); celix_bundleCache_destroyArchive(fw.cache, archive); - EXPECT_EQ(-1, celix_bundleCache_findBundleIdForLocation(fw.cache, SIMPLE_TEST_BUNDLE1_LOCATION)); + status = celix_bundleCache_findBundleIdForLocation(fw.cache, SIMPLE_TEST_BUNDLE1_LOCATION, &bndId); + EXPECT_EQ(CELIX_SUCCESS, status); + EXPECT_EQ(-1, bndId); EXPECT_FALSE(celix_bundleCache_isBundleIdAlreadyUsed(fw.cache, 1)); EXPECT_FALSE(celix_utils_directoryExists(loc.c_str())); } @@ -72,7 +77,10 @@ TEST_F(CelixBundleCacheTestSuite, NonPermanentDestroyTest) { std::string loc = celix_bundleArchive_getPersistentStoreRoot(archive); EXPECT_TRUE(celix_utils_directoryExists(loc.c_str())); celix_bundleCache_destroyArchive(fw.cache, archive); - EXPECT_EQ(1, celix_bundleCache_findBundleIdForLocation(fw.cache, SIMPLE_TEST_BUNDLE1_LOCATION)); + long bndId; + auto status = celix_bundleCache_findBundleIdForLocation(fw.cache, SIMPLE_TEST_BUNDLE1_LOCATION, &bndId); + EXPECT_EQ(CELIX_SUCCESS, status); + EXPECT_EQ(1, bndId); EXPECT_TRUE(celix_bundleCache_isBundleIdAlreadyUsed(fw.cache, 1)); EXPECT_TRUE(celix_utils_directoryExists(loc.c_str())); } @@ -94,8 +102,13 @@ TEST_F(CelixBundleCacheTestSuite, CreateBundleArchivesCacheTest) { celix_properties_set(fw.configurationMap, CELIX_AUTO_START_1, SIMPLE_TEST_BUNDLE1_LOCATION); celix_properties_set(fw.configurationMap, CELIX_AUTO_INSTALL, SIMPLE_TEST_BUNDLE2_LOCATION); EXPECT_EQ(CELIX_SUCCESS, celix_bundleCache_createBundleArchivesCache(&fw, true)); - EXPECT_EQ(1, celix_bundleCache_findBundleIdForLocation(fw.cache, SIMPLE_TEST_BUNDLE1_LOCATION)); + long bndId; + auto status = celix_bundleCache_findBundleIdForLocation(fw.cache, SIMPLE_TEST_BUNDLE1_LOCATION, &bndId); + EXPECT_EQ(CELIX_SUCCESS, status); + EXPECT_EQ(1, bndId); EXPECT_TRUE(celix_bundleCache_isBundleIdAlreadyUsed(fw.cache, 1)); - EXPECT_EQ(2, celix_bundleCache_findBundleIdForLocation(fw.cache, SIMPLE_TEST_BUNDLE2_LOCATION)); + status = celix_bundleCache_findBundleIdForLocation(fw.cache, SIMPLE_TEST_BUNDLE2_LOCATION, &bndId); + EXPECT_EQ(CELIX_SUCCESS, status); + EXPECT_EQ(2, bndId); EXPECT_TRUE(celix_bundleCache_isBundleIdAlreadyUsed(fw.cache, 2)); } \ No newline at end of file diff --git a/libs/framework/src/bundle_archive.c b/libs/framework/src/bundle_archive.c index b3cdd5e83..4607a7b62 100644 --- a/libs/framework/src/bundle_archive.c +++ b/libs/framework/src/bundle_archive.c @@ -18,7 +18,6 @@ */ #include <assert.h> -#include <dirent.h> #include <errno.h> #include <stdio.h> #include <stdlib.h> @@ -94,7 +93,6 @@ static celix_status_t celix_bundleArchive_storeBundleStateProperties(bundle_arch status = celix_properties_save( bundleStateProperties, archive->savedBundleStatePropertiesPath, CELIX_PROPERTIES_ENCODE_PRETTY); if (status != CELIX_SUCCESS) { - celix_framework_logTssErrors(archive->fw->logger, CELIX_LOG_LEVEL_ERROR); return status; } } diff --git a/libs/framework/src/celix_bundle_cache.c b/libs/framework/src/celix_bundle_cache.c index 770d83b33..54d6fc2fb 100644 --- a/libs/framework/src/celix_bundle_cache.c +++ b/libs/framework/src/celix_bundle_cache.c @@ -35,7 +35,7 @@ #include "framework_private.h" #include "bundle_archive_private.h" #include "celix_string_hash_map.h" -#include "celix_build_assert.h" +#include "celix_stdio_cleanup.h" //for Celix 3.0 update to a different bundle root scheme //#define CELIX_BUNDLE_ARCHIVE_ROOT_FORMAT "%s/bundle_%li" @@ -218,12 +218,12 @@ void celix_bundleCache_destroyArchive(celix_bundle_cache_t* cache, bundle_archiv /** * Update location->bundle id lookup map. */ -static void celix_bundleCache_updateIdForLocationLookupMap(celix_bundle_cache_t* cache) { - DIR* dir = opendir(cache->cacheDir); +static celix_status_t celix_bundleCache_updateIdForLocationLookupMap(celix_bundle_cache_t* cache) { + celix_autoptr(DIR) dir = opendir(cache->cacheDir); if (dir == NULL) { fw_logCode(cache->fw->logger, CELIX_LOG_LEVEL_ERROR, CELIX_BUNDLE_EXCEPTION, "Cannot open bundle cache directory %s", cache->cacheDir); - return; + return CELIX_FILE_IO_EXCEPTION; } char archiveRootBuffer[CELIX_DEFAULT_STRING_CREATE_BUFFER_SIZE]; struct dirent* dent = NULL; @@ -234,14 +234,15 @@ static void celix_bundleCache_updateIdForLocationLookupMap(celix_bundle_cache_t* char* bundleStateProperties = celix_utils_writeOrCreateString(archiveRootBuffer, sizeof(archiveRootBuffer), "%s/%s/%s", cache->cacheDir, dent->d_name, CELIX_BUNDLE_ARCHIVE_STATE_PROPERTIES_FILE_NAME); + celix_auto(celix_utils_string_guard_t) strGuard = celix_utils_stringGuard_init(archiveRootBuffer, bundleStateProperties); if (celix_utils_fileExists(bundleStateProperties)) { - celix_properties_t* props; - celix_status_t status = celix_properties_load2(bundleStateProperties, 0, &props); //validate the file (and ignore the result + celix_autoptr(celix_properties_t) props = NULL; + celix_status_t status = celix_properties_load2(bundleStateProperties, 0, &props); if (status != CELIX_SUCCESS) { 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); - continue; + return CELIX_FILE_IO_EXCEPTION; } 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); @@ -250,25 +251,26 @@ static void celix_bundleCache_updateIdForLocationLookupMap(celix_bundle_cache_t* visitLoc, bndId); celix_stringHashMap_putLong(cache->locationToBundleIdLookupMap, visitLoc, bndId); } - celix_properties_destroy(props); } - celix_utils_freeStringIfNotEqual(archiveRootBuffer, bundleStateProperties); } - closedir(dir); + return CELIX_SUCCESS; } -long celix_bundleCache_findBundleIdForLocation(celix_bundle_cache_t* cache, const char* location) { - long bndId = -1; - celixThreadMutex_lock(&cache->mutex); +celix_status_t +celix_bundleCache_findBundleIdForLocation(celix_bundle_cache_t* cache, const char* location, long* outBndId) { + *outBndId = -1L; + celix_auto(celix_mutex_lock_guard_t) lck = celixMutexLockGuard_init(&cache->mutex); if (!cache->locationToBundleIdLookupMapLoaded) { - celix_bundleCache_updateIdForLocationLookupMap(cache); + celix_status_t status = celix_bundleCache_updateIdForLocationLookupMap(cache); + if (status != CELIX_SUCCESS) { + return status; + } cache->locationToBundleIdLookupMapLoaded = true; } if (celix_stringHashMap_hasKey(cache->locationToBundleIdLookupMap, location)) { - bndId = celix_stringHashMap_getLong(cache->locationToBundleIdLookupMap, location, -1); + *outBndId = celix_stringHashMap_getLong(cache->locationToBundleIdLookupMap, location, -1); } - celixThreadMutex_unlock(&cache->mutex); - return bndId; + return CELIX_SUCCESS; } bool celix_bundleCache_isBundleIdAlreadyUsed(celix_bundle_cache_t* cache, long bndId) { diff --git a/libs/framework/src/celix_bundle_cache.h b/libs/framework/src/celix_bundle_cache.h index fef8a7422..c6b559716 100644 --- a/libs/framework/src/celix_bundle_cache.h +++ b/libs/framework/src/celix_bundle_cache.h @@ -112,11 +112,13 @@ celix_status_t celix_bundleCache_deleteCacheDir(celix_bundle_cache_t* cache); * @brief Find if the there is already a bundle cache for the provided bundle zip location and if this is true * return the bundle id for the bundle cache entry. * - * @param cache The cache. - * @param location The location of the bundle zip to find the id for. - * @return The bundle id or -1 if not found. + * @param[in] cache The cache. + * @param[in] location The location of the bundle zip to find the id for. + * @param[out] outBndId The bundle id for the bundle cache entry. + * @return CELIX_SUCCESS if the bundle id is found and CELIX_FILE_IO_EXCEPTION if the cache cannot be opened or read. */ -long celix_bundleCache_findBundleIdForLocation(celix_bundle_cache_t* cache, const char* location); +celix_status_t +celix_bundleCache_findBundleIdForLocation(celix_bundle_cache_t* cache, const char* location, long* outBndId); /** * @brief Find if the there is already a bundle cache for the provided bundle id. diff --git a/libs/framework/src/celix_log.c b/libs/framework/src/celix_log.c index c8606b43a..ebe3f236d 100644 --- a/libs/framework/src/celix_log.c +++ b/libs/framework/src/celix_log.c @@ -145,10 +145,10 @@ void celix_framework_logCode(celix_framework_logger_t* logger, celix_log_level_ void celix_framework_logTssErrors(celix_framework_logger_t* logger, celix_log_level_e level) { char buf[CELIX_ERR_BUFFER_SIZE] = {0}; - if (celix_err_dump(buf, sizeof(buf), "[TssErr] ", NULL) == 0) { + if (celix_err_dump(buf, sizeof(buf), NULL, NULL) == 0) { // nothing to output return; } celix_err_resetErrors(); - celix_framework_log(logger, level, NULL, NULL, 0, "Detected tss errors:\n%s", buf); + celix_framework_log(logger, level, NULL, NULL, 0, "Detected tss errors: %s", buf); } diff --git a/libs/framework/src/framework.c b/libs/framework/src/framework.c index 54e360595..99d4946f1 100644 --- a/libs/framework/src/framework.c +++ b/libs/framework/src/framework.c @@ -42,11 +42,12 @@ #include "bundle_archive_private.h" #include "bundle_context_private.h" #include "bundle_private.h" +#include "celix_err.h" +#include "celix_scheduled_event.h" +#include "celix_stdlib_cleanup.h" #include "framework_private.h" #include "service_reference_private.h" #include "service_registration_private.h" -#include "celix_scheduled_event.h" -#include "celix_err.h" #include "utils.h" struct celix_bundle_activator { @@ -624,63 +625,62 @@ bool celix_framework_getConfigPropertyAsBool(celix_framework_t* framework, const static celix_status_t celix_framework_installBundleInternalImpl(celix_framework_t* framework, const char* bndLoc, long* bndId) { - celix_status_t status = CELIX_SUCCESS; - celix_bundle_t* bundle = NULL; - long id = -1L; - - bundle_state_e state = CELIX_BUNDLE_STATE_UNKNOWN; - bool valid = celix_framework_utils_isBundleUrlValid(framework, bndLoc, false); if (!valid) { return CELIX_FILE_IO_EXCEPTION; } - //increase use count of framework bundle to prevent a stop. - celix_framework_bundle_entry_t *fwBundleEntry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(framework, - framework->bundleId); - status = CELIX_DO_IF(status, bundle_getState(framework->bundle, &state)); - if (status == CELIX_SUCCESS) { - if (state == CELIX_BUNDLE_STATE_STOPPING || state == CELIX_BUNDLE_STATE_UNINSTALLED) { - fw_log(framework->logger, CELIX_LOG_LEVEL_INFO, "The framework is being shutdown"); - status = CELIX_FRAMEWORK_SHUTDOWN; - } + // increase use count of framework bundle to prevent a stop. + celix_framework_bundle_entry_t* fwBundleEntry = + celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(framework, framework->bundleId); + + celix_bundle_state_e bndState = celix_bundle_getState(framework->bundle); + if (bndState == CELIX_BUNDLE_STATE_STOPPING || bndState == CELIX_BUNDLE_STATE_UNINSTALLED) { + fw_log(framework->logger, CELIX_LOG_LEVEL_INFO, "The framework is being shutdown"); + return CELIX_FRAMEWORK_SHUTDOWN; } - if (status == CELIX_SUCCESS) { - if (*bndId == -1L) { - id = framework_getBundle(framework, bndLoc); - if (id != -1L) { - celix_framework_bundleEntry_decreaseUseCount(fwBundleEntry); - *bndId = id; - return CELIX_SUCCESS; - } - long alreadyExistingBndId = celix_bundleCache_findBundleIdForLocation(framework->cache, bndLoc); - id = alreadyExistingBndId == -1 ? framework_getNextBundleId(framework) : alreadyExistingBndId; - } else { - id = *bndId; + long id; + if (*bndId == -1L) { + id = framework_getBundle(framework, bndLoc); + if (id != -1L) { + celix_framework_bundleEntry_decreaseUseCount(fwBundleEntry); + *bndId = id; + return CELIX_SUCCESS; } - bundle_archive_t* archive = NULL; - status = CELIX_DO_IF(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_bundle_entry_t *bEntry = fw_bundleEntry_create(bundle); - celix_framework_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); + long alreadyExistingBndId; + 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; } + id = alreadyExistingBndId == -1 ? framework_getNextBundleId(framework) : alreadyExistingBndId; + } else { + id = *bndId; } - if (status == CELIX_SUCCESS) { - *bndId = id; - } else { + 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_framework_bundleEntry_decreaseUseCount(fwBundleEntry); fw_logCode(framework->logger, CELIX_LOG_LEVEL_ERROR, status, "Could not install bundle"); + return status; } + celix_framework_bundle_entry_t *bEntry = fw_bundleEntry_create(bundle); + celix_framework_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); - return status; + *bndId = id; + return CELIX_SUCCESS; } celix_status_t @@ -2349,57 +2349,72 @@ celix_status_t celix_framework_updateBundleEntry(celix_framework_t* framework, const char* updatedBundleUrl) { celix_status_t status = CELIX_SUCCESS; long bundleId = bndEntry->bndId; - const char* errMsg; fw_log(framework->logger, CELIX_LOG_LEVEL_DEBUG, "Updating bundle %s", celix_bundle_getSymbolicName(bndEntry->bnd)); celix_bundle_state_e bndState = celix_bundle_getState(bndEntry->bnd); - char *location = celix_bundle_getLocation(bndEntry->bnd); - do { - // lock to reuse the bundle id - celixThreadMutex_lock(&framework->installLock); - if (updatedBundleUrl == NULL) { - updatedBundleUrl = location; - } else if (strcmp(location, updatedBundleUrl) != 0) { - long alreadyExistingBndId = celix_bundleCache_findBundleIdForLocation(framework->cache, updatedBundleUrl); - if (alreadyExistingBndId != -1 && alreadyExistingBndId != bundleId) { - errMsg = "specified bundle location exists in cache with a different id"; - celix_framework_bundleEntry_decreaseUseCount(bndEntry); - celixThreadMutex_unlock(&framework->installLock); - status = CELIX_ILLEGAL_STATE; - break; - } - celix_bundleArchive_invalidateCache(celix_bundle_getArchive(bndEntry->bnd)); - } - status = celix_framework_uninstallBundleEntryImpl(framework, bndEntry, false); + celix_autofree char* location = celix_bundle_getLocation(bndEntry->bnd); + + // lock to reuse the bundle id + celix_auto(celix_mutex_lock_guard_t) lck = celixMutexLockGuard_init(&framework->installLock); + 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) { - errMsg = "uninstall failure"; - celixThreadMutex_unlock(&framework->installLock); - break; - } - // bndEntry is now invalid - status = celix_framework_installBundleInternalImpl(framework, updatedBundleUrl, &bundleId); - if (status != CELIX_SUCCESS) { - errMsg = "reinstall failure"; - celixThreadMutex_unlock(&framework->installLock); - break; - } - if (bndState != CELIX_BUNDLE_STATE_STARTING && bndState != CELIX_BUNDLE_STATE_ACTIVE) { - // no need to restart the updated bundle - celixThreadMutex_unlock(&framework->installLock); - break; - } - celix_framework_bundle_entry_t* entry = celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(framework, bundleId); - celixThreadMutex_unlock(&framework->installLock); - // assert(entry != NULL); - status = celix_framework_startBundleEntry(framework, entry); - celix_framework_bundleEntry_decreaseUseCount(entry); - if (status != CELIX_SUCCESS) { - errMsg = "restart failure"; - break; - } - } while(0); - framework_logIfError(framework->logger, status, errMsg, "Could not update bundle from %s", updatedBundleUrl); - free(location); - return status; + 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) { + fw_log(framework->logger, + CELIX_LOG_LEVEL_ERROR, + "Specified bundle location %s exists in cache with a different id. Update failed.", + updatedBundleUrl); + celix_framework_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)); + return status; + } + + // bndEntry is now invalid + status = celix_framework_installBundleInternalImpl(framework, updatedBundleUrl, &bundleId); + if (status != CELIX_SUCCESS) { + fw_log(framework->logger, + CELIX_LOG_LEVEL_ERROR, + "Failed to install updated bundle %s", + updatedBundleUrl); + return status; + } + + if (bndState != CELIX_BUNDLE_STATE_STARTING && bndState != CELIX_BUNDLE_STATE_ACTIVE) { + // no need to restart the updated bundle + fw_log(framework->logger, CELIX_LOG_LEVEL_DEBUG, "Bundle %li is not active, no need to restart", bundleId); + return CELIX_SUCCESS; + } + + celix_framework_bundle_entry_t* entry = + celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(framework, bundleId); + celixMutexLockGuard_deinit(&lck); + status = celix_framework_startBundleEntry(framework, entry); + celix_framework_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)); + return CELIX_BUNDLE_EXCEPTION; + } + + return CELIX_SUCCESS; } bool celix_framework_updateBundle(celix_framework_t *fw, long bndId, const char* updatedBundleUrl) { diff --git a/libs/utils/gtest/src/CelixUtilsAutoCleanupTestSuite.cc b/libs/utils/gtest/src/CelixUtilsAutoCleanupTestSuite.cc index a74a6f331..5f02a9a99 100644 --- a/libs/utils/gtest/src/CelixUtilsAutoCleanupTestSuite.cc +++ b/libs/utils/gtest/src/CelixUtilsAutoCleanupTestSuite.cc @@ -193,3 +193,7 @@ TEST_F(CelixUtilsCleanupTestSuite, StealFdTest) { TEST_F(CelixUtilsCleanupTestSuite, FileTest) { celix_autoptr(FILE) file = tmpfile(); } + +TEST_F(CelixUtilsCleanupTestSuite, DirTest) { + celix_autoptr(DIR) dir = opendir("."); +} diff --git a/libs/utils/include/celix_stdio_cleanup.h b/libs/utils/include/celix_stdio_cleanup.h index 43cbff16b..32c4868a7 100644 --- a/libs/utils/include/celix_stdio_cleanup.h +++ b/libs/utils/include/celix_stdio_cleanup.h @@ -25,11 +25,14 @@ extern "C" { #endif #include <stdio.h> +#include <dirent.h> #include "celix_cleanup.h" CELIX_DEFINE_AUTOPTR_CLEANUP_FUNC(FILE, fclose) +CELIX_DEFINE_AUTOPTR_CLEANUP_FUNC(DIR, closedir) + #ifdef __cplusplus } #endif diff --git a/libs/utils/include/celix_threads.h b/libs/utils/include/celix_threads.h index 19c585f3b..612c1869e 100644 --- a/libs/utils/include/celix_threads.h +++ b/libs/utils/include/celix_threads.h @@ -142,6 +142,7 @@ static CELIX_UNUSED inline void celixMutexLockGuard_deinit(celix_mutex_lock_guar if (guard->mutex) { celixThreadMutex_unlock(guard->mutex); } + guard->mutex = NULL; } CELIX_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(celix_mutex_lock_guard_t, celixMutexLockGuard_deinit)
