Skip to content

Commit

Permalink
Merge pull request apache#602 from apache/hotfix/framework-startup-op…
Browse files Browse the repository at this point in the history 
…timization

Hotfix/The Correct Bundle Update behavior
  • Loading branch information
@PengZheng
PengZheng authored Aug 3, 2023
2 parents 072cba2 + 9a0709b commit 2eae9c2
Show file tree
Hide file tree
Showing 10 changed files with 141 additions and 104 deletions.
6 changes: 3 additions & 3 deletions libs/framework/gtest/src/BundleArchiveWithErrorInjectionTestSuite.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down
12 changes: 0 additions & 12 deletions libs/framework/gtest/src/CelixBundleCacheErrorInjectionTestSuite.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,18 +147,6 @@ TEST_F(CelixBundleCacheErrorInjectionTestSuite, SystemArchiveCreateErrorTest) {
EXPECT_EQ(CELIX_SUCCESS, celix_bundleCache_destroy(cache));
}

TEST_F(CelixBundleCacheErrorInjectionTestSuite, ArchiveDestroyErrorTest) {
celix_bundle_cache_t* cache = nullptr;
createCache(&cache);
bundle_archive_t* archive = nullptr;
EXPECT_EQ(CELIX_SUCCESS, celix_bundleCache_createArchive(cache, 1, SIMPLE_TEST_BUNDLE1_LOCATION, &archive));
celix_ei_expect_celix_utils_deleteDirectory((void*)celix_bundleCache_destroyArchive, 1, CELIX_FILE_IO_EXCEPTION);
std::string storeRoot = celix_bundleArchive_getPersistentStoreRoot(archive);
EXPECT_EQ(CELIX_FILE_IO_EXCEPTION, celix_bundleCache_destroyArchive(cache, archive, true));
EXPECT_TRUE(celix_utils_directoryExists(storeRoot.c_str()));
EXPECT_EQ(CELIX_SUCCESS, celix_bundleCache_destroy(cache));
}

TEST_F(CelixBundleCacheErrorInjectionTestSuite, CreateBundleArchivesCacheErrorTest) {
celix_bundle_cache_t* cache = nullptr;
celix_properties_set(fw.configurationMap, CELIX_AUTO_START_1, SIMPLE_TEST_BUNDLE1_LOCATION);
Expand Down
8 changes: 5 additions & 3 deletions libs/framework/gtest/src/CelixBundleCacheTestSuite.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ TEST_F(CelixBundleCacheTestSuite, ArchiveCreateDestroyTest) {
EXPECT_TRUE(celix_bundleCache_isBundleIdAlreadyUsed(fw.cache, 1));
std::string loc = celix_bundleArchive_getPersistentStoreRoot(archive);
EXPECT_TRUE(celix_utils_directoryExists(loc.c_str()));
EXPECT_EQ(CELIX_SUCCESS, celix_bundleCache_destroyArchive(fw.cache, archive, true));
celix_bundleArchive_invalidate(archive);
celix_bundleCache_destroyArchive(fw.cache, archive);
EXPECT_EQ(-1, celix_bundleCache_findBundleIdForLocation(fw.cache, SIMPLE_TEST_BUNDLE1_LOCATION));
EXPECT_FALSE(celix_bundleCache_isBundleIdAlreadyUsed(fw.cache, 1));
EXPECT_FALSE(celix_utils_directoryExists(loc.c_str()));
Expand All @@ -70,7 +71,7 @@ TEST_F(CelixBundleCacheTestSuite, NonPermanentDestroyTest) {
EXPECT_NE(nullptr, archive);
std::string loc = celix_bundleArchive_getPersistentStoreRoot(archive);
EXPECT_TRUE(celix_utils_directoryExists(loc.c_str()));
EXPECT_EQ(CELIX_SUCCESS, celix_bundleCache_destroyArchive(fw.cache, archive, false));
celix_bundleCache_destroyArchive(fw.cache, archive);
EXPECT_EQ(1, celix_bundleCache_findBundleIdForLocation(fw.cache, SIMPLE_TEST_BUNDLE1_LOCATION));
EXPECT_TRUE(celix_bundleCache_isBundleIdAlreadyUsed(fw.cache, 1));
EXPECT_TRUE(celix_utils_directoryExists(loc.c_str()));
Expand All @@ -85,7 +86,8 @@ TEST_F(CelixBundleCacheTestSuite, SystemArchiveCreateDestroyTest) {
EXPECT_EQ(CELIX_SUCCESS, bundleArchive_getArchiveRoot(archive, &archiveRoot));
EXPECT_EQ(nullptr, archiveRoot);
EXPECT_EQ(nullptr, celix_bundleArchive_getLocation(archive));
EXPECT_EQ(CELIX_SUCCESS, celix_bundleCache_destroyArchive(fw.cache, archive, true));
celix_bundleArchive_invalidate(archive);
celix_bundleCache_destroyArchive(fw.cache, archive);
}

TEST_F(CelixBundleCacheTestSuite, CreateBundleArchivesCacheTest) {
Expand Down
21 changes: 21 additions & 0 deletions libs/framework/gtest/src/CelixBundleContextBundlesTestSuite.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@

#include "bundle_context_private.h"
#include "celix_api.h"
#include "celix_bundle.h"
#include "celix_bundle_context.h"
#include "celix_stdlib_cleanup.h"
#include "celix_file_utils.h"

class CelixBundleContextBundlesTestSuite : public ::testing::Test {
Expand Down Expand Up @@ -313,6 +316,24 @@ TEST_F(CelixBundleContextBundlesTestSuite, UpdateBundlesTest) {
ASSERT_FALSE(celix_bundleContext_updateBundle(ctx, bndId1, NULL));
}

TEST_F(CelixBundleContextBundlesTestSuite, ForceUpdateUsingBundleFromDifferentLocation) {
long bndId1 = celix_bundleContext_installBundle(ctx, TEST_BND1_LOC, true);
ASSERT_TRUE(bndId1 >= 0L);
ASSERT_TRUE(celix_bundleContext_isBundleInstalled(ctx, bndId1));
ASSERT_TRUE(celix_bundleContext_isBundleActive(ctx, bndId1));
celix_autofree char* name1 = celix_bundleContext_getBundleSymbolicName(ctx, bndId1);
EXPECT_TRUE(celix_bundleContext_useBundle(ctx, bndId1, nullptr, [] (void *, const bundle_t *bundle) {
// make bundle cache root newer than the bundle at location TEST_BND2_LOC
celix_autofree char* root = celix_bundle_getEntry(bundle, "/");
celix_utils_touch(root);
}));
ASSERT_TRUE(celix_bundleContext_updateBundle(ctx, bndId1, TEST_BND2_LOC));
ASSERT_TRUE(celix_bundleContext_isBundleInstalled(ctx, bndId1));
celix_autofree char* name2 = celix_bundleContext_getBundleSymbolicName(ctx, bndId1);
// bundle cache contains the bundle at location TEST_BND2_LOC
ASSERT_STRNE(name1, name2);
}

TEST_F(CelixBundleContextBundlesTestSuite, UpdateCorruptUncompressedBundlesTest) {
const char* testExtractDir1 = "extractBundleTestDir1";
const char* testExtractDir2 = "extractBundleTestDir2";
Expand Down
16 changes: 2 additions & 14 deletions libs/framework/src/bundle.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -334,20 +334,8 @@ celix_status_t bundle_close(const_bundle_pt bundle) {
}

celix_status_t bundle_closeAndDelete(const_bundle_pt bundle) {
celix_status_t status;

bundle_archive_pt archive = NULL;

bundle_closeModules(bundle);
bundle_closeRevisions(bundle);
status = bundle_getArchive(bundle, &archive);
if (status == CELIX_SUCCESS) {
bundleArchive_closeAndDelete(archive);
}

framework_logIfError(bundle->framework->logger, status, NULL, "Failed to close and delete bundle");

return status;
fw_log(bundle->framework->logger, CELIX_LOG_LEVEL_DEBUG, "Usage of bundle_closeAndDelete is deprecated and no longer needed. Called for bundle %s", bundle->symbolicName);
return CELIX_SUCCESS;
}

celix_status_t bundle_closeRevisions(const_bundle_pt bundle) {
Expand Down
113 changes: 68 additions & 45 deletions libs/framework/src/bundle_archive.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,10 @@ struct bundleArchive {
char* resourceCacheRoot;
char* bundleSymbolicName; // read from the manifest
char* bundleVersion; // read from the manifest

celix_thread_mutex_t lock; // protects below and saving of bundle state properties
bundle_revision_t* revision; // the current revision
char* location;
bool cacheValid; // is the cache valid (e.g. not deleted)
bool valid; // is the archive valid (e.g. not deleted)
};

static celix_status_t celix_bundleArchive_storeBundleStateProperties(bundle_archive_pt archive) {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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.");
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -296,7 +304,8 @@ celix_status_t celix_bundleArchive_create(celix_framework_t* fw, const char *arc
goto store_prop_failed;
}
}

archive->cacheValid = true;
archive->valid = true;
*bundle_archive = archive;
return CELIX_SUCCESS;
store_prop_failed:
Expand All @@ -312,7 +321,7 @@ celix_status_t celix_bundleArchive_create(celix_framework_t* fw, const char *arc
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);
Expand All @@ -322,10 +331,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) {
Expand All @@ -342,19 +349,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;
}

Expand All @@ -371,9 +374,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;
}

Expand Down Expand Up @@ -416,9 +417,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;
}

Expand Down Expand Up @@ -447,17 +446,15 @@ celix_status_t bundleArchive_close(bundle_archive_pt archive) {
// not yet needed/possible
return CELIX_SUCCESS;
}
//LCOV_EXCL_STOP

celix_status_t bundleArchive_closeAndDelete(bundle_archive_pt archive) {
celix_status_t status = CELIX_SUCCESS;
if (archive->id != CELIX_FRAMEWORK_BUNDLE_ID) {
const char* err = NULL;
status = celix_utils_deleteDirectory(archive->archiveRoot, &err);
framework_logIfError(archive->fw->logger, status, NULL, "Failed to delete archive root '%s': %s", archive->archiveRoot, err);
}
return status;
fw_log(archive->fw->logger,
CELIX_LOG_LEVEL_DEBUG,
"Usage of bundleArchive_closeAndDelete is deprecated and no longer needed. Called for bundle %s",
archive->bundleSymbolicName);
return CELIX_SUCCESS;
}
//LCOV_EXCL_STOP

const char* celix_bundleArchive_getPersistentStoreRoot(bundle_archive_t* archive) {
return archive->storeRoot;
Expand All @@ -468,3 +465,29 @@ const char* celix_bundleArchive_getCurrentRevisionRoot(bundle_archive_t* archive
}


void celix_bundleArchive_invalidate(bundle_archive_pt archive) {
archive->valid = false;
archive->cacheValid = false;
}

void celix_bundleArchive_invalidateCache(bundle_archive_pt archive) {
archive->cacheValid = false;
}

bool celix_bundleArchive_isCacheValid(bundle_archive_pt archive) {
return archive->cacheValid;
}

void celix_bundleArchive_removeInvalidDirs(bundle_archive_pt archive) {
if (archive->id == CELIX_FRAMEWORK_BUNDLE_ID) {
return;
}
if (!archive->valid) {
celix_status_t status = CELIX_SUCCESS;
const char* err = NULL;
status = celix_utils_deleteDirectory(archive->archiveRoot, &err);
framework_logIfError(archive->fw->logger, status, NULL, "Failed to remove invalid archive root '%s': %s", archive->archiveRoot, err);
} else if (!archive->cacheValid){
(void)celix_bundleArchive_removeResourceCache(archive);
}
}
22 changes: 21 additions & 1 deletion libs/framework/src/bundle_archive_private.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -75,6 +75,26 @@ const char* celix_bundleArchive_getPersistentStoreRoot(bundle_archive_t *archive
*/
const char* celix_bundleArchive_getCurrentRevisionRoot(bundle_archive_pt archive);

/**
* @brief Invalidate the whole bundle archive.
*/
void celix_bundleArchive_invalidate(bundle_archive_pt archive);

/**
* @brief Invalidate the bundle archive's bundle cache.
*/
void celix_bundleArchive_invalidateCache(bundle_archive_pt archive);

/**
* @brief Return if the bundle cache is valid.
*/
bool celix_bundleArchive_isCacheValid(bundle_archive_pt archive);

/**
* @brief Remove all valid directories of the bundle archive.
*/
void celix_bundleArchive_removeInvalidDirs(bundle_archive_pt archive);

#ifdef __cplusplus
}
#endif
Expand Down
Loading

0 comments on commit 2eae9c2

Please sign in to comment.