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

pnoltes pushed a commit to branch hotfix/dm_all_component_active_segfault
in repository https://gitbox.apache.org/repos/asf/celix.git

commit 205321eb881c7554f4ffbed31d3d9bb188cd47b1
Author: Pepijn Noltes <[email protected]>
AuthorDate: Tue May 23 19:33:15 2023 +0200

    Add fix to handle dm cmp allActive / createInfo for installed but not 
started bundles
---
 .../src/CelixBundleContextBundlesTestSuite.cc      | 36 ++++++++++++++++++-
 libs/framework/include/celix_dependency_manager.h  | 42 +++++++++++++---------
 libs/framework/src/dm_dependency_manager_impl.c    | 17 +++++++--
 3 files changed, 74 insertions(+), 21 deletions(-)

diff --git a/libs/framework/gtest/src/CelixBundleContextBundlesTestSuite.cc 
b/libs/framework/gtest/src/CelixBundleContextBundlesTestSuite.cc
index 1fa35982..cf9c08bf 100644
--- a/libs/framework/gtest/src/CelixBundleContextBundlesTestSuite.cc
+++ b/libs/framework/gtest/src/CelixBundleContextBundlesTestSuite.cc
@@ -631,7 +631,7 @@ TEST_F(CelixBundleContextBundlesTestSuite, 
TestBundleStateToString) {
     EXPECT_STREQ(result, "STOPPING");
 
     result = celix_bundleState_getName(CELIX_BUNDLE_STATE_ACTIVE);
-    EXPECT_STREQ(result, "ACTIVE");
+    EXPECT_STREQ(result, "ACTIVE"); 
 
     result = celix_bundleState_getName(OSGI_FRAMEWORK_BUNDLE_UNKNOWN);
     EXPECT_STREQ(result, "UNKNOWN");
@@ -658,6 +658,40 @@ TEST_F(CelixBundleContextBundlesTestSuite, 
TestBundleStateToString) {
     EXPECT_STREQ(result, "UNKNOWN");
 }
 
+TEST_F(CelixBundleContextBundlesTestSuite, 
UsingDmFunctionsWithInstalledBundlesTest) {
+    //Given a clean framework and a fw dependency manager
+    auto* dm = celix_bundleContext_getDependencyManager(ctx);
+
+    //Then all components are active (no bundles installed yet)
+    bool allActive = celix_dependencyManager_allComponentsActive(dm);
+    EXPECT_TRUE(allActive);
+
+    //When installing a bundle
+    long bndId = celix_bundleContext_installBundle(ctx, TEST_BND1_LOC, false);
+    EXPECT_GE(bndId, 0);
+
+    //Then all compnents are still active (bundle not started and no 
components in bundle) 
+    allActive = celix_dependencyManager_allComponentsActive(dm);
+    EXPECT_TRUE(allActive);
+
+    //And a dm info can be received for the fw bundle
+    auto* info = celix_dependencyManager_createInfo(dm, 
CELIX_FRAMEWORK_BUNDLE_ID);
+    EXPECT_NE(info, nullptr);
+    celix_dependencyManager_destroyInfo(dm, info);
+
+    //But a valid dm info cannot be received for the installed bundle (not 
started yet)
+    info = celix_dependencyManager_createInfo(dm, bndId);
+    EXPECT_EQ(info, nullptr);
+    celix_dependencyManager_destroyInfo(dm, info); //should be safe
+
+    //And infos can be received and only contains dm info for 1 bundle (the 
framework bundle)
+    auto* infos = celix_dependencyManager_createInfos(dm);
+    EXPECT_NE(infos, nullptr);
+    EXPECT_EQ(1, celix_arrayList_size(infos));
+    celix_dependencyManager_destroyInfos(dm, infos);
+}
+
+
 class CelixBundleContextTempBundlesTestSuite : public ::testing::Test {
 public:
     celix_framework_t* fw = nullptr;
diff --git a/libs/framework/include/celix_dependency_manager.h 
b/libs/framework/include/celix_dependency_manager.h
index b8c9c330..4625fa5d 100644
--- a/libs/framework/include/celix_dependency_manager.h
+++ b/libs/framework/include/celix_dependency_manager.h
@@ -31,6 +31,8 @@ extern "C" {
 #endif
 
 /**
+ * @file celix_dependency_manager.h
+ * 
  * The `celix_dependencyManager_add`, `celix_dependencyManager_remove` and 
`celix_dependencyManager_removeAllComponents`
  * funcions for celix_dependency_manager_t should be called outside the Celix 
event thread.
  * Note that bundle activators are started and stopped outside the Celix event 
thread and thus these
@@ -43,7 +45,7 @@ extern "C" {
 
 
 /**
- * Adds a DM component to the dependency manager
+ * @brief Add a DM component to the dependency manager
  *
  * After this call the components will be build and if the components can be 
started, they
  * will be started and the services will be registered.
@@ -56,14 +58,14 @@ extern "C" {
 CELIX_FRAMEWORK_EXPORT celix_status_t 
celix_dependencyManager_add(celix_dependency_manager_t *manager, 
celix_dm_component_t *component);
 
 /**
- * Same as celix_dependencyManager_add, but this call will not wait until all 
service registrations and
+ * @brief Same as celix_dependencyManager_add, but this call will not wait 
until all service registrations and
  * tracker are registered/opened on the Celix event thread.
  * Can be called on the Celix event thread.
  */
 CELIX_FRAMEWORK_EXPORT celix_status_t 
celix_dependencyManager_addAsync(celix_dependency_manager_t *manager, 
celix_dm_component_t *component);
 
 /**
- * Removes a DM component from the dependency manager and destroys it
+ * @brief Remove a DM component from the dependency manager and destroys it
  *
  * After this call - and if the component was found - the component will be 
destroyed and if the component was started,
  * the component will have been stopped and de-initialized.
@@ -73,7 +75,7 @@ CELIX_FRAMEWORK_EXPORT celix_status_t 
celix_dependencyManager_addAsync(celix_dep
 CELIX_FRAMEWORK_EXPORT celix_status_t 
celix_dependencyManager_remove(celix_dependency_manager_t *manager, 
celix_dm_component_t *component);
 
 /**
- * Same as celix_dependencyManager_remove, but this call will not wait until 
component is deactivated.
+ * @brief Same as celix_dependencyManager_remove, but this call will not wait 
until component is deactivated.
  * Can be called on the Celix event thread.
  *
  * The doneCallback will be called (if not NULL) with doneData as argument 
when the component is removed
@@ -87,14 +89,14 @@ CELIX_FRAMEWORK_EXPORT celix_status_t 
celix_dependencyManager_removeAsync(
         void (*doneCallback)(void* data));
 
 /**
- * Removes all DM components from the dependency manager.
+ * @brief Remove all DM components from the dependency manager.
  *
  * Should not be called from the Celix event thread.
  */
 CELIX_FRAMEWORK_EXPORT celix_status_t 
celix_dependencyManager_removeAllComponents(celix_dependency_manager_t 
*manager);
 
 /**
- * Same as celix_dependencyManager_removeAllComponents, but this call will not 
wait til all
+ * @brief Same as celix_dependencyManager_removeAllComponents, but this call 
will not wait til all
  * service registration and service trackers are unregistered/closed.
  *
  * The doneCallback will be called (if not NULL) with doneData as argument 
when the all component are removed
@@ -105,22 +107,23 @@ CELIX_FRAMEWORK_EXPORT celix_status_t 
celix_dependencyManager_removeAllComponent
 CELIX_FRAMEWORK_EXPORT celix_status_t 
celix_dependencyManager_removeAllComponentsAsync(celix_dependency_manager_t 
*manager, void *doneData, void (*doneCallback)(void *data));
 
 /**
- * Check if all components for the bundle of the dependency manager are active 
(all required dependencies resolved).
+ * @brief Check if all components for the bundle of the dependency manager are 
active (all required dependencies resolved).
  */
 CELIX_FRAMEWORK_EXPORT bool 
celix_dependencyManager_areComponentsActive(celix_dependency_manager_t 
*manager);
 
 /**
- * Check if all components - for all bundles - are active (all required 
dependencies resolved).
+ * @brief Check if all components - for all bundles - are active (all required 
dependencies resolved).
  */
 CELIX_FRAMEWORK_EXPORT bool 
celix_dependencyManager_allComponentsActive(celix_dependency_manager_t 
*manager);
 
 /**
- * Return the nr of components for this dependency manager
+ * @brief Return the nr of components for this dependency manager
  */
 CELIX_FRAMEWORK_EXPORT size_t 
celix_dependencyManager_nrOfComponents(celix_dependency_manager_t *manager);
 
 /**
- * Wait for an empty Celix event queue.
+ * @brief Wait for an empty Celix event queue.
+ * 
  * Should not be called on the Celix event queue thread.
  *
  * Can be used to ensure that all created/updated components are completely 
processed (services registered
@@ -129,19 +132,22 @@ CELIX_FRAMEWORK_EXPORT size_t 
celix_dependencyManager_nrOfComponents(celix_depen
 CELIX_FRAMEWORK_EXPORT void 
celix_dependencyManager_wait(celix_dependency_manager_t* manager);
 
 /**
- * Create and returns a dependency manager info struct for the specified 
bundle.
+ * @brief Create and returns a dependency manager info struct for the 
specified bundle.
+ * 
  * The dependency manager info contains information about the state of the 
dependency manager components
  *
  * Caller has ownership of the return value (use 
celix_dependencyManager_destroyInfo to free the memory).
  *
  * @param manager The dependency manager
  * @param bndId The bundle id to get the info from.
- * @returns The dependency manager info for the provided bundle id or NULL if 
the bundle id is invalid.
+ * @returns The dependency manager info for the provided bundle id or NULL if 
the bundle id is invalid or the bundle of
+ *          the provided bundle id is not started.
  */
 CELIX_FRAMEWORK_EXPORT celix_dependency_manager_info_t* 
celix_dependencyManager_createInfo(celix_dependency_manager_t *manager, long 
bndId);
 
 /**
- * Create and returns a dependency manager info struct for all started bundles.
+ * @brief Create and returns a dependency manager info struct for all started 
bundles.
+ * 
  * The dependency manager info contains information about the state of the 
dependency manager components
  *
  * Caller has ownership of the return values (use celix_arrayList_destroy to 
free the memory).
@@ -152,18 +158,19 @@ CELIX_FRAMEWORK_EXPORT celix_dependency_manager_info_t* 
celix_dependencyManager_
 CELIX_FRAMEWORK_EXPORT celix_array_list_t * /*celix_dependency_manager_info_t* 
entries*/ celix_dependencyManager_createInfos(celix_dependency_manager_t 
*manager);
 
 /**
- * Destroys a DM info struct.
+ * @brief Destroy a DM info struct.
  */
 CELIX_FRAMEWORK_EXPORT void 
celix_dependencyManager_destroyInfo(celix_dependency_manager_t *manager, 
celix_dependency_manager_info_t *info);
 
 /**
- * Destroys a celix array list of  DM info structs.
+ * @brief Destroy a celix array list of  DM info structs.
  * @deprecated use celix_arrayList_destroy instead.
  */
 CELIX_FRAMEWORK_EXPORT void 
celix_dependencyManager_destroyInfos(celix_dependency_manager_t *manager, 
celix_array_list_t * infos /*entries celix_dependency_manager_info_t*/);
 
 /**
- * Print the dependency manager info for all bundles to the provided output 
stream.
+ * @brief Print the dependency manager info for all bundles to the provided 
output stream.
+ * 
  * @param manager The dependency manager.
  * @param fullInfo Whether to print the full info or summary.
  * @param useAnsiColors Whether to use ansi colors when printing info.
@@ -172,7 +179,8 @@ CELIX_FRAMEWORK_EXPORT void 
celix_dependencyManager_destroyInfos(celix_dependenc
 CELIX_FRAMEWORK_EXPORT void 
celix_dependencyManager_printInfo(celix_dependency_manager_t* manager, bool 
fullInfo, bool useAnsiColors, FILE* stream);
 
 /**
- * Print the dependency manager info for the provided bundle id to the 
provided output stream.
+ * @brief Print the dependency manager info for the provided bundle id to the 
provided output stream.
+ * 
  * @param manager The dependency manager.
  * @param fullInfo whether to print the full info or summary.
  * @param useAnsiColors Whether to use ansi colors when printing info.
diff --git a/libs/framework/src/dm_dependency_manager_impl.c 
b/libs/framework/src/dm_dependency_manager_impl.c
index 1de53a89..21a77d45 100644
--- a/libs/framework/src/dm_dependency_manager_impl.c
+++ b/libs/framework/src/dm_dependency_manager_impl.c
@@ -182,6 +182,10 @@ celix_status_t 
celix_dependencyManager_removeAllComponentsAsync(celix_dependency
 static void celix_dm_getInfoCallback(void *handle, const celix_bundle_t *bnd) {
     celix_dependency_manager_info_t **out = handle;
 
+    if (celix_bundle_getState(bnd) != CELIX_BUNDLE_STATE_ACTIVE) {
+        return;
+    }
+
     celix_bundle_context_t *context = NULL;
     bundle_getContext((celix_bundle_t*)bnd, &context);
     celix_dependency_manager_t *mng = 
celix_bundleContext_getDependencyManager(context);
@@ -261,6 +265,11 @@ celix_array_list_t * 
celix_dependencyManager_createInfos(celix_dependency_manage
 
 static void celix_dm_allComponentsActiveCallback(void *handle, const 
celix_bundle_t *bnd) {
        bool *allActivePtr = handle;
+
+    if (celix_bundle_getState(bnd) != CELIX_BUNDLE_STATE_ACTIVE) {
+        return;
+    }
+
        celix_bundle_context_t *context = NULL;
        bundle_getContext((celix_bundle_t*)bnd, &context);
        celix_dependency_manager_t *mng = 
celix_bundleContext_getDependencyManager(context);
@@ -310,9 +319,11 @@ bool 
celix_dependencyManager_areComponentsActive(celix_dependency_manager_t *mng
 }
 
 void celix_dependencyManager_destroyInfo(celix_dependency_manager_t *manager 
__attribute__((unused)), celix_dependency_manager_info_t *info) {
-       celix_arrayList_destroy(info->components);
-       free(info->bndSymbolicName);
-       free(info);
+    if (info != NULL) {
+        celix_arrayList_destroy(info->components);
+        free(info->bndSymbolicName);
+        free(info);
+    }
 }
 
 

Reply via email to