pnoltes commented on code in PR #569:
URL: https://github.com/apache/celix/pull/569#discussion_r1218578275
##########
libs/framework/src/framework.c:
##########
@@ -237,6 +241,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);
+ celix_thread_mutexattr_t muAttr;
+ celixThreadMutexAttr_create(&muAttr);
+ celixThreadMutexAttr_settype(&muAttr, CELIX_THREAD_MUTEX_RECURSIVE);
Review Comment:
I rather prevent the usage of recursive mutexes.
Is a recursive mutex needed? What is the impact if this is not a recursive
mutex, that bundles cannot install bundle recursively?
##########
examples/celix-examples/track_tracker_example/src/activator.c:
##########
@@ -86,7 +86,7 @@ celix_status_t activator_start(activator_data_t* act,
celix_bundle_context_t *ct
act->svc.handle = act;
act->svc.calc = calc;
//note only triggers on calcTrk1, because of filter restrictions
- act->svcId = celix_bundleContext_registerService(ctx, &act->svc,
CALC_SERVICE_NAME, NULL);
+ act->svcId = celix_bundleContext_registerServiceAsync(ctx, &act->svc,
CALC_SERVICE_NAME, NULL);
Review Comment:
Are most of the changes to async just "last boy scout" improvements or now
necessary because of the bundle locking changes?
##########
cmake/cmake_celix/BundlePackaging.cmake:
##########
@@ -484,7 +484,8 @@ function(celix_bundle_libs)
get_filename_component(LIB_NAME ${LIB} NAME)
set(OUT "${BUNDLE_DIR}/${LIB_NAME}")
add_custom_command(OUTPUT ${OUT}
- COMMAND ${CMAKE_COMMAND} -E copy_if_different ${LIB} ${OUT}
+ COMMAND ${CMAKE_COMMAND} -E copy_if_different ${LIB} ${OUT}
+ DEPENDS ${LIB}
Review Comment:
nice catch, if was not aware this was missing
##########
libs/framework/include/celix_bundle_context.h:
##########
@@ -905,6 +905,22 @@ CELIX_FRAMEWORK_EXPORT long
celix_bundleContext_installBundle(celix_bundle_conte
*/
CELIX_FRAMEWORK_EXPORT bool
celix_bundleContext_uninstallBundle(celix_bundle_context_t *ctx, long bndId);
+/**
+ * @brief Unload the bundle with the provided bundle id. If needed the bundle
will be stopped first.
+ * Will silently ignore bundle ids < 0.
+ * Note that unloaded bundle is kept in bundle cache and can be reloaded with
the celix_bundleContext_installBundle function.
+ *
+ * If this function is called on the Celix event thread, the actual stopping
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 is stopped.
+ *
+ * @param ctx The bundle context
+ * @param bndId The bundle id to unload.
+ * @return true if the bundle is correctly unloaded. False if not.
+ */
+CELIX_FRAMEWORK_EXPORT bool
celix_bundleContext_unloadBundle(celix_bundle_context_t *ctx, long bndId);
Review Comment:
Should this not introduce a new bundle states? e.g.
Uninstalled -> Loaded -> Installed -> Starting -> Active
Active -> Stopping -> Resolved -> Unloaded -> Uninstalled
Or do we want to consider unload as a special uninstalled?
##########
libs/framework/src/framework.c:
##########
@@ -1543,6 +1568,9 @@ bool celix_framework_useBundle(framework_t *fw, bool
onlyActive, long bundleId,
bundleId);
if (entry != NULL) {
+ if (onlyActive) {
Review Comment:
Why is this only done on `onlyActive`? I would expect a readLock can also be
done on using `celix_bundleContext_useBundles`
##########
libs/framework/gtest/src/CelixBundleContextBundlesTestSuite.cc:
##########
@@ -165,6 +189,90 @@ TEST_F(CelixBundleContextBundlesTestSuite,
InstallAndUninstallBundlesTest) {
ASSERT_FALSE(celix_bundleContext_isBundleActive(ctx, bndId2));
ASSERT_FALSE(celix_bundleContext_isBundleActive(ctx, bndId3));
+ ASSERT_FALSE(celix_utils_directoryExists(bndRoot1));
Review Comment:
👍 , nice addition
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@celix.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
