pnoltes commented on a change in pull request #399:
URL: https://github.com/apache/celix/pull/399#discussion_r818612305
##########
File path: libs/framework/src/bundle_context.c
##########
@@ -1181,78 +1182,70 @@ static void
celix_bundleContext_useServiceWithOptions_2_UseServiceTracker(void *
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, d->opts->callbackHandle, d->opts->use,
d->opts->useWithProperties, d->opts->useWithOwner);
-}
-
-static void
celix_bundleContext_useServiceWithOptions_3_CloseServiceTracker(void *data) {
- celix_bundle_context_use_service_data_t* d = data;
- assert(celix_framework_isCurrentThreadTheEventLoop(d->ctx->framework));
-
- celix_service_tracker_t *tracker = d->svcTracker;
- d->svcTracker = NULL;
-
- celix_serviceTracker_destroy(tracker);
+ 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) {
- if (opts == NULL) {
+ if (opts == NULL || opts->filter.serviceName == NULL) {
return false;
}
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_bundleContext_useServiceWithOptions_3_CloseServiceTracker(&data);
+ celix_serviceTracker_destroy(data.svcTracker);
return data.called;
}
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_framework_waitForEmptyEventQueue(ctx->framework); //ensure that a
useService wait if a listener hooks concept, which triggers an async service
registration
- struct timespec startTime = celix_gettime(CLOCK_MONOTONIC);
- bool useServiceIsDone = false;
- bool called = 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);
+ 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
Review comment:
That is an interesting scenario and would IMO indeed make it possible
to make better performance use service calls without breaking api or putting
extra stress on the service registry.
I also agree with making parts of the service tracker more accessible.
The idea of returning "service tracker ids" instead of pointers to a service
tracker has to do with ownership and difficulties in C to arrange or share
ownership. IMO is was too easy to forward service tracker (or service
registration) pointers without providing control of the service tracker
lifecycle or checks if the service tracker is still valid.
But indeed the downside is that the service tracker api is now hidden and we
could provide the api through the bundle context adds additional overhead.
Another note: The service tracker was designed for multi threading and has
not yet been updated for the fact that is should only be created, destroyed and
get services update from a single Celix event thread. I have not look into this
yet, but this could also mean that service trackers should be able to operate
without mutexes, but with atomics or even lock-free constructs instead.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
