diff --git a/CHANGES.md b/CHANGES.md index 3221802a1..7fee14e7d 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -66,6 +66,9 @@ limitations under the License. The `celix_bundleContext_trackServicesWithOptions` is still available for more advanced use-cases. - Function `celix_bundle_destroyServiceTrackerList` is removed. The returned array list from `celix_bundle_listServiceTrackers` is now configured to destroy the service trackers info entries. +- It is no longer possible to use the `celix_bundleContext_useService*` functions or `celix::BundleContxt::useService*` + methods on the Celix event thread. The calls will now immediately return and log an error if called on the + Celix event thread. ## New Features diff --git a/bundles/remote_services/remote_service_admin_shm_v2/rsa_shm/gtest/src/RsaShmImplUnitTestSuite.cc b/bundles/remote_services/remote_service_admin_shm_v2/rsa_shm/gtest/src/RsaShmImplUnitTestSuite.cc index e602ee9ee..833b55a08 100644 --- a/bundles/remote_services/remote_service_admin_shm_v2/rsa_shm/gtest/src/RsaShmImplUnitTestSuite.cc +++ b/bundles/remote_services/remote_service_admin_shm_v2/rsa_shm/gtest/src/RsaShmImplUnitTestSuite.cc @@ -813,7 +813,6 @@ TEST_F(RsaShmRpcTestSuite, CallRemoteService) { EXPECT_EQ(CELIX_SUCCESS, calc->add(calc->handle, 1, 2, &result)); EXPECT_EQ(3.0, result); }; - opts.flags = CELIX_SERVICE_USE_DIRECT | CELIX_SERVICE_USE_SOD; auto found = celix_bundleContext_useServiceWithOptions(clientCtx.get(), &opts); EXPECT_TRUE(found); @@ -835,7 +834,6 @@ TEST_F(RsaShmRpcTestSuite, CallRemoteService) { double result; EXPECT_NE(CELIX_SUCCESS, calc->add(calc->handle, 1, 2, &result)); }; - opts.flags = CELIX_SERVICE_USE_DIRECT | CELIX_SERVICE_USE_SOD; found = celix_bundleContext_useServiceWithOptions(clientCtx.get(), &opts); EXPECT_TRUE(found); diff --git a/bundles/shell/shell/gtest/CMakeLists.txt b/bundles/shell/shell/gtest/CMakeLists.txt index 91a50aef6..75d18ea88 100644 --- a/bundles/shell/shell/gtest/CMakeLists.txt +++ b/bundles/shell/shell/gtest/CMakeLists.txt @@ -19,8 +19,10 @@ add_executable(test_shell src/ShellTestSuite.cc ) +add_celix_bundle(celix_shell_empty_resource_test_bundle NO_ACTIVATOR VERSION 0.0.0) + target_link_libraries(test_shell PRIVATE Celix::framework Celix::shell_api GTest::gtest GTest::gtest_main) -celix_target_bundle_set_definition(test_shell NAME TEST_BUNDLES Celix::shell) +celix_target_bundle_set_definition(test_shell NAME TEST_BUNDLES Celix::shell celix_shell_empty_resource_test_bundle) target_compile_options(test_shell PRIVATE -Wno-deprecated-declarations) add_test(NAME test_shell COMMAND test_shell) @@ -31,7 +33,7 @@ if (CELIX_CXX14) add_executable(test_cxx_shell src/ShellTestSuite.cc) target_link_libraries(test_cxx_shell PRIVATE Celix::framework Celix::shell_api GTest::gtest GTest::gtest_main) - celix_target_bundle_set_definition(test_cxx_shell NAME TEST_BUNDLES Celix::ShellCxx) + celix_target_bundle_set_definition(test_cxx_shell NAME TEST_BUNDLES Celix::ShellCxx celix_shell_empty_resource_test_bundle) target_compile_definitions(test_cxx_shell PRIVATE -DCXX_SHELL) add_test(NAME test_cxx_shell COMMAND test_cxx_shell) diff --git a/bundles/shell/shell/gtest/src/ShellTestSuite.cc b/bundles/shell/shell/gtest/src/ShellTestSuite.cc index 318a83790..b7b5f791e 100644 --- a/bundles/shell/shell/gtest/src/ShellTestSuite.cc +++ b/bundles/shell/shell/gtest/src/ShellTestSuite.cc @@ -21,19 +21,20 @@ #include #include -#include "celix_shell_command.h" -#include "celix_framework_factory.h" #include "celix_bundle_context.h" -#include "celix_shell.h" -#include "celix_framework_utils.h" #include "celix_constants.h" +#include "celix_framework_factory.h" +#include "celix_framework_utils.h" +#include "celix_shell.h" +#include "celix_shell_command.h" +#include "celix_stdlib_cleanup.h" class ShellTestSuite : public ::testing::Test { public: ShellTestSuite() : ctx{createFrameworkContext()} { auto* fw = celix_bundleContext_getFramework(ctx.get()); size_t nr = celix_framework_utils_installBundleSet(fw, TEST_BUNDLES, true); - EXPECT_EQ(nr, 1); //shell bundle + EXPECT_EQ(nr, 2); //shell and celix_shell_empty_resource_test_bundle bundle } static std::shared_ptr createFrameworkContext() { @@ -54,12 +55,25 @@ class ShellTestSuite : public ::testing::Test { }}; } + long getBundleIdForResourceBundle() const { + celix_autoptr(celix_array_list_t) bundles = celix_bundleContext_listBundles(ctx.get()); + for (auto i = 0 ; i < celix_arrayList_size(bundles); ++i) { + auto bndId = celix_arrayList_getLong(bundles, i); + celix_autofree char* name = celix_bundleContext_getBundleSymbolicName(ctx.get(), bndId); + if (strstr(name, "resource") != nullptr) { + return bndId; + } + + } + return -1; + } + std::shared_ptr ctx; }; TEST_F(ShellTestSuite, shellBundleInstalledTest) { auto *bndIds = celix_bundleContext_listBundles(ctx.get()); - EXPECT_EQ(1, celix_arrayList_size(bndIds)); + EXPECT_EQ(2, celix_arrayList_size(bndIds)); celix_arrayList_destroy(bndIds); } @@ -71,8 +85,6 @@ static void callCommand(std::shared_ptr& ctx, const char celix_bundle_context_t* context{}; long tracker{-1}; }; - // Note that using celix_bundleContext_useServiceWithOptions to call command quit/stop may result in deadlock. - // For more on this, see https://github.com/apache/celix/issues/629 struct callback_data data{}; data.cmdLine = cmdLine; data.cmdShouldSucceed = cmdShouldSucceed; @@ -104,9 +116,10 @@ TEST_F(ShellTestSuite, testAllCommandsAreCallable) { callCommand(ctx, "non-existing", false); callCommand(ctx, "install a-bundle-loc.zip", true); callCommand(ctx, "help", true); - callCommand(ctx, "help lb", false); //note need namespace + callCommand(ctx, "help lb", true); callCommand(ctx, "help celix::lb", true); callCommand(ctx, "help non-existing-command", false); + callCommand(ctx, "help celix::non-existing-command-with-namespace", false); callCommand(ctx, "lb", true); callCommand(ctx, "lb -l", true); callCommand(ctx, "query", true); @@ -144,7 +157,8 @@ TEST_F(ShellTestSuite, stopFrameworkTest) { TEST_F(ShellTestSuite, stopSelfTest) { auto* list = celix_bundleContext_listBundles(ctx.get()); - EXPECT_EQ(1, celix_arrayList_size(list)); + ASSERT_GE(celix_arrayList_size(list), 1); + auto nrOfBundles = celix_arrayList_size(list); long shellBundleId = celix_arrayList_getLong(list, 0); celix_arrayList_destroy(list); @@ -156,24 +170,33 @@ TEST_F(ShellTestSuite, stopSelfTest) { std::this_thread::sleep_for(std::chrono::milliseconds{100}); list = celix_bundleContext_listBundles(ctx.get()); - EXPECT_EQ(0, celix_arrayList_size(list)); + EXPECT_EQ(nrOfBundles-1, celix_arrayList_size(list)); celix_arrayList_destroy(list); } TEST_F(ShellTestSuite, queryTest) { + struct data { + long resourceBundleId; + }; + data data{}; + data.resourceBundleId = getBundleIdForResourceBundle(); + celix_service_use_options_t opts{}; opts.filter.serviceName = CELIX_SHELL_COMMAND_SERVICE_NAME; opts.filter.filter = "(command.name=celix::query)"; opts.waitTimeoutInSeconds = 1.0; - opts.use = [](void */*handle*/, void *svc) { + opts.callbackHandle = (void*)&data; + opts.use = [](void* handle, void *svc) { + auto *d = static_cast(handle); auto *command = static_cast(svc); - EXPECT_TRUE(command != nullptr); + ASSERT_TRUE(command != nullptr); + ASSERT_TRUE(d != nullptr); { char *buf = nullptr; size_t len; FILE *sout = open_memstream(&buf, &len); - command->executeCommand(command->handle, (char *) "query", sout, sout); + command->executeCommand(command->handle, "query", sout, sout); fclose(sout); char* found = strstr(buf, "Provided services found 1"); //note could be 11, 12, etc EXPECT_TRUE(found != nullptr); @@ -183,7 +206,8 @@ TEST_F(ShellTestSuite, queryTest) { char *buf = nullptr; size_t len; FILE *sout = open_memstream(&buf, &len); - command->executeCommand(command->handle, (char *) "query 0", sout, sout); //note query framework bundle -> no results + auto cmd = std::string{"query "} + std::to_string(d->resourceBundleId); + command->executeCommand(command->handle, cmd.c_str(), sout, sout); //note query test resource bundle -> no results fclose(sout); char* found = strstr(buf, "No results"); //note could be 11, 12, etc EXPECT_TRUE(found != nullptr); diff --git a/bundles/shell/shell/src/help_command.c b/bundles/shell/shell/src/help_command.c index 092a32f4a..746de5c0a 100644 --- a/bundles/shell/shell/src/help_command.c +++ b/bundles/shell/shell/src/help_command.c @@ -22,8 +22,9 @@ #include #include "celix_array_list.h" -#include "celix_utils.h" #include "celix_shell.h" +#include "celix_stdlib_cleanup.h" +#include "celix_utils.h" #include "std_commands.h" struct print_handle { @@ -51,11 +52,9 @@ static void printHelp(void *handle, void *svc) { sub = strtok_r(NULL, CELIX_SHELL_COMMAND_SEPARATOR, &save_ptr); if (sub == NULL) { - unsigned int i; celix_array_list_t *commands = NULL; - shell->getCommands(shell->handle, &commands); - for (i = 0; i < celix_arrayList_size(commands); i++) { + for (int i = 0; i < celix_arrayList_size(commands); i++) { char *name = celix_arrayList_get(commands, i); fprintf(out, "%s\n", name); free(name); @@ -71,17 +70,19 @@ static void printHelp(void *handle, void *svc) { shell->getCommands(shell->handle, &commands); bool cmdFound = false; for (i = 0; i < celix_arrayList_size(commands); i++) { - char *name = celix_arrayList_get(commands, i); - if (strcmp(sub, name) == 0) { + char *fqn = celix_arrayList_get(commands, i); + bool hasNamespace = strstr(fqn, "::") != NULL; + char* localName = hasNamespace ? strstr(fqn, "::") + 2 : fqn; + if (strcmp(sub, fqn) == 0 || (hasNamespace && strcmp(sub, localName) == 0)) { cmdFound = true; char *usage_str = NULL; char *desc_str = NULL; - sub_status_desc = shell->getCommandDescription(shell->handle, name, &desc_str); - sub_status_usage = shell->getCommandUsage(shell->handle, name, &usage_str); + sub_status_desc = shell->getCommandDescription(shell->handle, fqn, &desc_str); + sub_status_usage = shell->getCommandUsage(shell->handle, fqn, &usage_str); if (sub_status_usage == CELIX_SUCCESS && sub_status_desc == CELIX_SUCCESS) { - fprintf(out, "Command : %s\n", name); + fprintf(out, "Command : %s\n", fqn); fprintf(out, "Usage : %s\n", usage_str == NULL ? "" : usage_str); fprintf(out, "Description : %s\n", desc_str == NULL ? "" : desc_str); } else { @@ -91,7 +92,7 @@ static void printHelp(void *handle, void *svc) { free(usage_str); free(desc_str); } - free(name); + free(fqn); } celix_arrayList_destroy(commands); @@ -103,16 +104,23 @@ static void printHelp(void *handle, void *svc) { } } -bool helpCommand_execute(void *handle, const char *const_cmdLine, FILE *out, FILE *err) { - celix_bundle_context_t *ctx = handle; - struct print_handle printHandle; - char *cmdLine = celix_utils_strdup(const_cmdLine); +bool helpCommand_execute(void* handle, const char* const_cmdLine, FILE* out, FILE* err) { + celix_bundle_context_t* ctx = handle; + celix_autofree char* cmdLine = celix_utils_strdup(const_cmdLine); + + long trkId = celix_bundleContext_trackServices(ctx, CELIX_SHELL_SERVICE_NAME); + if (trkId < 0) { + fprintf(err, "Error tracking shell services\n"); + return false; + } + struct print_handle printHandle; printHandle.cmdLine = cmdLine; printHandle.out = out; printHandle.err = err; - bool called = celix_bundleContext_useService(ctx, CELIX_SHELL_SERVICE_NAME, &printHandle, printHelp); + bool called = celix_bundleContext_useTrackedService(ctx, trkId, &printHandle, printHelp); + + celix_bundleContext_stopTracker(ctx, trkId); - free(cmdLine); return called & printHandle.callSucceeded; } diff --git a/examples/conan_test_package/test_shell.c b/examples/conan_test_package/test_shell.c index b49193506..626202e59 100644 --- a/examples/conan_test_package/test_shell.c +++ b/examples/conan_test_package/test_shell.c @@ -65,7 +65,6 @@ int main() { opts.filter.serviceName = CELIX_SHELL_SERVICE_NAME; opts.callbackHandle = NULL; opts.waitTimeoutInSeconds = 1.0; - opts.flags = CELIX_SERVICE_USE_DIRECT | CELIX_SERVICE_USE_SOD; opts.use = use; bool called = celix_bundleContext_useServiceWithOptions(ctx, &opts); assert(called); @@ -73,5 +72,3 @@ int main() { celix_frameworkFactory_destroyFramework(fw); return 0; } - - diff --git a/libs/framework/gtest/src/CelixBundleContextServicesTestSuite.cc b/libs/framework/gtest/src/CelixBundleContextServicesTestSuite.cc index 12ffcd949..0a0c6af96 100644 --- a/libs/framework/gtest/src/CelixBundleContextServicesTestSuite.cc +++ b/libs/framework/gtest/src/CelixBundleContextServicesTestSuite.cc @@ -64,7 +64,7 @@ class CelixBundleContextServicesTestSuite : public ::testing::Test { CelixBundleContextServicesTestSuite& operator=(CelixBundleContextServicesTestSuite&&) = delete; CelixBundleContextServicesTestSuite& operator=(const CelixBundleContextServicesTestSuite&) = delete; - void registerAndUseServiceWithCorrectVersion(bool direct) const { + void registerAndUseServiceWithCorrectVersion() const { struct calc { int (*calc)(int); }; @@ -78,9 +78,6 @@ class CelixBundleContextServicesTestSuite : public ::testing::Test { celix_service_use_options_t use_opts{}; use_opts.filter.serviceName = "calc"; use_opts.filter.versionRange = "[1,2)"; - if(direct) { - use_opts.flags = CELIX_SERVICE_USE_DIRECT; - } celix_service_registration_options_t reg_opts{}; reg_opts.serviceName = calcName; @@ -105,7 +102,7 @@ class CelixBundleContextServicesTestSuite : public ::testing::Test { celix_bundleContext_unregisterService(ctx, svcId); } - void registerAndUseServiceWithIncorrectVersion(bool direct) const { + void registerAndUseServiceWithIncorrectVersion() const { struct calc { int (*calc)(int); }; @@ -119,9 +116,6 @@ class CelixBundleContextServicesTestSuite : public ::testing::Test { celix_service_use_options_t use_opts{}; use_opts.filter.serviceName = "calc"; use_opts.filter.versionRange = "[2,3)"; - if(direct) { - use_opts.flags = CELIX_SERVICE_USE_DIRECT; - } celix_service_registration_options_t reg_opts{}; reg_opts.serviceName = calcName; @@ -145,7 +139,7 @@ class CelixBundleContextServicesTestSuite : public ::testing::Test { celix_bundleContext_unregisterService(ctx, svcId); } - void registerAndUseServiceWithTimeout(bool direct) const { + void registerAndUseServiceWithTimeout() const { 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) { printf("Iter %i\n", i); @@ -161,9 +155,6 @@ class CelixBundleContextServicesTestSuite : public ::testing::Test { celix_service_use_options_t opts{}; opts.filter.serviceName = "calc"; - if(direct) { - opts.flags = CELIX_SERVICE_USE_DIRECT; - } bool called = celix_bundleContext_useServiceWithOptions(ctx, &opts); EXPECT_FALSE(called); //service not avail. @@ -197,7 +188,7 @@ class CelixBundleContextServicesTestSuite : public ::testing::Test { } } - void registerAsyncAndUseServiceWithTimeout(bool direct) const { + void registerAsyncAndUseServiceWithTimeout() const { 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) { printf("Iter %i\n", i); @@ -213,9 +204,6 @@ class CelixBundleContextServicesTestSuite : public ::testing::Test { celix_service_use_options_t opts{}; opts.filter.serviceName = "calc"; - if(direct) { - opts.flags = CELIX_SERVICE_USE_DIRECT; - } bool called = celix_bundleContext_useServiceWithOptions(ctx, &opts); EXPECT_FALSE(called); //service not avail. @@ -355,7 +343,7 @@ TEST_F(CelixBundleContextServicesTestSuite, UseServicesWithoutNameTest) { celix_bundleContext_unregisterService(ctx, svcId1); } -TEST_F(CelixBundleContextServicesTestSuite, TegisterMultipleAndUseServicesTest) { +TEST_F(CelixBundleContextServicesTestSuite, RegisterMultipleAndUseServicesTest) { struct calc { int (*calc)(int); }; @@ -393,13 +381,11 @@ TEST_F(CelixBundleContextServicesTestSuite, TegisterMultipleAndUseServicesTest) use_opts.filter.versionRange = nullptr; use_opts.callbackHandle = &total; use_opts.use = use; - use_opts.flags = 0; total = 0; count = celix_bundleContext_useServicesWithOptions(ctx, &use_opts); EXPECT_EQ(3, count); EXPECT_EQ(42 * 3, total); - use_opts.flags = CELIX_SERVICE_USE_DIRECT; total = 0; count = celix_bundleContext_useServicesWithOptions(ctx, &use_opts); EXPECT_EQ(3, count); @@ -411,13 +397,11 @@ TEST_F(CelixBundleContextServicesTestSuite, TegisterMultipleAndUseServicesTest) EXPECT_EQ(2, count); EXPECT_EQ(42 * 2, total); - use_opts.flags = 0; total = 0; count = celix_bundleContext_useServicesWithOptions(ctx, &use_opts); EXPECT_EQ(2, count); EXPECT_EQ(42 * 2, total); - use_opts.flags = CELIX_SERVICE_USE_DIRECT; total = 0; count = celix_bundleContext_useServicesWithOptions(ctx, &use_opts); EXPECT_EQ(2, count); @@ -470,7 +454,6 @@ TEST_F(CelixBundleContextServicesTestSuite, UseServiceInUseCallbackTest) { opts.filter.serviceName = calcName; opts.callbackHandle = ctx; opts.use = use; - opts.flags = 0; bool called = celix_bundleContext_useServiceWithOptions(ctx, &opts); EXPECT_TRUE(called); @@ -519,35 +502,19 @@ TEST_F(CelixBundleContextServicesTestSuite, RegisterAndUseServiceTest) { }; TEST_F(CelixBundleContextServicesTestSuite, RegisterAndUseServiceWithTimeoutTest) { - registerAndUseServiceWithTimeout(false); -} - -TEST_F(CelixBundleContextServicesTestSuite, RegisterAndUseServiceDirectWithTimeoutTest) { - registerAndUseServiceWithTimeout(true); + registerAndUseServiceWithTimeout(); } TEST_F(CelixBundleContextServicesTestSuite, RegisterAsyncAndUseServiceWithTimeoutTest) { - registerAsyncAndUseServiceWithTimeout(false); -} - -TEST_F(CelixBundleContextServicesTestSuite, RegisterAsyncAndUseServiceDirectWithTimeoutTest) { - registerAsyncAndUseServiceWithTimeout(true); + registerAsyncAndUseServiceWithTimeout(); } TEST_F(CelixBundleContextServicesTestSuite, RegisterAndUseServiceWithCorrectVersionTest) { - registerAndUseServiceWithCorrectVersion(false); -} - -TEST_F(CelixBundleContextServicesTestSuite, RegisterAndUseServiceDirectWithCorrectVersionTest) { - registerAndUseServiceWithCorrectVersion(true); + registerAndUseServiceWithCorrectVersion(); } TEST_F(CelixBundleContextServicesTestSuite, RegisterAndUseServiceWithIncorrectVersionTest) { - registerAndUseServiceWithIncorrectVersion(false); -} - -TEST_F(CelixBundleContextServicesTestSuite, RegisterAndUseServiceDirectWithIncorrectVersionTest) { - registerAndUseServiceWithIncorrectVersion(true); + registerAndUseServiceWithIncorrectVersion(); } TEST_F(CelixBundleContextServicesTestSuite, RegisterAndUseWithForcedRaceConditionTest) { @@ -974,16 +941,14 @@ TEST_F(CelixBundleContextServicesTestSuite, ServiceTrackerWithRaceConditionTest) celix_bundleContext_stopTracker(ctx, trackerId); }; -TEST_F(CelixBundleContextServicesTestSuite, UseServiceDoesNotBlockInEventLoop) { - void *svc1 = (void*)0x100; - - auto set = [](void *handle, void */*svc*/) { - celix_bundle_context_t *ctx = static_cast(handle); +TEST_F(CelixBundleContextServicesTestSuite, UseServiceDoesNotWorkInEventLoop) { + auto callback = [](void* data) { + auto* ctx = static_cast(data); celix_service_use_options_t use_opts{}; use_opts.filter.serviceName = "NotAvailable"; use_opts.waitTimeoutInSeconds = 3600; // unacceptable long blocking use_opts.callbackHandle = nullptr; - use_opts.use = [](void *handle, void *svc) { + use_opts.use = [](void* handle, void* svc) { FAIL() << "We shouldn't get here: (" << handle << "," << svc << ")"; }; @@ -991,25 +956,9 @@ TEST_F(CelixBundleContextServicesTestSuite, UseServiceDoesNotBlockInEventLoop) { ASSERT_FALSE(called); }; - long svcId1 = celix_bundleContext_registerService(ctx, svc1, "NA", nullptr); - ASSERT_GE(svcId1, 0); - - celix_service_tracking_options_t opts{}; - opts.callbackHandle = (void*)ctx; - opts.filter.serviceName = "NA"; - opts.set = set; - long trackerId = celix_bundleContext_trackServicesWithOptionsAsync(ctx, &opts); - ASSERT_TRUE(trackerId >= 0); - - void *svc2 = (void*)0x200; //no ranking - long svcId2 = celix_bundleContext_registerServiceAsync(ctx, svc2, "NotAvailable", nullptr); - ASSERT_GE(svcId2, 0); - celix_bundleContext_waitForAsyncTracker(ctx, trackerId); - celix_bundleContext_waitForAsyncRegistration(ctx, svcId2); - - celix_bundleContext_unregisterService(ctx, svcId2); - celix_bundleContext_stopTracker(ctx, trackerId); - celix_bundleContext_unregisterService(ctx, svcId1); + long eventId = celix_framework_fireGenericEvent( + fw, 0, CELIX_FRAMEWORK_BUNDLE_ID, "test event", ctx, callback, nullptr, nullptr); + celix_framework_waitForGenericEvent(fw, eventId); } TEST_F(CelixBundleContextServicesTestSuite, ServicesTrackerSetTest) { @@ -1480,7 +1429,6 @@ TEST_F(CelixBundleContextServicesTestSuite, UseServiceOnDemandDirectlyWithAsyncR }, nullptr); celix_service_use_options_t opts{}; opts.filter.serviceName = "test"; - opts.flags = CELIX_SERVICE_USE_DIRECT | CELIX_SERVICE_USE_SOD; called = celix_bundleContext_useServiceWithOptions(ctx, &opts); EXPECT_TRUE(called); //service created on demand. @@ -1518,7 +1466,6 @@ TEST_F(CelixBundleContextServicesTestSuite, UseServicesOnDemandDirectlyWithAsync celix_service_use_options_t opts{}; opts.filter.serviceName = "test"; - opts.flags = CELIX_SERVICE_USE_DIRECT | CELIX_SERVICE_USE_SOD; size_t count = celix_bundleContext_useServicesWithOptions(ctx, &opts); EXPECT_EQ(2, count); diff --git a/libs/framework/include/celix/BundleContext.h b/libs/framework/include/celix/BundleContext.h index d98c0c5e1..608fa37f8 100644 --- a/libs/framework/include/celix/BundleContext.h +++ b/libs/framework/include/celix/BundleContext.h @@ -33,7 +33,7 @@ #include "celix/Bundle.h" #include "celix/Framework.h" -#include "celix/dm/DependencyManager.h" //TODO, TBD include or forward declaration? +#include "celix/dm/DependencyManager.h" namespace celix { @@ -106,10 +106,16 @@ namespace celix { /** * @brief Use a service registered in the Celix framework using a fluent builder API. * + * @warning Cannot be called from the Celix event thread. + * + * @note celix::BundleContext::useService should be considered a test util method. + * For production code, use celix::BundleContext::trackServices combined with use callback methods on the + * service tracker instead. + * * The service use can be fine tuned using the returned UseServiceBuilder API. * * With this API a Celix service can be used by providing use functions. - * The use function will be executed on the Celix event thread and the Celix framework + * The use function will be executed on the calling thread and the Celix framework * will ensure that the service cannot be removed while in use. * * If there are more 1 matching service, the highest ranking service will be used. @@ -137,11 +143,17 @@ namespace celix { /** * @brief Use services registered in the Celix framework using a fluent builder API. * + * @warning Cannot be called from the Celix event thread. + * + * @note celix::BundleContext::useServices should be considered a test util method. + * For production code, use celix::BundleContext::trackServices combined with use callback methods on the + * service tracker instead. + * * The service use can be fine tuned using the returned UseServiceBuilder API. * * With this API Celix services can be used by providing use functions. - * The use function will be executed on the Celix event thread and the Celix framework - * will ensure that the service cannot be removed while in use. + * The use function will be executed on the calling thread and the Celix framework + * will ensure that the used services cannot be removed while in use. * * The use callbacks will be called for every matching service found. * diff --git a/libs/framework/include/celix_bundle_context.h b/libs/framework/include/celix_bundle_context.h index fb9dc94b7..890de3fe3 100644 --- a/libs/framework/include/celix_bundle_context.h +++ b/libs/framework/include/celix_bundle_context.h @@ -376,7 +376,8 @@ typedef struct celix_service_filter_options { * @param opts The pointer to the filter options. * @return If found a valid service id (>= 0) if not found -1. */ -CELIX_FRAMEWORK_EXPORT long celix_bundleContext_findServiceWithOptions(celix_bundle_context_t *ctx, const celix_service_filter_options_t *opts); +CELIX_FRAMEWORK_EXPORT long celix_bundleContext_findServiceWithOptions(celix_bundle_context_t* ctx, + const celix_service_filter_options_t* opts); /** * @brief Finds the services conform the provider filter options and returns a list of the found service ids. @@ -385,17 +386,21 @@ CELIX_FRAMEWORK_EXPORT long celix_bundleContext_findServiceWithOptions(celix_bun * @param opts The pointer to the filter options. * @return A array list with as value a long int. */ -CELIX_FRAMEWORK_EXPORT celix_array_list_t* celix_bundleContext_findServicesWithOptions(celix_bundle_context_t *ctx, const celix_service_filter_options_t *opts); +CELIX_FRAMEWORK_EXPORT celix_array_list_t* +celix_bundleContext_findServicesWithOptions(celix_bundle_context_t* ctx, const celix_service_filter_options_t* opts); /** * @brief Use the service with the provided service id using the provided callback. The Celix framework will ensure that * the targeted service cannot be removed during the callback. * - * @deprecated Use celix_bundleContext_trackService* combined with celix_bundleContext_useTrackedService* functions - * instead. + * @warning Cannot be called from the Celix event thread. * - * The svc is should only be considered valid during the callback. - * If no service is found, the callback will not be invoked and this function will return false immediately. + * @deprecated celix_bundleContext_useServiceWithId is deprecated and should be considered test utils functions. In + * operational code use celix_bundleContext_trackService* combined with celix_bundleContext_useTrackedService* + * functions instead. + * + * The Celix framework will ensure that the targeted service cannot be removed during the callback and the callback + * is called on the calling thread. * * This function will block until the callback is finished. As result it is possible to provide callback data from the * stack. @@ -406,7 +411,7 @@ CELIX_FRAMEWORK_EXPORT celix_array_list_t* celix_bundleContext_findServicesWithO * service id (sanity check) * @param callbackHandle The data pointer, which will be used in the callbacks * @param use The callback, which will be called when service is retrieved. - * @param bool returns true if a service was found. + * @param bool returns true if a service was found and the callback was called. */ CELIX_FRAMEWORK_DEPRECATED_EXPORT bool celix_bundleContext_useServiceWithId(celix_bundle_context_t* ctx, long serviceId, @@ -417,10 +422,14 @@ CELIX_FRAMEWORK_DEPRECATED_EXPORT bool celix_bundleContext_useServiceWithId(celi /** * @brief Use the highest ranking service with the provided service name using the provided callback. * - * @deprecated Use celix_bundleContext_trackService* combined with celix_bundleContext_useTrackedService* functions + * @warning Cannot be called from the Celix event thread. + * + * @deprecated celix_bundleContext_useService is deprecated and should be considered test utils functions. In + * operational code use celix_bundleContext_trackService* combined with celix_bundleContext_useTrackedService* functions * instead. * - * The Celix framework will ensure that the targeted service cannot be removed during the callback. + * The Celix framework will ensure that the targeted service cannot be removed during the callback and the callback + * is called on the calling thread. * * The svc is should only be considered valid during the callback. * If no service is found, the callback will not be invoked and this function will return false immediately. @@ -432,7 +441,7 @@ CELIX_FRAMEWORK_DEPRECATED_EXPORT bool celix_bundleContext_useServiceWithId(celi * @param serviceName the required service name. * @param callbackHandle The data pointer, which will be used in the callbacks * @param use The callback, which will be called when service is retrieved. - * @return True if a service was found. + * @return True if a service was found and the callback was called. */ CELIX_FRAMEWORK_DEPRECATED_EXPORT bool celix_bundleContext_useService( celix_bundle_context_t *ctx, @@ -444,10 +453,14 @@ CELIX_FRAMEWORK_DEPRECATED_EXPORT bool celix_bundleContext_useService( /** * @brief Use the services with the provided service name using the provided callback. * - * @deprecated Use celix_bundleContext_trackService* combined with celix_bundleContext_useTrackedService* functions + * @warning Cannot be called from the Celix event thread. + * + * @deprecated celix_bundleContext_useServices is deprecated and should be considered test utils functions. In + * operational code use celix_bundleContext_trackService* combined with celix_bundleContext_useTrackedService* functions * instead. * - * The Celix framework will ensure that the targeted service cannot be removed during the callback. + * The Celix framework will ensure that the targeted service cannot be removed during the callback and the callback + * is called on the calling thread. * * The svc is should only be considered valid during the callback. * If no service is found, the callback will not be invoked and this function will return 0 immediately. @@ -459,7 +472,7 @@ CELIX_FRAMEWORK_DEPRECATED_EXPORT bool celix_bundleContext_useService( * @param serviceName the required service name. * @param callbackHandle The data pointer, which will be used in the callbacks * @param use The callback, which will be called for every service found. - * @return The number of services found and called + * @return The number of services found and called. */ CELIX_FRAMEWORK_DEPRECATED_EXPORT size_t celix_bundleContext_useServices( celix_bundle_context_t *ctx, @@ -516,18 +529,6 @@ typedef struct celix_service_use_options { */ void (*useWithOwner)(void* handle, void* svc, const celix_properties_t* props, const celix_bundle_t* svcOwner) CELIX_OPTS_INIT; - /** - * @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) - /** - * @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 CELIX_OPTS_INIT; } celix_service_use_options_t; #ifndef __cplusplus @@ -541,15 +542,15 @@ typedef struct celix_service_use_options { .callbackHandle = NULL, \ .use = NULL, \ .useWithProperties = NULL, \ - .useWithOwner = NULL, \ - .flags=0} + .useWithOwner = NULL} #endif /** * @brief Use the highest ranking service satisfying the provided service filter options using the provided callback. * - * @deprecated Use celix_bundleContext_trackService* combined with celix_bundleContext_useTrackedService* functions - * instead. + * @note celix_bundleContext_useService should be considered a test util function. + * For production code, use celix_bundleContext_trackService* combined with celix_bundleContext_useTrackedService* + * functions instead. * * The Celix framework will ensure that the targeted service cannot be removed during the callback. * @@ -564,7 +565,7 @@ typedef struct celix_service_use_options { * @param opts The required options. Note that the serviceName is required. * @return True if a service was found. */ -CELIX_FRAMEWORK_DEPRECATED_EXPORT bool celix_bundleContext_useServiceWithOptions( +CELIX_FRAMEWORK_EXPORT bool celix_bundleContext_useServiceWithOptions( celix_bundle_context_t *ctx, const celix_service_use_options_t *opts); @@ -572,8 +573,9 @@ CELIX_FRAMEWORK_DEPRECATED_EXPORT bool celix_bundleContext_useServiceWithOptions /** * @brief Use the services with the provided service filter options using the provided callback. * - * @deprecated Use celix_bundleContext_trackService* combined with celix_bundleContext_useTrackedService* functions - * instead. + * @note celix_bundleContext_useService should be considered test utils functions. + * For production code, use celix_bundleContext_trackService* combined with celix_bundleContext_useTrackedService* + * functions instead. * * The Celix framework will ensure that the targeted service cannot be removed during the callback. * @@ -588,7 +590,7 @@ CELIX_FRAMEWORK_DEPRECATED_EXPORT bool celix_bundleContext_useServiceWithOptions * @param opts The required options. Note that the serviceName is required. * @return The number of services found and called */ -CELIX_FRAMEWORK_DEPRECATED_EXPORT size_t celix_bundleContext_useServicesWithOptions( +CELIX_FRAMEWORK_EXPORT size_t celix_bundleContext_useServicesWithOptions( celix_bundle_context_t *ctx, const celix_service_use_options_t *opts); diff --git a/libs/framework/src/bundle_context.c b/libs/framework/src/bundle_context.c index adb4db10b..7032a7e35 100644 --- a/libs/framework/src/bundle_context.c +++ b/libs/framework/src/bundle_context.c @@ -1118,130 +1118,64 @@ size_t celix_bundleContext_useServices( return celix_bundleContext_useServicesWithOptions(ctx, &opts); } -typedef struct celix_bundle_context_use_service_data { - celix_bundle_context_t* ctx; - const celix_service_use_options_t* opts; - - bool called; //for use service - size_t count; //for use services - celix_service_tracker_t * svcTracker; -} celix_bundle_context_use_service_data_t; - -static void celix_bundleContext_useServiceWithOptions_1_CreateServiceTracker(void *data) { - celix_bundle_context_use_service_data_t* d = data; - assert(celix_framework_isCurrentThreadTheEventLoop(d->ctx->framework)); - - celix_service_tracking_options_t trkOpts = CELIX_EMPTY_SERVICE_TRACKING_OPTIONS; - trkOpts.filter = d->opts->filter; - - d->called = false; - d->count = 0; - d->svcTracker = celix_serviceTracker_createWithOptions(d->ctx, &trkOpts); -} - -static void celix_bundleContext_useServiceWithOptions_2_UseServiceTracker(void *data) { - celix_bundle_context_use_service_data_t* d = data; - assert(celix_framework_isCurrentThreadTheEventLoop(d->ctx->framework)); - - d->called = celix_serviceTracker_useHighestRankingService(d->svcTracker, d->opts->filter.serviceName, 0, d->opts->callbackHandle, d->opts->use, d->opts->useWithProperties, d->opts->useWithOwner); -} - -bool celix_bundleContext_useServiceWithOptions( - celix_bundle_context_t *ctx, - const celix_service_use_options_t *opts) { +static size_t celix_bundleContext_useServicesInternal(celix_bundle_context_t *ctx, + const celix_service_use_options_t *opts, bool singular) { if (opts == NULL || opts->filter.serviceName == NULL) { - return false; + return 0; } - celix_bundle_context_use_service_data_t data = {0}; - data.ctx = ctx; - data.opts = opts; - bool called = false; - if (celix_framework_isCurrentThreadTheEventLoop(ctx->framework)) { - // Ignore timeout: blocking the event loop prevents any progress to be made - celix_bundleContext_useServiceWithOptions_1_CreateServiceTracker(&data); - celix_bundleContext_useServiceWithOptions_2_UseServiceTracker(&data); - celix_serviceTracker_destroy(data.svcTracker); - return data.called; + fw_log(ctx->framework->logger, + CELIX_LOG_LEVEL_ERROR, + "Cannot use services on the event loop thread. Ignoring call."); + return 0; } - long eventId = celix_framework_fireGenericEvent(ctx->framework, -1, celix_bundle_getId(ctx->bundle), "create service tracker for celix_bundleContext_useServiceWithOptions", &data, celix_bundleContext_useServiceWithOptions_1_CreateServiceTracker, NULL, NULL); - celix_framework_waitForGenericEvent(ctx->framework, eventId); - + celix_service_tracking_options_t trackingOpts = CELIX_EMPTY_SERVICE_TRACKING_OPTIONS; + memcpy(&trackingOpts.filter, &opts->filter, sizeof(opts->filter)); + long trkId = celix_bundleContext_trackServicesWithOptions(ctx, &trackingOpts); + if (trkId < 0) { + return 0; + } - if(opts->flags & CELIX_SERVICE_USE_DIRECT) { - if(opts->flags & CELIX_SERVICE_USE_SOD) { - // check CelixBundleContextServicesTestSuite.UseServiceOnDemandDirectlyWithAsyncRegisterTest to see what is "service on demand". - celix_framework_waitUntilNoPendingRegistration(ctx->framework); + //Note because useService* is primary an API used in tests, waiting for events to that service-on-demand is + //working and useService* calls work after an async service (un)registration. + celix_framework_waitForEmptyEventQueue(ctx->framework); + + celix_tracked_service_use_options_t useOpts = CELIX_EMPTY_TRACKER_SERVICE_USE_OPTIONS; + useOpts.callbackHandle = opts->callbackHandle; + useOpts.use = opts->use; + useOpts.useWithProperties = opts->useWithProperties; + useOpts.useWithOwner = opts->useWithOwner; + + size_t count; + if (singular) { + struct timespec start = celix_gettime(CLOCK_MONOTONIC); + bool called = celix_bundleContext_useTrackedServiceWithOptions(ctx, trkId, &useOpts); + if (!called && opts->waitTimeoutInSeconds > 0) { + while (!called && celix_elapsedtime(CLOCK_MONOTONIC, start) < opts->waitTimeoutInSeconds) { + usleep(1000); + called = celix_bundleContext_useTrackedServiceWithOptions(ctx, trkId, &useOpts); + } } - called = celix_serviceTracker_useHighestRankingService(data.svcTracker, NULL, opts->waitTimeoutInSeconds, opts->callbackHandle, opts->use, opts->useWithProperties, opts->useWithOwner); + count = called ? 1 : 0; } else { - struct timespec startTime = celix_gettime(CLOCK_MONOTONIC); - bool useServiceIsDone = false; - do { - eventId = celix_framework_fireGenericEvent(ctx->framework, -1, celix_bundle_getId(ctx->bundle), "use service tracker for celix_bundleContext_useServiceWithOptions", &data, celix_bundleContext_useServiceWithOptions_2_UseServiceTracker, NULL, NULL); - celix_framework_waitForGenericEvent(ctx->framework, eventId); - - bool timeoutNotUsed = opts->waitTimeoutInSeconds == 0; - bool timeoutExpired = celix_elapsedtime(CLOCK_MONOTONIC, startTime) > opts->waitTimeoutInSeconds; - - called = data.called; - - useServiceIsDone = timeoutNotUsed || timeoutExpired || called; - if (!useServiceIsDone) { - usleep(10); - } - } while (!useServiceIsDone); + count = celix_bundleContext_useTrackedServicesWithOptions(ctx, trkId, &useOpts); } - - eventId = celix_framework_fireGenericEvent(ctx->framework, -1, celix_bundle_getId(ctx->bundle), "close service tracker for celix_bundleContext_useServiceWithOptions", data.svcTracker, (void *)celix_serviceTracker_destroy, NULL, NULL); - celix_framework_waitForGenericEvent(ctx->framework, eventId); - - return called; + celix_bundleContext_stopTracker(ctx, trkId); + return count; } -static void celix_bundleContext_useServicesWithOptions_2_UseServiceTracker(void *data) { - celix_bundle_context_use_service_data_t* d = data; - d->count = celix_serviceTracker_useServices(d->svcTracker, d->opts->filter.serviceName, d->opts->callbackHandle, d->opts->use, d->opts->useWithProperties, d->opts->useWithOwner); +bool celix_bundleContext_useServiceWithOptions( + celix_bundle_context_t *ctx, + const celix_service_use_options_t *opts) { + return celix_bundleContext_useServicesInternal(ctx, opts, true) > 0; } size_t celix_bundleContext_useServicesWithOptions( celix_bundle_context_t *ctx, const celix_service_use_options_t *opts) { - if (opts == NULL || opts->filter.serviceName == NULL) { - return 0; - } - - celix_bundle_context_use_service_data_t data = {0}; - data.ctx = ctx; - data.opts = opts; - - if (celix_framework_isCurrentThreadTheEventLoop(ctx->framework)) { - celix_bundleContext_useServiceWithOptions_1_CreateServiceTracker(&data); - celix_bundleContext_useServicesWithOptions_2_UseServiceTracker(&data); - celix_serviceTracker_destroy(data.svcTracker); - return data.count; - } - - long eventId = celix_framework_fireGenericEvent(ctx->framework, -1, celix_bundle_getId(ctx->bundle), "create service tracker for celix_bundleContext_useServicesWithOptions", &data, celix_bundleContext_useServiceWithOptions_1_CreateServiceTracker, NULL, NULL); - celix_framework_waitForGenericEvent(ctx->framework, eventId); - - if (opts->flags & CELIX_SERVICE_USE_DIRECT) { - if(opts->flags & CELIX_SERVICE_USE_SOD) { - // check CelixBundleContextServicesTestSuite.UseServicesOnDemandDirectlyWithAsyncRegisterTest to see what is "service on demand". - 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); - celix_framework_waitForGenericEvent(ctx->framework, eventId); - } - - eventId = celix_framework_fireGenericEvent(ctx->framework, -1, celix_bundle_getId(ctx->bundle), "use service tracker for celix_bundleContext_useServicesWithOptions", data.svcTracker, (void *)celix_serviceTracker_destroy, NULL, NULL); - celix_framework_waitForGenericEvent(ctx->framework, eventId); - - return data.count; + return celix_bundleContext_useServicesInternal(ctx, opts, false); } long celix_bundleContext_trackServices(bundle_context_t* ctx, const char* serviceName) {