From f5fe0f4f2d26a1fd6dc7c9948355114f3f8645f2 Mon Sep 17 00:00:00 2001 From: PengZheng Date: Fri, 12 May 2023 22:26:09 +0800 Subject: [PATCH 1/2] Implement the OSGi semantics of bundle uninstallation. "To whatever extent possible, the Framework must remove any resources related to the bundle. This method must always uninstall the bundle from the persistent storage of the Framework." --- .../gtest/src/BundleArchiveTestSuite.cc | 27 ++++++++++++-- libs/framework/src/framework.c | 37 +++++++++---------- .../src/framework_bundle_lifecycle_handler.c | 4 +- libs/framework/src/framework_private.h | 3 +- 4 files changed, 45 insertions(+), 26 deletions(-) diff --git a/libs/framework/gtest/src/BundleArchiveTestSuite.cc b/libs/framework/gtest/src/BundleArchiveTestSuite.cc index 98b376a82..59cc1c1c5 100644 --- a/libs/framework/gtest/src/BundleArchiveTestSuite.cc +++ b/libs/framework/gtest/src/BundleArchiveTestSuite.cc @@ -74,8 +74,18 @@ TEST_F(CxxBundleArchiveTestSuite, BundleArchiveReusedTest) { auto firstBundleRevisionTime = installTime; lock.unlock(); - //uninstall and reinstall - ctx->uninstallBundle(bndId1); + tracker.reset(); + fw = celix::createFramework({ + {"CELIX_LOGGING_DEFAULT_ACTIVE_LOG_LEVEL", "trace"}, + {CELIX_FRAMEWORK_CLEAN_CACHE_DIR_ON_CREATE, "false"} + }); + ctx = fw->getFrameworkBundleContext(); + tracker = ctx->trackBundles() + .addOnInstallCallback([&](const celix::Bundle& b) { + std::lock_guard lock{m}; + auto *archive = celix_bundle_getArchive(b.getCBundle()); + EXPECT_EQ(CELIX_SUCCESS, celix_bundleArchive_getLastModified(archive, &installTime)); + }).build(); std::this_thread::sleep_for(std::chrono::milliseconds{100}); //wait so that the zip <-> archive dir modification time is different long bndId2 = ctx->installBundle(SIMPLE_TEST_BUNDLE1_LOCATION); EXPECT_GT(bndId2, -1); @@ -89,7 +99,18 @@ TEST_F(CxxBundleArchiveTestSuite, BundleArchiveReusedTest) { auto secondBundleRevisionTime = installTime; - ctx->uninstallBundle(bndId1); + tracker.reset(); + fw = celix::createFramework({ + {"CELIX_LOGGING_DEFAULT_ACTIVE_LOG_LEVEL", "trace"}, + {CELIX_FRAMEWORK_CLEAN_CACHE_DIR_ON_CREATE, "false"} + }); + ctx = fw->getFrameworkBundleContext(); + tracker = ctx->trackBundles() + .addOnInstallCallback([&](const celix::Bundle& b) { + std::lock_guard lock{m}; + auto *archive = celix_bundle_getArchive(b.getCBundle()); + EXPECT_EQ(CELIX_SUCCESS, celix_bundleArchive_getLastModified(archive, &installTime)); + }).build(); std::this_thread::sleep_for(std::chrono::milliseconds{100}); //wait so that the zip <-> archive dir modification time is different celix_utils_touch(SIMPLE_TEST_BUNDLE1_LOCATION); //touch the bundle zip file to force an update long bndId3 = ctx->installBundle(SIMPLE_TEST_BUNDLE1_LOCATION); diff --git a/libs/framework/src/framework.c b/libs/framework/src/framework.c index 446628504..df4bb7b49 100644 --- a/libs/framework/src/framework.c +++ b/libs/framework/src/framework.c @@ -1180,7 +1180,7 @@ static void* framework_shutdown(void *framework) { } 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_framework_uninstallBundleEntry(fw, entry); + celix_framework_uninstallBundleEntry(fw, entry, false); } celix_arrayList_destroy(stopEntries); @@ -1891,14 +1891,14 @@ void celix_framework_uninstallBundleAsync(celix_framework_t *fw, long bndId) { celix_framework_uninstallBundleInternal(fw, bndId, true); } -celix_status_t celix_framework_uninstallBundleEntry(celix_framework_t* framework, celix_framework_bundle_entry_t* bndEntry) { - assert(!celix_framework_isCurrentThreadTheEventLoop(framework)); +celix_status_t celix_framework_uninstallBundleEntry(celix_framework_t* fw, celix_framework_bundle_entry_t* bndEntry, bool permanent) { + assert(!celix_framework_isCurrentThreadTheEventLoop(fw)); celix_bundle_state_e bndState = celix_bundle_getState(bndEntry->bnd); if (bndState == CELIX_BUNDLE_STATE_ACTIVE) { - celix_framework_stopBundleEntry(framework, bndEntry); + celix_framework_stopBundleEntry(fw, bndEntry); } - celix_framework_bundle_entry_t* removedEntry = fw_bundleEntry_removeBundleEntryAndIncreaseUseCount(framework, bndEntry->bndId); + celix_framework_bundle_entry_t* removedEntry = fw_bundleEntry_removeBundleEntryAndIncreaseUseCount(fw, bndEntry->bndId); celix_framework_bundleEntry_decreaseUseCount(bndEntry); if (removedEntry != NULL) { @@ -1906,35 +1906,32 @@ celix_status_t celix_framework_uninstallBundleEntry(celix_framework_t* framework celix_bundle_t *bnd = removedEntry->bnd; if (status == CELIX_SUCCESS) { - bundle_archive_t *archive = NULL; - bundle_revision_t *revision = NULL; celix_module_t* module = NULL; - status = CELIX_DO_IF(status, bundle_getArchive(bnd, &archive)); - status = CELIX_DO_IF(status, bundleArchive_getCurrentRevision(archive, &revision)); status = CELIX_DO_IF(status, bundle_getCurrentModule(bnd, &module)); - if (module) { celix_module_closeLibraries(module); } - - CELIX_DO_IF(status, fw_fireBundleEvent(framework, OSGI_FRAMEWORK_BUNDLE_EVENT_UNRESOLVED, removedEntry)); - + CELIX_DO_IF(status, fw_fireBundleEvent(fw, OSGI_FRAMEWORK_BUNDLE_EVENT_UNRESOLVED, removedEntry)); status = CELIX_DO_IF(status, bundle_setState(bnd, CELIX_BUNDLE_STATE_UNINSTALLED)); - - CELIX_DO_IF(status, fw_fireBundleEvent(framework, OSGI_FRAMEWORK_BUNDLE_EVENT_UNINSTALLED, removedEntry)); - + CELIX_DO_IF(status, fw_fireBundleEvent(fw, OSGI_FRAMEWORK_BUNDLE_EVENT_UNINSTALLED, removedEntry)); //NOTE wait outside installedBundles.mutex celix_framework_bundleEntry_decreaseUseCount(removedEntry); fw_bundleEntry_destroy(removedEntry , true); //wait till use count is 0 -> e.g. not used if (status == CELIX_SUCCESS) { - celix_framework_waitForEmptyEventQueue(framework); //to ensure that the uninstall event is triggered and handled - bundleArchive_destroy(archive); - status = CELIX_DO_IF(status, bundle_closeModules(bnd)); + celix_framework_waitForEmptyEventQueue(fw); //to ensure that the uninstall event is triggered and handled + if (permanent) { + status = CELIX_DO_IF(status, bundle_closeAndDelete(bnd)); + } else { + status = CELIX_DO_IF(status, bundle_closeModules(bnd)); + } + bundle_archive_t *archive = NULL; + status = CELIX_DO_IF(status, bundle_getArchive(bnd, &archive)); + status = CELIX_DO_IF(status, bundleArchive_destroy(archive)); status = CELIX_DO_IF(status, bundle_destroy(bnd)); } } - framework_logIfError(framework->logger, status, "", "Cannot uninstall bundle"); + framework_logIfError(fw->logger, status, "", "Cannot uninstall bundle"); return status; } else { diff --git a/libs/framework/src/framework_bundle_lifecycle_handler.c b/libs/framework/src/framework_bundle_lifecycle_handler.c index c0851943e..669778d88 100644 --- a/libs/framework/src/framework_bundle_lifecycle_handler.c +++ b/libs/framework/src/framework_bundle_lifecycle_handler.c @@ -47,7 +47,7 @@ static void* celix_framework_BundleLifecycleHandlingThread(void *data) { break; case CELIX_BUNDLE_LIFECYCLE_UNINSTALL: celix_framework_bundleEntry_decreaseUseCount(handler->bndEntry); - celix_framework_uninstallBundleEntry(handler->framework, handler->bndEntry); + celix_framework_uninstallBundleEntry(handler->framework, handler->bndEntry, true); break; default: //update celix_framework_updateBundleEntry(handler->framework, handler->bndEntry, handler->updatedBundleUrl); @@ -143,7 +143,7 @@ celix_status_t celix_framework_uninstallBundleOnANonCelixEventThread(celix_frame celix_framework_createAndStartBundleLifecycleHandler(fw, bndEntry, CELIX_BUNDLE_LIFECYCLE_UNINSTALL, NULL); return CELIX_SUCCESS; } else { - return celix_framework_uninstallBundleEntry(fw, bndEntry); + return celix_framework_uninstallBundleEntry(fw, bndEntry, true); } } diff --git a/libs/framework/src/framework_private.h b/libs/framework/src/framework_private.h index 47b22c8bd..8d60d4c70 100644 --- a/libs/framework/src/framework_private.h +++ b/libs/framework/src/framework_private.h @@ -40,6 +40,7 @@ #include "celix_threads.h" #include "service_registry.h" +#include #ifndef CELIX_FRAMEWORK_DEFAULT_STATIC_EVENT_QUEUE_SIZE #define CELIX_FRAMEWORK_DEFAULT_STATIC_EVENT_QUEUE_SIZE 1024 @@ -426,7 +427,7 @@ celix_status_t celix_framework_stopBundleEntry(celix_framework_t* fw, celix_fram /** * 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); +celix_status_t celix_framework_uninstallBundleEntry(celix_framework_t* fw, celix_framework_bundle_entry_t* bndEntry, bool permanent); /** * Uninstall a bundle. Cannot be called on the Celix event thread. From b1ab9caffcaa3a56bb5bd8a4adade089447081e8 Mon Sep 17 00:00:00 2001 From: PengZheng Date: Sat, 13 May 2023 11:47:02 +0800 Subject: [PATCH 2/2] Add test for bundle uninstall. --- .../src/CelixBundleContextBundlesTestSuite.cc | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/libs/framework/gtest/src/CelixBundleContextBundlesTestSuite.cc b/libs/framework/gtest/src/CelixBundleContextBundlesTestSuite.cc index 1fa35982c..2d8ae49c4 100644 --- a/libs/framework/gtest/src/CelixBundleContextBundlesTestSuite.cc +++ b/libs/framework/gtest/src/CelixBundleContextBundlesTestSuite.cc @@ -28,6 +28,7 @@ #include #include "celix_api.h" +#include "celix_file_utils.h" class CelixBundleContextBundlesTestSuite : public ::testing::Test { public: @@ -152,6 +153,29 @@ TEST_F(CelixBundleContextBundlesTestSuite, InstallAndUninstallBundlesTest) { ASSERT_FALSE(celix_bundleContext_isBundleActive(ctx, bndId2)); //not auto started ASSERT_TRUE(celix_bundleContext_isBundleActive(ctx, bndId3)); + char *bndRoot1 = nullptr; + ASSERT_TRUE(celix_bundleContext_useBundle(ctx, bndId1, &bndRoot1, [](void* handle, const celix_bundle_t* bnd) { + char **root = static_cast(handle); + *root = celix_bundle_getEntry(bnd, "/"); + })); + ASSERT_TRUE(bndRoot1 != nullptr); + char* bndRoot2 = nullptr; + ASSERT_TRUE(celix_bundleContext_useBundle(ctx, bndId2, &bndRoot2, [](void* handle, const celix_bundle_t* bnd) { + char **root = static_cast(handle); + *root = celix_bundle_getEntry(bnd, "/"); + })); + ASSERT_TRUE(bndRoot2 != nullptr); + char* bndRoot3 = nullptr; + ASSERT_TRUE(celix_bundleContext_useBundle(ctx, bndId3, &bndRoot3, [](void* handle, const celix_bundle_t* bnd) { + char **root = static_cast(handle); + *root = celix_bundle_getEntry(bnd, "/"); + })); + ASSERT_TRUE(bndRoot3 != nullptr); + + ASSERT_TRUE(celix_utils_directoryExists(bndRoot1)); + ASSERT_TRUE(celix_utils_directoryExists(bndRoot2)); + ASSERT_TRUE(celix_utils_directoryExists(bndRoot3)); + //uninstall bundles ASSERT_TRUE(celix_bundleContext_uninstallBundle(ctx, bndId1)); ASSERT_TRUE(celix_bundleContext_uninstallBundle(ctx, bndId2)); @@ -165,6 +189,14 @@ TEST_F(CelixBundleContextBundlesTestSuite, InstallAndUninstallBundlesTest) { ASSERT_FALSE(celix_bundleContext_isBundleActive(ctx, bndId2)); ASSERT_FALSE(celix_bundleContext_isBundleActive(ctx, bndId3)); + ASSERT_FALSE(celix_utils_directoryExists(bndRoot1)); + ASSERT_FALSE(celix_utils_directoryExists(bndRoot2)); + ASSERT_FALSE(celix_utils_directoryExists(bndRoot3)); + + free(bndRoot1); + free(bndRoot2); + free(bndRoot3); + //reinstall bundles long bndId4 = celix_bundleContext_installBundle(ctx, TEST_BND1_LOC, true); long bndId5 = celix_bundleContext_installBundle(ctx, TEST_BND2_LOC, false);