This is an automated email from the ASF dual-hosted git repository. pengzheng pushed a commit to branch hotfix/framework-startup-optimization in repository https://gitbox.apache.org/repos/asf/celix.git
commit 8f08933285bff807b927d2872da9913a701c4878 Author: PengZheng <[email protected]> AuthorDate: Wed Aug 2 17:25:02 2023 +0800 Code cleanup for bundle_archive.c --- .../BundleArchiveWithErrorInjectionTestSuite.cc | 6 +- libs/framework/src/bundle_archive.c | 66 +++++++++++----------- libs/framework/src/bundle_archive_private.h | 2 +- libs/framework/src/celix_bundle_cache.c | 2 +- 4 files changed, 37 insertions(+), 39 deletions(-) diff --git a/libs/framework/gtest/src/BundleArchiveWithErrorInjectionTestSuite.cc b/libs/framework/gtest/src/BundleArchiveWithErrorInjectionTestSuite.cc index e4d5d422..03100ddf 100644 --- a/libs/framework/gtest/src/BundleArchiveWithErrorInjectionTestSuite.cc +++ b/libs/framework/gtest/src/BundleArchiveWithErrorInjectionTestSuite.cc @@ -198,7 +198,7 @@ TEST_F(CelixBundleArchiveErrorInjectionTestSuite, ArchiveCreateErrorTest) { EXPECT_FALSE(celix_utils_directoryExists(TEST_ARCHIVE_ROOT)); teardownErrorInjectors(); - celix_ei_expect_lstat((void*) celix_bundleArchive_create, 2, -1); + celix_ei_expect_lstat((void*) celix_bundleArchive_create, 3, -1); EXPECT_EQ(CELIX_ERROR_MAKE(CELIX_FACILITY_CERRNO,EACCES), celix_bundleArchive_create(&fw, TEST_ARCHIVE_ROOT, 1, SIMPLE_TEST_BUNDLE1_LOCATION, &archive)); EXPECT_EQ(nullptr, archive); @@ -213,7 +213,7 @@ TEST_F(CelixBundleArchiveErrorInjectionTestSuite, ArchiveCreateErrorTest) { archive = nullptr; std::this_thread::sleep_for(std::chrono::milliseconds(10)); celix_utils_touch(SIMPLE_TEST_BUNDLE1_LOCATION); - celix_ei_expect_unlink((void*)celix_bundleArchive_create, 2, -1); + celix_ei_expect_unlink((void*)celix_bundleArchive_create, 3, -1); EXPECT_EQ(CELIX_ERROR_MAKE(CELIX_FACILITY_CERRNO,EACCES), celix_bundleArchive_create(&fw, TEST_ARCHIVE_ROOT, 2, SIMPLE_TEST_BUNDLE1_LOCATION, &archive)); EXPECT_EQ(nullptr, archive); @@ -226,7 +226,7 @@ TEST_F(CelixBundleArchiveErrorInjectionTestSuite, ArchiveCreateErrorTest) { archive = nullptr; std::this_thread::sleep_for(std::chrono::milliseconds(10)); celix_utils_touch(SIMPLE_TEST_BUNDLE1_LOCATION); - celix_ei_expect_celix_utils_deleteDirectory((void*)celix_bundleArchive_create, 2, CELIX_FILE_IO_EXCEPTION); + celix_ei_expect_celix_utils_deleteDirectory((void*)celix_bundleArchive_create, 3, CELIX_FILE_IO_EXCEPTION); EXPECT_EQ(CELIX_FILE_IO_EXCEPTION, celix_bundleArchive_create(&fw, TEST_ARCHIVE_ROOT, 1, SIMPLE_TEST_BUNDLE1_LOCATION, &archive)); EXPECT_EQ(nullptr, archive); diff --git a/libs/framework/src/bundle_archive.c b/libs/framework/src/bundle_archive.c index 8d55e77e..37f5ed2c 100644 --- a/libs/framework/src/bundle_archive.c +++ b/libs/framework/src/bundle_archive.c @@ -97,6 +97,35 @@ static celix_status_t celix_bundleArchive_storeBundleStateProperties(bundle_arch return CELIX_SUCCESS; } +static celix_status_t celix_bundleArchive_removeResourceCache(bundle_archive_t* archive) { + celix_status_t status = CELIX_SUCCESS; + const char* error = NULL; + struct stat st; + status = lstat(archive->resourceCacheRoot, &st); + if (status == -1 && errno == ENOENT) { + return CELIX_SUCCESS; + } else if(status == -1 && errno != ENOENT) { + status = CELIX_ERROR_MAKE(CELIX_FACILITY_CERRNO,errno); + fw_logCode(archive->fw->logger, CELIX_LOG_LEVEL_ERROR, status, "Failed to stat bundle archive directory '%s'", archive->resourceCacheRoot); + return status; + } + // assert(status == 0); + // celix_utils_deleteDirectory does not work for dangling symlinks, so handle this case separately + if (S_ISLNK(st.st_mode)) { + status = unlink(archive->resourceCacheRoot); + if (status == -1) { + status = CELIX_ERROR_MAKE(CELIX_FACILITY_CERRNO,errno); + error = "Failed to remove existing bundle symlink"; + } + } else { + status = celix_utils_deleteDirectory(archive->resourceCacheRoot, &error); + } + if (status != CELIX_SUCCESS) { + fw_logCode(archive->fw->logger, CELIX_LOG_LEVEL_ERROR, status, "Failed to remove existing bundle archive revision directory '%s': %s", archive->resourceCacheRoot, error); + } + return status; +} + static celix_status_t celix_bundleArchive_extractBundle(bundle_archive_t* archive, const char* bundleUrl) { celix_status_t status = CELIX_SUCCESS; @@ -126,30 +155,10 @@ celix_bundleArchive_extractBundle(bundle_archive_t* archive, const char* bundleU * If dlopen/dlsym is used with newer files, but with the same inode already used in dlopen/dlsym this leads to * segfaults. */ - const char* error = NULL; - struct stat st; - status = lstat(archive->resourceCacheRoot, &st); - if(status == -1 && errno != ENOENT) { - status = CELIX_ERROR_MAKE(CELIX_FACILITY_CERRNO,errno); - fw_logCode(archive->fw->logger, CELIX_LOG_LEVEL_ERROR, status, "Failed to stat bundle archive directory '%s'", archive->resourceCacheRoot); + status = celix_bundleArchive_removeResourceCache(archive); + if (status != CELIX_SUCCESS) { return status; } - if (status == 0) { - // celix_utils_deleteDirectory does not work for dangling symlinks, so handle this case separately - if (S_ISLNK(st.st_mode)) { - status = unlink(archive->resourceCacheRoot); - if (status == -1) { - status = CELIX_ERROR_MAKE(CELIX_FACILITY_CERRNO,errno); - error = "Failed to remove existing bundle symlink"; - } - } else { - status = celix_utils_deleteDirectory(archive->resourceCacheRoot, &error); - } - if (status != CELIX_SUCCESS) { - fw_logCode(archive->fw->logger, CELIX_LOG_LEVEL_ERROR, status, "Failed to remove existing bundle archive revision directory '%s': %s", archive->resourceCacheRoot, error); - return status; - } - } status = celix_framework_utils_extractBundle(archive->fw, bundleUrl, archive->resourceCacheRoot); if (status != CELIX_SUCCESS) { fw_log(archive->fw->logger, CELIX_LOG_LEVEL_ERROR, "Failed to initialize archive. Failed to extract bundle zip to revision directory."); @@ -235,7 +244,6 @@ celix_status_t celix_bundleArchive_create(celix_framework_t* fw, const char *arc archive->fw = fw; archive->id = id; - celixThreadMutex_create(&archive->lock, NULL); if (isSystemBundle) { archive->resourceCacheRoot = getcwd(NULL, 0); @@ -312,7 +320,7 @@ calloc_failed: return status; } -celix_status_t bundleArchive_destroy(bundle_archive_pt archive) { +void bundleArchive_destroy(bundle_archive_pt archive) { if (archive != NULL) { free(archive->location); free(archive->savedBundleStatePropertiesPath); @@ -322,10 +330,8 @@ celix_status_t bundleArchive_destroy(bundle_archive_pt archive) { free(archive->bundleSymbolicName); free(archive->bundleVersion); bundleRevision_destroy(archive->revision); - celixThreadMutex_destroy(&archive->lock); free(archive); } - return CELIX_SUCCESS; } celix_status_t bundleArchive_getId(bundle_archive_pt archive, long* id) { @@ -342,19 +348,15 @@ const char* celix_bundleArchive_getSymbolicName(bundle_archive_pt archive) { } celix_status_t bundleArchive_getLocation(bundle_archive_pt archive, const char **location) { - celixThreadMutex_lock(&archive->lock); *location = archive->location; - celixThreadMutex_unlock(&archive->lock); return CELIX_SUCCESS; } char* celix_bundleArchive_getLocation(bundle_archive_pt archive) { char* result = NULL; - celixThreadMutex_lock(&archive->lock); if (archive->location) { result = celix_utils_strdup(archive->location); } - celixThreadMutex_unlock(&archive->lock); return result; } @@ -371,9 +373,7 @@ celix_status_t bundleArchive_getCurrentRevisionNumber(bundle_archive_pt archive, //LCOV_EXCL_STOP celix_status_t bundleArchive_getCurrentRevision(bundle_archive_pt archive, bundle_revision_pt* revision) { - celixThreadMutex_lock(&archive->lock); *revision = archive->revision; - celixThreadMutex_unlock(&archive->lock); return CELIX_SUCCESS; } @@ -416,9 +416,7 @@ celix_status_t bundleArchive_getLastModified(bundle_archive_pt archive, time_t* celix_status_t celix_bundleArchive_getLastModified(bundle_archive_pt archive, struct timespec* lastModified) { celix_status_t status; - celixThreadMutex_lock(&archive->lock); status = celix_utils_getLastModified(archive->resourceCacheRoot, lastModified); - celixThreadMutex_unlock(&archive->lock); return status; } diff --git a/libs/framework/src/bundle_archive_private.h b/libs/framework/src/bundle_archive_private.h index b6a837d9..d777ea50 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); -celix_status_t bundleArchive_destroy(bundle_archive_pt archive); +void 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 64fb332f..059c26a4 100644 --- a/libs/framework/src/celix_bundle_cache.c +++ b/libs/framework/src/celix_bundle_cache.c @@ -234,8 +234,8 @@ celix_status_t celix_bundleCache_destroyArchive(celix_bundle_cache_t* cache, bun celix_status_t status = CELIX_SUCCESS; const char* loc = NULL; celixThreadMutex_lock(&cache->mutex); - (void) bundleArchive_getLocation(archive, &loc); if (permanent) { + (void) bundleArchive_getLocation(archive, &loc); (void) celix_stringHashMap_remove(cache->locationToBundleIdLookupMap, loc); status = bundleArchive_closeAndDelete(archive); }
