This is an automated email from the ASF dual-hosted git repository. pengzheng pushed a commit to branch revert-554-feature/correct_uninstall_semantics in repository https://gitbox.apache.org/repos/asf/celix.git
commit 77722f5ae74e47da225eec7bdea0647f4edd66d4 Author: PengZheng <[email protected]> AuthorDate: Sun May 14 11:06:11 2023 +0800 Revert "[RFC]the OSGi semantics of bundle uninstall." --- libs/framework/gtest/src/BundleArchiveTestSuite.cc | 27 ++-------------- .../src/CelixBundleContextBundlesTestSuite.cc | 32 ------------------- libs/framework/src/framework.c | 37 ++++++++++++---------- .../src/framework_bundle_lifecycle_handler.c | 4 +-- libs/framework/src/framework_private.h | 3 +- 5 files changed, 26 insertions(+), 77 deletions(-) diff --git a/libs/framework/gtest/src/BundleArchiveTestSuite.cc b/libs/framework/gtest/src/BundleArchiveTestSuite.cc index 59cc1c1c..98b376a8 100644 --- a/libs/framework/gtest/src/BundleArchiveTestSuite.cc +++ b/libs/framework/gtest/src/BundleArchiveTestSuite.cc @@ -74,18 +74,8 @@ TEST_F(CxxBundleArchiveTestSuite, BundleArchiveReusedTest) { auto firstBundleRevisionTime = installTime; lock.unlock(); - 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<std::mutex> lock{m}; - auto *archive = celix_bundle_getArchive(b.getCBundle()); - EXPECT_EQ(CELIX_SUCCESS, celix_bundleArchive_getLastModified(archive, &installTime)); - }).build(); + //uninstall and reinstall + ctx->uninstallBundle(bndId1); 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); @@ -99,18 +89,7 @@ TEST_F(CxxBundleArchiveTestSuite, BundleArchiveReusedTest) { auto secondBundleRevisionTime = installTime; - 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<std::mutex> lock{m}; - auto *archive = celix_bundle_getArchive(b.getCBundle()); - EXPECT_EQ(CELIX_SUCCESS, celix_bundleArchive_getLastModified(archive, &installTime)); - }).build(); + ctx->uninstallBundle(bndId1); 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/gtest/src/CelixBundleContextBundlesTestSuite.cc b/libs/framework/gtest/src/CelixBundleContextBundlesTestSuite.cc index 2d8ae49c..1fa35982 100644 --- a/libs/framework/gtest/src/CelixBundleContextBundlesTestSuite.cc +++ b/libs/framework/gtest/src/CelixBundleContextBundlesTestSuite.cc @@ -28,7 +28,6 @@ #include <celix_log_utils.h> #include "celix_api.h" -#include "celix_file_utils.h" class CelixBundleContextBundlesTestSuite : public ::testing::Test { public: @@ -153,29 +152,6 @@ 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<char **>(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<char **>(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<char **>(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)); @@ -189,14 +165,6 @@ 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); diff --git a/libs/framework/src/framework.c b/libs/framework/src/framework.c index df4bb7b4..44662850 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, false); + celix_framework_uninstallBundleEntry(fw, entry); } 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* fw, celix_framework_bundle_entry_t* bndEntry, bool permanent) { - assert(!celix_framework_isCurrentThreadTheEventLoop(fw)); +celix_status_t celix_framework_uninstallBundleEntry(celix_framework_t* framework, celix_framework_bundle_entry_t* bndEntry) { + assert(!celix_framework_isCurrentThreadTheEventLoop(framework)); celix_bundle_state_e bndState = celix_bundle_getState(bndEntry->bnd); if (bndState == CELIX_BUNDLE_STATE_ACTIVE) { - celix_framework_stopBundleEntry(fw, bndEntry); + celix_framework_stopBundleEntry(framework, bndEntry); } - celix_framework_bundle_entry_t* removedEntry = fw_bundleEntry_removeBundleEntryAndIncreaseUseCount(fw, bndEntry->bndId); + celix_framework_bundle_entry_t* removedEntry = fw_bundleEntry_removeBundleEntryAndIncreaseUseCount(framework, bndEntry->bndId); celix_framework_bundleEntry_decreaseUseCount(bndEntry); if (removedEntry != NULL) { @@ -1906,32 +1906,35 @@ celix_status_t celix_framework_uninstallBundleEntry(celix_framework_t* fw, celix 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(fw, OSGI_FRAMEWORK_BUNDLE_EVENT_UNRESOLVED, removedEntry)); + + CELIX_DO_IF(status, fw_fireBundleEvent(framework, OSGI_FRAMEWORK_BUNDLE_EVENT_UNRESOLVED, removedEntry)); + status = CELIX_DO_IF(status, bundle_setState(bnd, CELIX_BUNDLE_STATE_UNINSTALLED)); - CELIX_DO_IF(status, fw_fireBundleEvent(fw, OSGI_FRAMEWORK_BUNDLE_EVENT_UNINSTALLED, removedEntry)); + + CELIX_DO_IF(status, fw_fireBundleEvent(framework, 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(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)); + 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)); status = CELIX_DO_IF(status, bundle_destroy(bnd)); } } - framework_logIfError(fw->logger, status, "", "Cannot uninstall bundle"); + framework_logIfError(framework->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 669778d8..c0851943 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, true); + celix_framework_uninstallBundleEntry(handler->framework, handler->bndEntry); 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, true); + return celix_framework_uninstallBundleEntry(fw, bndEntry); } } diff --git a/libs/framework/src/framework_private.h b/libs/framework/src/framework_private.h index 8d60d4c7..47b22c8b 100644 --- a/libs/framework/src/framework_private.h +++ b/libs/framework/src/framework_private.h @@ -40,7 +40,6 @@ #include "celix_threads.h" #include "service_registry.h" -#include <stdbool.h> #ifndef CELIX_FRAMEWORK_DEFAULT_STATIC_EVENT_QUEUE_SIZE #define CELIX_FRAMEWORK_DEFAULT_STATIC_EVENT_QUEUE_SIZE 1024 @@ -427,7 +426,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, bool permanent); +celix_status_t celix_framework_uninstallBundleEntry(celix_framework_t* fw, celix_framework_bundle_entry_t* bndEntry); /** * Uninstall a bundle. Cannot be called on the Celix event thread.
