Using service directly does not support SOD by default.

To request such support, the caller has to explicitly set CELIX_SERVICE_USE_SOD flags. Other code review issues are also addressed.

libs/framework/gtest/src/bundle_context_services_test.cpp

@@ -102,6 +102,7 @@ class CelixBundleContextServicesTests : public ::testing::Test {
 
         celix_bundleContext_unregisterService(ctx, svcId);
     }
+
     void registerAndUseServiceWithIncorrectVersion(bool direct) {
         struct calc {
             int (*calc)(int);
@@ -141,6 +142,7 @@ class CelixBundleContextServicesTests : public ::testing::Test {
 
         celix_bundleContext_unregisterService(ctx, svcId);
     }
+
     void registerAndUseServiceWithTimeout(bool direct) {
         const int NR_ITERATIONS = 5; //NOTE this test is sensitive for triggering race condition in the celix framework, therefore is used a few times.
         for (int i = 0; i < NR_ITERATIONS; ++i) {
@@ -192,6 +194,7 @@ class CelixBundleContextServicesTests : public ::testing::Test {
             EXPECT_FALSE(result2.get()); //note service is away, so even with a wait the service is not found.
         }
     }
+
     void registerAsyncAndUseServiceWithTimeout(bool direct) {
         const int NR_ITERATIONS = 5; //NOTE this test is sensitive for triggering race condition in the celix framework, therefore is used a few times.
         for (int i = 0; i < NR_ITERATIONS; ++i) {
@@ -1442,7 +1445,7 @@ TEST_F(CelixBundleContextServicesTests, UseServiceOnDemandDirectlyWithAsyncRegis
     }, nullptr);
     celix_service_use_options_t opts{};
     opts.filter.serviceName = "test";
-    opts.flags = CELIX_SERVICE_USE_DIRECT;
+    opts.flags = CELIX_SERVICE_USE_DIRECT | CELIX_SERVICE_USE_SOD;
     called = celix_bundleContext_useServiceWithOptions(ctx, &opts);
     EXPECT_TRUE(called); //service created on demand.
 
@@ -1480,7 +1483,7 @@ TEST_F(CelixBundleContextServicesTests, UseServicesOnDemandDirectlyWithAsyncRegi
 
     celix_service_use_options_t opts{};
     opts.filter.serviceName = "test";
-    opts.flags = CELIX_SERVICE_USE_DIRECT;
+    opts.flags = CELIX_SERVICE_USE_DIRECT | CELIX_SERVICE_USE_SOD;
     size_t count = celix_bundleContext_useServicesWithOptions(ctx, &opts);
     EXPECT_EQ(2, count);
 

libs/framework/include/celix_bundle_context.h

@@ -775,8 +775,13 @@ typedef struct celix_service_use_options {
      * @brief Call the provided callbacks from the caller thread directly if set, otherwise the callbacks will be called from the Celix event loop (most likely indirectly).
      * Note that using blocking service in the Celix event loop is generally a bad idea, which should be avoided if possible.
      */
-#define CELIX_SERVICE_USE_DIRECT          (1)
-    int flags;
+#define CELIX_SERVICE_USE_DIRECT              (1)
+    /**
+     * @brief Whether "service on demand" pattern is supported when CELIX_SERVICE_USE_DIRECT is set.
+     * Note that it has no effect in indirect mode, in which case "service on demand" is supported.
+     */
+#define CELIX_SERVICE_USE_SOD                 (2)
+    int flags OPTS_INIT;
 } celix_service_use_options_t;
 
 /**

libs/framework/src/bundle_context.c

@@ -1210,7 +1210,10 @@ bool celix_bundleContext_useServiceWithOptions(
 
 
     if(opts->flags & CELIX_SERVICE_USE_DIRECT) {
-        celix_framework_waitUntilNoPendingRegistration(ctx->framework); // TL;DR make "service on demand" pattern work. Try comment it out and run CelixBundleContextServicesTests.UseServiceOnDemandDirectlyWithAsyncRegisterTest
+        if(opts->flags & CELIX_SERVICE_USE_SOD) {
+            // TL;DR make "service on demand" pattern work. Try comment it out and run CelixBundleContextServicesTests.UseServiceOnDemandDirectlyWithAsyncRegisterTest
+            celix_framework_waitUntilNoPendingRegistration(ctx->framework);
+        }
         called = celix_serviceTracker_useHighestRankingService(data.svcTracker, NULL, opts->waitTimeoutInSeconds, opts->callbackHandle, opts->use, opts->useWithProperties, opts->useWithOwner);
     } else {
         struct timespec startTime = celix_gettime(CLOCK_MONOTONIC);
@@ -1264,7 +1267,10 @@ size_t celix_bundleContext_useServicesWithOptions(
     celix_framework_waitForGenericEvent(ctx->framework, eventId);
 
     if (opts->flags & CELIX_SERVICE_USE_DIRECT) {
-        celix_framework_waitUntilNoPendingRegistration(ctx->framework); // TL;DR make "service on demand" pattern work. Try comment it out and run CelixBundleContextServicesTests.UseServicesOnDemandDirectlyWithAsyncRegisterTest
+        if(opts->flags & CELIX_SERVICE_USE_SOD) {
+            // TL;DR make "service on demand" pattern work. Try comment it out and run CelixBundleContextServicesTests.UseServicesOnDemandDirectlyWithAsyncRegisterTest
+            celix_framework_waitUntilNoPendingRegistration(ctx->framework);
+        }
         celix_bundleContext_useServicesWithOptions_2_UseServiceTracker(&data);
     } else {
         eventId = celix_framework_fireGenericEvent(ctx->framework, -1, celix_bundle_getId(ctx->bundle), "use service tracker for celix_bundleContext_useServicesWithOptions", &data, celix_bundleContext_useServicesWithOptions_2_UseServiceTracker, NULL, NULL);

libs/framework/src/service_registration.c

@@ -140,14 +140,13 @@ celix_status_t serviceRegistration_unregister(service_registration_pt registrati
     if(!__atomic_compare_exchange_n(&registration->isUnregistering, &unregistering /* expected*/ , true /* desired */,
                                     false /* weak */, __ATOMIC_RELAXED/*success memorder*/, __ATOMIC_RELAXED/*failure memorder*/)) {
         status = CELIX_ILLEGAL_STATE;
-        goto immediate_return;
+    } else {
+        callback = registration->callback;
+        if (callback.unregister != NULL) {
+            status = callback.unregister(callback.handle, registration->bundle, registration);
+        }
     }
-    callback = registration->callback;
-	if (callback.unregister != NULL) {
-        status = callback.unregister(callback.handle, registration->bundle, registration);
-	}
 
-immediate_return:
     framework_logIfError(celix_frameworkLogger_globalLogger(), status, NULL, "Cannot unregister service registration");
 	return status;
 }