This is an automated email from the ASF dual-hosted git repository.

pengzheng pushed a commit to branch feature/556-osgi-uninstall
in repository https://gitbox.apache.org/repos/asf/celix.git

commit cccf30aac326a919727f0e164a474004921b9756
Author: PengZheng <[email protected]>
AuthorDate: Fri Jun 2 23:00:57 2023 +0800

    Add full support of bundle update.
    
    It fixes #563 and #557.
---
 .../BundleArchiveWithErrorInjectionTestSuite.cc    |  42 +-----
 .../src/CelixBundleContextBundlesTestSuite.cc      |  33 +++++
 .../framework/gtest/src/CelixFrameworkTestSuite.cc |  18 ++-
 libs/framework/include/celix_bundle_context.h      |  14 +-
 libs/framework/include/celix_framework.h           |  32 +++--
 libs/framework/src/bundle.c                        |  26 +---
 libs/framework/src/bundle_archive.c                |  39 +----
 libs/framework/src/bundle_revision.c               |  27 ++--
 libs/framework/src/framework.c                     | 157 ++++++++++-----------
 .../src/framework_bundle_lifecycle_handler.c       |   6 +-
 libs/framework/src/framework_private.h             |   4 +-
 11 files changed, 168 insertions(+), 230 deletions(-)

diff --git 
a/libs/framework/gtest/src/BundleArchiveWithErrorInjectionTestSuite.cc 
b/libs/framework/gtest/src/BundleArchiveWithErrorInjectionTestSuite.cc
index 6fc2118f..d0363d02 100644
--- a/libs/framework/gtest/src/BundleArchiveWithErrorInjectionTestSuite.cc
+++ b/libs/framework/gtest/src/BundleArchiveWithErrorInjectionTestSuite.cc
@@ -229,44 +229,4 @@ TEST_F(CelixBundleArchiveErrorInjectionTestSuite, 
ArchiveCreateErrorTest) {
     teardownErrorInjectors();
 
     EXPECT_EQ(CELIX_SUCCESS, celix_bundleCache_destroy(cache));
-}
-
-TEST_F(CelixBundleArchiveErrorInjectionTestSuite, ArchiveReviseErrorTest) {
-    celix_bundle_cache_t* cache = nullptr;
-    createCache(&cache);
-    bundle_archive_t* archive = nullptr;
-
-    // revision creation failure
-    EXPECT_EQ(CELIX_SUCCESS,
-              celix_bundleArchive_create(&fw, TEST_ARCHIVE_ROOT, 1, 
SIMPLE_TEST_BUNDLE1_LOCATION, &archive));
-    EXPECT_NE(nullptr, archive);
-    celix_ei_expect_calloc((void*)celix_bundleRevision_create, 0, nullptr);
-    EXPECT_EQ(CELIX_ENOMEM, bundleArchive_revise(archive, 
SIMPLE_TEST_BUNDLE1_LOCATION, nullptr));
-    EXPECT_EQ(CELIX_SUCCESS, bundleArchive_destroy(archive));
-    EXPECT_EQ(CELIX_SUCCESS, celix_utils_deleteDirectory(TEST_ARCHIVE_ROOT, 
nullptr));
-    teardownErrorInjectors();
-
-    // manifest clone failure
-    EXPECT_EQ(CELIX_SUCCESS,
-              celix_bundleArchive_create(&fw, TEST_ARCHIVE_ROOT, 1, 
SIMPLE_TEST_BUNDLE1_LOCATION, &archive));
-    EXPECT_NE(nullptr, archive);
-    celix_ei_expect_malloc((void*)manifest_clone, 1, nullptr);
-    EXPECT_EQ(CELIX_ENOMEM, bundleArchive_revise(archive, 
SIMPLE_TEST_BUNDLE1_LOCATION, nullptr));
-    EXPECT_EQ(CELIX_SUCCESS, bundleArchive_destroy(archive));
-    EXPECT_EQ(CELIX_SUCCESS, celix_utils_deleteDirectory(TEST_ARCHIVE_ROOT, 
nullptr));
-    teardownErrorInjectors();
-
-    // extract zip data failure
-    EXPECT_EQ(CELIX_SUCCESS,
-              celix_bundleArchive_create(&fw, TEST_ARCHIVE_ROOT, 1, 
SIMPLE_TEST_BUNDLE1_LOCATION, &archive));
-    EXPECT_NE(nullptr, archive);
-    
celix_ei_expect_celix_utils_extractZipFile((void*)celix_framework_utils_extractBundle,
 1, CELIX_FILE_IO_EXCEPTION);
-    usleep(10000); // sleep to ensure that the last modified time is different 
(otherwise the revision is not updated
-    celix_utils_touch(SIMPLE_TEST_BUNDLE1_LOCATION);
-    EXPECT_EQ(CELIX_FILE_IO_EXCEPTION, bundleArchive_revise(archive, 
SIMPLE_TEST_BUNDLE1_LOCATION, nullptr));
-    EXPECT_EQ(CELIX_SUCCESS, bundleArchive_destroy(archive));
-    EXPECT_EQ(CELIX_SUCCESS, celix_utils_deleteDirectory(TEST_ARCHIVE_ROOT, 
nullptr));
-    teardownErrorInjectors();
-
-    EXPECT_EQ(CELIX_SUCCESS, celix_bundleCache_destroy(cache));
-}
+}
\ No newline at end of file
diff --git a/libs/framework/gtest/src/CelixBundleContextBundlesTestSuite.cc 
b/libs/framework/gtest/src/CelixBundleContextBundlesTestSuite.cc
index f7672087..0d222bb7 100644
--- a/libs/framework/gtest/src/CelixBundleContextBundlesTestSuite.cc
+++ b/libs/framework/gtest/src/CelixBundleContextBundlesTestSuite.cc
@@ -286,6 +286,39 @@ TEST_F(CelixBundleContextBundlesTestSuite, 
InstallAndUnloadBundlesTest) {
     ASSERT_TRUE(bndId3 == bndId6); //bundle cache -> reuse of bundle id.
 }
 
+TEST_F(CelixBundleContextBundlesTestSuite, UpdateBundlesTest) {
+    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));
+
+    ASSERT_TRUE(celix_bundleContext_updateBundle(ctx, bndId1, NULL));
+    ASSERT_TRUE(celix_bundleContext_isBundleInstalled(ctx, bndId1));
+    ASSERT_TRUE(celix_bundleContext_isBundleActive(ctx, bndId1));
+
+    ASSERT_TRUE(celix_bundleContext_stopBundle(ctx, bndId1));
+    ASSERT_TRUE(celix_bundleContext_updateBundle(ctx, bndId1, NULL));
+    ASSERT_TRUE(celix_bundleContext_isBundleInstalled(ctx, bndId1));
+    ASSERT_FALSE(celix_bundleContext_isBundleActive(ctx, bndId1));
+
+    long bndId2 = celix_bundleContext_installBundle(ctx, TEST_BND2_LOC, false);
+    ASSERT_TRUE(bndId2 >= 0L);
+    ASSERT_FALSE(celix_bundleContext_updateBundle(ctx, bndId1, TEST_BND2_LOC));
+    ASSERT_TRUE(celix_bundleContext_isBundleInstalled(ctx, bndId1));
+    ASSERT_EQ(bndId2, celix_bundleContext_installBundle(ctx, TEST_BND2_LOC, 
false));
+    ASSERT_TRUE(celix_bundleContext_uninstallBundle(ctx, bndId2));
+
+    auto sn1 = celix_bundleContext_getBundleSymbolicName(ctx, bndId1);
+    ASSERT_TRUE(celix_bundleContext_updateBundle(ctx, bndId1, TEST_BND2_LOC));
+    auto sn2 = celix_bundleContext_getBundleSymbolicName(ctx, bndId1);
+    ASSERT_STRNE(sn1, sn2);
+    free(sn2);
+    free(sn1);
+
+    ASSERT_TRUE(celix_bundleContext_unloadBundle(ctx, bndId1));
+    ASSERT_FALSE(celix_bundleContext_updateBundle(ctx, bndId1, NULL));
+}
+
 TEST_F(CelixBundleContextBundlesTestSuite, StartBundleWithException) {
     long bndId = celix_bundleContext_installBundle(ctx, 
TEST_BND_WITH_EXCEPTION_LOC, true);
     ASSERT_TRUE(bndId > 0); //bundle is installed, but not started
diff --git a/libs/framework/gtest/src/CelixFrameworkTestSuite.cc 
b/libs/framework/gtest/src/CelixFrameworkTestSuite.cc
index 89c20f61..95e7d434 100644
--- a/libs/framework/gtest/src/CelixFrameworkTestSuite.cc
+++ b/libs/framework/gtest/src/CelixFrameworkTestSuite.cc
@@ -79,17 +79,29 @@ TEST_F(CelixFrameworkTestSuite, EventQueueTest) {
     EXPECT_EQ(4, count);
 }
 
-TEST_F(CelixFrameworkTestSuite, AsyncInstallStartStopAndUninstallBundleTest) {
+TEST_F(CelixFrameworkTestSuite, 
AsyncInstallStartStopUpdateAndUninstallBundleTest) {
     long bndId = celix_framework_installBundleAsync(framework.get(), 
SIMPLE_TEST_BUNDLE1_LOCATION, false);
     EXPECT_GE(bndId, 0);
     EXPECT_TRUE(celix_framework_isBundleInstalled(framework.get(), bndId));
     EXPECT_FALSE(celix_framework_isBundleActive(framework.get(), bndId));
 
-    celix_framework_startBundle(framework.get(), bndId);
+    celix_framework_updateBundleAsync(framework.get(), bndId, NULL);
+    std::this_thread::sleep_for(std::chrono::milliseconds{100});
+    EXPECT_FALSE(celix_framework_isBundleActive(framework.get(), bndId));
+
+    celix_framework_startBundleAsync(framework.get(), bndId);
     std::this_thread::sleep_for(std::chrono::milliseconds{100});
     EXPECT_TRUE(celix_framework_isBundleActive(framework.get(), bndId));
 
-    celix_framework_stopBundle(framework.get(), bndId);
+    celix_framework_updateBundleAsync(framework.get(), bndId, NULL);
+    std::this_thread::sleep_for(std::chrono::milliseconds{100});
+    EXPECT_TRUE(celix_framework_isBundleActive(framework.get(), bndId));
+
+    celix_framework_stopBundleAsync(framework.get(), bndId);
+    std::this_thread::sleep_for(std::chrono::milliseconds{100});
+    EXPECT_FALSE(celix_framework_isBundleActive(framework.get(), bndId));
+
+    celix_framework_updateBundleAsync(framework.get(), bndId, NULL);
     std::this_thread::sleep_for(std::chrono::milliseconds{100});
     EXPECT_FALSE(celix_framework_isBundleActive(framework.get(), bndId));
 
diff --git a/libs/framework/include/celix_bundle_context.h 
b/libs/framework/include/celix_bundle_context.h
index fc73b180..3d3d71e1 100644
--- a/libs/framework/include/celix_bundle_context.h
+++ b/libs/framework/include/celix_bundle_context.h
@@ -952,22 +952,23 @@ CELIX_FRAMEWORK_EXPORT bool 
celix_bundleContext_stopBundle(celix_bundle_context_
 CELIX_FRAMEWORK_EXPORT bool 
celix_bundleContext_startBundle(celix_bundle_context_t *ctx, long bndId);
 
 /**
- * @brief Update the bundle with the provided bundle id async.
+ * @brief Update the bundle with the provided bundle id.
  *
  * This will do the following:
- *  - stop the bundle (if needed);
- *  - update the bundle revision if a newer bundle zip if found;
- *  - start the bundle, if it was started before the update.
+ *  - uninstall the bundle with the specified bundle id;
+ *  - reinstall the bundle from the specified location with the specified 
bundle id;
+ *  - start the bundle, if it was previously active.
  *
  * Will silently ignore bundle ids < 0.
  *
+ * Note if specified bundle location already exists in the bundle cache but 
with a different bundle id, the bundle
+ * will NOT be reinstalled, and the update is cancelled.
+ *
  * If this function is called on the Celix event thread, the actual updating 
of the bundle will be done async and
  * on a separate thread.
  * If this function is called from a different thread than the Celix event 
thread, then the function will
  * return after the bundle update is completed.
  *
- * @warning Update bundle is not yet fully supported. Use at your own risk.
- *
  * @param ctx The bundle context
  * @param bndId The bundle id to update.
  * @param updatedBundleUrl The optional updated bundle url to the bundle zip 
file. If NULL, the existing bundle url
@@ -986,7 +987,6 @@ CELIX_FRAMEWORK_EXPORT bool 
celix_bundleContext_updateBundle(celix_bundle_contex
  */
 CELIX_FRAMEWORK_EXPORT char* 
celix_bundleContext_getBundleSymbolicName(celix_bundle_context_t *ctx, long 
bndId);
 
-
 /**
  * @brief Track bundles.
  *
diff --git a/libs/framework/include/celix_framework.h 
b/libs/framework/include/celix_framework.h
index 5f3d338a..ad833ce5 100644
--- a/libs/framework/include/celix_framework.h
+++ b/libs/framework/include/celix_framework.h
@@ -186,18 +186,19 @@ CELIX_FRAMEWORK_EXPORT bool 
celix_framework_unloadBundle(celix_framework_t *fw,
  * @brief Update the bundle with the provided bundle id.
  *
  * This will do the following:
- *  - stop the bundle (if needed);
- *  - update the bundle revision if a newer bundle zip if found;
- *  - start the bundle, if it was started.
+ *  - uninstall the bundle with the specified bundle id;
+ *  - reinstall the bundle from the specified location with the specified 
bundle id;
+ *  - start the bundle, if it was previously active.
  *
  *  Will silently ignore bundle ids < 0.
  *
- * @warning Update bundle is not yet fully supported. Use at your own risk.
+ *  Note if specified bundle location already exists in the bundle cache but 
with a different bundle id, the bundle
+ *  will NOT be reinstalled, and the update is cancelled.
  *
- * @param fw The Celix framework
- * @parma bndId the bundle id to update.
- * @param updatedBundleUrl The optional updated bundle url to the bundle zip 
file. If NULL, the existing bundle url
- *                         from the bundle cache will be used.
+ * @param [in] fw The Celix framework
+ * @param [in] bndId the bundle id to update.
+ * @param [in] updatedBundleUrl The optional updated bundle url to the bundle 
zip file.
+ * If NULL, the existing bundle url from the bundle cache will be used.
  * @return true if the bundle is correctly updated. False if not.
  */
 CELIX_FRAMEWORK_EXPORT bool celix_framework_updateBundle(celix_framework_t 
*fw, long bndId, const char* updatedBundleUrl);
@@ -239,17 +240,18 @@ CELIX_FRAMEWORK_EXPORT long 
celix_framework_installBundleAsync(celix_framework_t
  * @brief Update the bundle with the provided bundle id async.
  *
  * This will do the following:
- *  - stop the bundle (if needed);
- *  - update the bundle revision if a newer bundle zip if found;
- *  - start the bundle, if it was started.
+ *  - uninstall the bundle with the specified bundle id;
+ *  - reinstall the bundle from the specified location with the specified 
bundle id;
+ *  - start the bundle, if it was previously active.
  *
  *  Will silently ignore bundle ids < 0.
  *
- *  @warning Update bundle is not yet fully supported. Use at your own risk.
+ *  Note if specified bundle location already exists in the bundle cache but 
with a different bundle id, the bundle
+ *  will NOT be reinstalled, and the update is cancelled.
  *
- *  @param fw The Celix framework
- *  @parma bndId the bundle id to update.
- *  @param updatedBundleUrl The optional updated bundle url to the bundle zip 
file. If NULL, the existing bundle url
+ * @param [in] fw The Celix framework
+ * @param [in] bndId the bundle id to update.
+ * @param [in] updatedBundleUrl The optional updated bundle url to the bundle 
zip file. If NULL, the existing bundle url
  *                         from the bundle cache will be used.
  */
 CELIX_FRAMEWORK_EXPORT void 
celix_framework_updateBundleAsync(celix_framework_t *fw, long bndId, const 
char* updatedBundleUrl);
diff --git a/libs/framework/src/bundle.c b/libs/framework/src/bundle.c
index 64eaa241..083a6944 100644
--- a/libs/framework/src/bundle.c
+++ b/libs/framework/src/bundle.c
@@ -280,30 +280,8 @@ celix_status_t 
bundle_setPersistentStateUninstalled(bundle_pt bundle) {
 }
 
 celix_status_t bundle_revise(bundle_pt bundle, const char * location, const 
char *inputFile) {
-       celix_status_t status;
-
-       bundle_archive_pt archive = NULL;
-       status = bundle_getArchive(bundle, &archive);
-       if (status == CELIX_SUCCESS) {
-               status = bundleArchive_revise(archive, location, inputFile);
-               if (status == CELIX_SUCCESS) {
-                       module_pt module;
-                       status = bundle_createModule(bundle, &module);
-                       if (status == CELIX_SUCCESS) {
-                               status = bundle_addModule(bundle, module);
-                       } else {
-                               bool rolledback;
-                               status = bundleArchive_rollbackRevise(archive, 
&rolledback);
-                               if (status == CELIX_SUCCESS) {
-                                       status = CELIX_BUNDLE_EXCEPTION;
-                               }
-                       }
-               }
-       }
-
-       framework_logIfError(bundle->framework->logger, status, NULL, "Failed 
to revise bundle");
-
-       return status;
+    fw_log(bundle->framework->logger, CELIX_LOG_LEVEL_DEBUG, "Usage of 
bundle_revise is deprecated and no longer needed. Called for bundle %s", 
bundle->symbolicName);
+    return CELIX_SUCCESS;
 }
 
 celix_status_t bundle_addModule(bundle_pt bundle, module_pt module) {
diff --git a/libs/framework/src/bundle_archive.c 
b/libs/framework/src/bundle_archive.c
index aec52ce4..818d03a6 100644
--- a/libs/framework/src/bundle_archive.c
+++ b/libs/framework/src/bundle_archive.c
@@ -433,46 +433,11 @@ celix_status_t 
bundleArchive_setLastModified(bundle_archive_pt archive __attribu
     celix_utils_freeStringIfNotEqual(manifestPathBuffer, manifestPath);
     return status;
 }
-//LCOV_EXCL_STOP
 
 celix_status_t bundleArchive_revise(bundle_archive_pt archive, const char * 
location __attribute__((unused)), const char *updatedBundleUrl) {
-    celixThreadMutex_lock(&archive->lock);
-
-    const char* updateUrl = archive->location;
-    if (updatedBundleUrl != NULL && strcmp(updateUrl, updatedBundleUrl) != 0) {
-        fw_log(archive->fw->logger, CELIX_LOG_LEVEL_INFO, "Updating bundle 
archive bundle url location to %s", updatedBundleUrl);
-        updateUrl = updatedBundleUrl;
-    }
-
-    const char* reason = NULL;
-    celix_status_t status = celix_bundleArchive_extractBundle(archive, 
updateUrl);
-    if (status == CELIX_SUCCESS) {
-        bundle_revision_t* current = archive->revision;
-        bundle_revision_t* revised = bundleRevision_revise(current, updateUrl);
-        if (revised != NULL) {
-            archive->revision = revised;
-            bundleRevision_destroy(current);
-        } else {
-            status = CELIX_ENOMEM;
-            reason = "bundle revision creation";
-        }
-    } else {
-        reason = "bundle extraction";
-    }
-    if (status != CELIX_SUCCESS) {
-        goto revise_finished;
-    }
-    if (archive->location != updateUrl) {
-        free(archive->location);
-        archive->location = celix_utils_strdup(updateUrl);
-    }
-revise_finished:
-    celixThreadMutex_unlock(&archive->lock);
-    framework_logIfError(archive->fw->logger, status, reason, "Cannot update 
bundle archive %s", updateUrl);
-    return status;
+    fw_log(archive->fw->logger, CELIX_LOG_LEVEL_ERROR, "Revise not 
supported.");
+    return CELIX_BUNDLE_EXCEPTION;
 }
-
-//LCOV_EXCL_START
 celix_status_t bundleArchive_rollbackRevise(bundle_archive_pt archive, bool* 
rolledback) {
     *rolledback = true;
     fw_log(archive->fw->logger, CELIX_LOG_LEVEL_ERROR, "Revise rollback not 
supported.");
diff --git a/libs/framework/src/bundle_revision.c 
b/libs/framework/src/bundle_revision.c
index b0bdbbde..8b0a68b7 100644
--- a/libs/framework/src/bundle_revision.c
+++ b/libs/framework/src/bundle_revision.c
@@ -52,15 +52,6 @@ celix_status_t 
celix_bundleRevision_create(celix_framework_t* fw, const char *ro
     return status;
 }
 
-bundle_revision_t* bundleRevision_revise(const bundle_revision_t* rev, const 
char* updatedBundleUrl) {
-    bundle_revision_pt newRev = NULL;
-    manifest_pt clonedManifest = manifest_clone(rev->manifest);
-    if (clonedManifest) {
-        celix_bundleRevision_create(rev->fw, rev->root, updatedBundleUrl, 
clonedManifest, &newRev);
-    }
-    return newRev;
-}
-
 celix_status_t bundleRevision_destroy(bundle_revision_pt revision) {
     if (revision != NULL) {
         manifest_destroy(revision->manifest);
@@ -71,6 +62,18 @@ celix_status_t bundleRevision_destroy(bundle_revision_pt 
revision) {
     return CELIX_SUCCESS;
 }
 
+celix_status_t bundleRevision_getManifest(const bundle_revision_t* revision, 
manifest_pt* manifest) {
+    *manifest = revision->manifest;
+    return CELIX_SUCCESS;
+}
+
+//LCOV_EXCL_START
+bundle_revision_t* bundleRevision_revise(const bundle_revision_t* rev, const 
char* updatedBundleUrl) {
+    fw_log(rev->fw->logger, CELIX_LOG_LEVEL_ERROR, "Revision revise 
unsupported.");
+    return NULL;
+}
+
+
 celix_status_t bundleRevision_getNumber(const bundle_revision_t* revision, 
long *revisionNr) {
     *revisionNr = 1; //note revision nr is deprecated
     return CELIX_SUCCESS;
@@ -86,11 +89,6 @@ celix_status_t bundleRevision_getRoot(const 
bundle_revision_t* revision, const c
     return CELIX_SUCCESS;
 }
 
-celix_status_t bundleRevision_getManifest(const bundle_revision_t* revision, 
manifest_pt* manifest) {
-    *manifest = revision->manifest;
-    return CELIX_SUCCESS;
-}
-
 celix_status_t bundleRevision_getHandles(const bundle_revision_t* revision 
__attribute__((unused)), celix_array_list_t** handles) {
     //nop, usage deprecated
     if (handles) {
@@ -98,4 +96,5 @@ celix_status_t bundleRevision_getHandles(const 
bundle_revision_t* revision __att
     }
     return CELIX_SUCCESS;
 }
+//LCOV_EXCL_STOP
 
diff --git a/libs/framework/src/framework.c b/libs/framework/src/framework.c
index 0c6c31b8..e730133c 100644
--- a/libs/framework/src/framework.c
+++ b/libs/framework/src/framework.c
@@ -239,7 +239,11 @@ celix_status_t framework_create(framework_pt *out, 
celix_properties_t* config) {
     celixThreadMutex_create(&framework->dispatcher.mutex, NULL);
     celixThreadMutex_create(&framework->frameworkListenersLock, NULL);
     celixThreadMutex_create(&framework->bundleListenerLock, NULL);
-    celixThreadMutex_create(&framework->installLock, NULL);
+    celix_thread_mutexattr_t muAttr;
+    celixThreadMutexAttr_create(&muAttr);
+    celixThreadMutexAttr_settype(&muAttr, CELIX_THREAD_MUTEX_RECURSIVE);
+    celixThreadMutex_create(&framework->installLock, &muAttr);
+    celixThreadMutexAttr_destroy(&muAttr);
     celixThreadMutex_create(&framework->installedBundles.mutex, NULL);
     celixThreadCondition_init(&framework->dispatcher.cond, NULL);
     framework->dispatcher.active = true;
@@ -625,7 +629,7 @@ bool 
celix_framework_getConfigPropertyAsBool(celix_framework_t* framework, const
 }
 
 celix_status_t
-celix_framework_installBundleInternal(celix_framework_t* framework, const 
char* bndLoc, long* bundleOut) {
+celix_framework_installBundleInternal(celix_framework_t* framework, const 
char* bndLoc, long* bndId) {
     celix_status_t status = CELIX_SUCCESS;
     celix_bundle_t* bundle = NULL;
     long id = -1L;
@@ -650,16 +654,19 @@ celix_framework_installBundleInternal(celix_framework_t* 
framework, const char*
 
     celixThreadMutex_lock(&framework->installLock);
     if (status == CELIX_SUCCESS) {
-        id = framework_getBundle(framework, bndLoc);
-        if (id != -1L) {
-            celix_framework_bundleEntry_decreaseUseCount(fwBundleEntry);
-            *bundleOut = id;
-            celixThreadMutex_unlock(&framework->installLock);
-            return CELIX_SUCCESS;
+        if (*bndId == -1L) {
+            id = framework_getBundle(framework, bndLoc);
+            if (id != -1L) {
+                celix_framework_bundleEntry_decreaseUseCount(fwBundleEntry);
+                celixThreadMutex_unlock(&framework->installLock);
+                *bndId = id;
+                return CELIX_SUCCESS;
+            }
+            long alreadyExistingBndId = 
celix_bundleCache_findBundleIdForLocation(framework->cache, bndLoc);
+            id = alreadyExistingBndId == -1 ? 
framework_getNextBundleId(framework) : alreadyExistingBndId;
+        } else {
+            id = *bndId;
         }
-
-        long alreadyExistingBndId = 
celix_bundleCache_findBundleIdForLocation(framework->cache, bndLoc);
-        id = alreadyExistingBndId == -1 ? framework_getNextBundleId(framework) 
: alreadyExistingBndId;
         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));
@@ -675,7 +682,7 @@ celix_framework_installBundleInternal(celix_framework_t* 
framework, const char*
     }
 
     if (status == CELIX_SUCCESS) {
-        *bundleOut = id;
+        *bndId = id;
     } else {
         fw_logCode(framework->logger, CELIX_LOG_LEVEL_ERROR, status, "Could 
not install bundle");
     }
@@ -2267,89 +2274,71 @@ static bool 
celix_framework_updateBundleInternal(celix_framework_t *fw, long bnd
         fw_log(fw->logger, CELIX_LOG_LEVEL_INFO, "Cannot update framework 
bundle. Ignore update cmd.");
         return true;
     }
-
-    fw_log(fw->logger, CELIX_LOG_LEVEL_WARNING, "Update bundle is not yet 
fully supported. Use at your own risk.");
-
     bool updated = false;
     celix_framework_bundle_entry_t *bndEntry = 
celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(fw, bndId);
     if (bndEntry != NULL) {
         celix_status_t rc = 
celix_framework_updateBundleOnANonCelixEventThread(fw, bndEntry, 
updatedBundleUrl, forcedAsync);
+        //note not decreasing bndEntry, because this entry should now be 
deleted (uninstalled)
         updated = rc == CELIX_SUCCESS;
-        celix_framework_bundleEntry_decreaseUseCount(bndEntry);
         celix_framework_waitForBundleEvents(fw, bndId);
     }
     return updated;
 }
 
-
-celix_status_t 
celix_framework_bundleEntry_refreshBundleEntry(celix_framework_t* framework, 
celix_framework_bundle_entry_t *entry, const char* updatedBundleUrl) {
+celix_status_t celix_framework_updateBundleEntry(celix_framework_t* framework,
+                                                 
celix_framework_bundle_entry_t* bndEntry,
+                                                 const char* updatedBundleUrl) 
{
     celix_status_t status = CELIX_SUCCESS;
-    celix_bundle_state_e state = celix_bundle_getState(entry->bnd);
-    if (state != CELIX_BUNDLE_STATE_RESOLVED) {
-        fw_log(framework->logger, CELIX_LOG_LEVEL_ERROR, "Cannot refresh 
bundle %s (id=%li), bundle is not in the resolved state",
-               celix_bundle_getSymbolicName(entry->bnd),
-               entry->bndId);
-        return CELIX_ILLEGAL_STATE;
-    }
-
-    bundle_archive_t* archive = NULL;
-    celix_module_t* module = NULL;
-    status = CELIX_DO_IF(status, bundle_getArchive(entry->bnd, &archive));
-    status = CELIX_DO_IF(status, bundle_getCurrentModule(entry->bnd, &module));
-
-    if (!archive || !module) {
-        fw_log(framework->logger, CELIX_LOG_LEVEL_ERROR, "Cannot refresh 
bundle %s (id=%li), bundle has no archive or module",
-               celix_bundle_getSymbolicName(entry->bnd),
-               entry->bndId);
-        return status;
-    }
-
-    status = celix_module_closeLibraries(module);
-    if (status != CELIX_SUCCESS) {
-        fw_log(framework->logger, CELIX_LOG_LEVEL_ERROR, "Cannot refresh 
bundle %s (id=%li), cannot close libraries",
-               celix_bundle_getSymbolicName(entry->bnd),
-               entry->bndId);
-        return status;
-    }
-
-    status = bundleArchive_revise(archive, NULL, updatedBundleUrl);
-    if (status != CELIX_SUCCESS) {
-        fw_log(framework->logger, CELIX_LOG_LEVEL_ERROR, "Cannot refresh 
bundle %s (id=%li), bundle archive revision failed",
-               celix_bundle_getSymbolicName(entry->bnd),
-               entry->bndId);
-        return status;
-    }
-
-    status = celix_module_loadLibraries(module);
-    if (status != CELIX_SUCCESS) {
-        fw_log(framework->logger, CELIX_LOG_LEVEL_ERROR, "Cannot refresh 
bundle %s (id=%li), cannot load libraries",
-               celix_bundle_getSymbolicName(entry->bnd),
-               entry->bndId);
-        return status;
-    }
-
-    return status;
-}
-
-celix_status_t celix_framework_updateBundleEntry(celix_framework_t* framework, 
celix_framework_bundle_entry_t* bndEntry, const char* updatedBundleUrl) {
-    celix_status_t status = celix_framework_stopBundleEntry(framework, 
bndEntry);
-    if (status != CELIX_SUCCESS) {
-        fw_logCode(framework->logger, CELIX_LOG_LEVEL_ERROR, status, "Could 
not stop bundle for update");
-        return status;
-    }
-
-    status = celix_framework_bundleEntry_refreshBundleEntry(framework, 
bndEntry, updatedBundleUrl);
-    if (status != CELIX_SUCCESS) {
-        fw_logCode(framework->logger, CELIX_LOG_LEVEL_ERROR, status, "Could 
not refresh bundle for update");
-        return status;
-    }
-
-    status = celix_framework_startBundleEntry(framework, bndEntry);
-    if (status != CELIX_SUCCESS) {
-        fw_logCode(framework->logger, CELIX_LOG_LEVEL_ERROR, status, "Could 
not start bundle after update");
-    }
-
-    fw_log(framework->logger, CELIX_LOG_LEVEL_DEBUG, "Update bundle %s done", 
celix_bundle_getSymbolicName(bndEntry->bnd));
+    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;
+            }
+        }
+        status = celix_framework_uninstallBundleEntry(framework, bndEntry, 
true);
+        if (status != CELIX_SUCCESS) {
+            errMsg = "uninstall failure";
+            celixThreadMutex_unlock(&framework->installLock);
+            break;
+        }
+        // bndEntry is now invalid
+        status = celix_framework_installBundleInternal(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;
 }
 
diff --git a/libs/framework/src/framework_bundle_lifecycle_handler.c 
b/libs/framework/src/framework_bundle_lifecycle_handler.c
index 6ffa9bb3..40caeb28 100644
--- a/libs/framework/src/framework_bundle_lifecycle_handler.c
+++ b/libs/framework/src/framework_bundle_lifecycle_handler.c
@@ -41,9 +41,11 @@ static void* 
celix_framework_BundleLifecycleHandlingThread(void *data) {
     switch (handler->command) {
         case CELIX_BUNDLE_LIFECYCLE_START:
             celix_framework_startBundleEntry(handler->framework, 
handler->bndEntry);
+            celix_framework_bundleEntry_decreaseUseCount(handler->bndEntry);
             break;
         case CELIX_BUNDLE_LIFECYCLE_STOP:
             celix_framework_stopBundleEntry(handler->framework, 
handler->bndEntry);
+            celix_framework_bundleEntry_decreaseUseCount(handler->bndEntry);
             break;
         case CELIX_BUNDLE_LIFECYCLE_UNINSTALL:
             celix_framework_bundleEntry_decreaseUseCount(handler->bndEntry);
@@ -54,12 +56,10 @@ static void* 
celix_framework_BundleLifecycleHandlingThread(void *data) {
             celix_framework_uninstallBundleEntry(handler->framework, 
handler->bndEntry, false);
             break;
         default: //update
+            celix_framework_bundleEntry_decreaseUseCount(handler->bndEntry);
             celix_framework_updateBundleEntry(handler->framework, 
handler->bndEntry, handler->updatedBundleUrl);
             break;
     }
-    if (handler->command != CELIX_BUNDLE_LIFECYCLE_UNINSTALL && 
handler->command != CELIX_BUNDLE_LIFECYCLE_UNLOAD) {
-        celix_framework_bundleEntry_decreaseUseCount(handler->bndEntry);
-    }
     celix_framework_cleanupBundleLifecycleHandler(handler->framework, handler);
     return NULL;
 }
diff --git a/libs/framework/src/framework_private.h 
b/libs/framework/src/framework_private.h
index c506e94b..d2a36cfc 100644
--- a/libs/framework/src/framework_private.h
+++ b/libs/framework/src/framework_private.h
@@ -253,7 +253,7 @@ double 
celix_framework_getConfigPropertyAsDouble(celix_framework_t* framework, c
  */
 bool celix_framework_getConfigPropertyAsBool(celix_framework_t* framework, 
const char* name, bool defaultValue, bool* found);
 
-celix_status_t celix_framework_installBundleInternal(celix_framework_t* 
framework, const char* bndLoc, long* bundleOut);
+celix_status_t celix_framework_installBundleInternal(celix_framework_t* 
framework, const char* bndLoc, long* bndId);
 
 celix_status_t fw_registerService(framework_pt framework, 
service_registration_pt * registration, long bundleId, const char* serviceName, 
const void* svcObj, properties_pt properties);
 celix_status_t fw_registerServiceFactory(framework_pt framework, 
service_registration_pt * registration, long bundleId, const char* serviceName, 
service_factory_pt factory, properties_pt properties);
@@ -433,7 +433,7 @@ celix_status_t 
celix_framework_stopBundleEntry(celix_framework_t* fw, celix_fram
 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.
+ * Update a bundle. Cannot be called on the Celix event thread.
  */
 celix_status_t celix_framework_updateBundleEntry(celix_framework_t* fw, 
celix_framework_bundle_entry_t* bndEntry, const char* updatedBundleUrl);
 

Reply via email to