PengZheng commented on a change in pull request #399:
URL: https://github.com/apache/celix/pull/399#discussion_r816607673
##########
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:
Now `CELIX_SERVICE_USE_DIRECT` will not support SOD by default, unless
explicitly requested by user.
Since last mails, I've thought about this. I see one potential problem with
direct registry usage: global lock contention, i.e. users interested in
different services will affect each other. I have the following scenario in
mind:
1. Create one tracker (on bundle start maybe).
2. Use service via the same tracker concurrently from multiple threads.
3. Destroy the tracker (on bundle stop maybe).
By design, users of different services use different trackers, global lock
contention is reduced into per tracker lock contention. Furthermore,
performance of concurrent use of the same tracker can be further improved by
using read-write lock instead of the tracker lock.
However, current tracker implementation is sub-optimal: currently there is
no way to obtain opaque tracker pointer, only the tracker id is returned. The
translation between tracker id and opaque tracker pointer involves bundle
context lock. Thus our design only reduces global lock contention into per
bundle lock contention, not the optimal per tracker lock contention (the same
bundle may be interested in more than one services, then these trackers affect
each other through the bundle context lock). If we can get opaque tracker
pointer from tracker id (service tracker is still created on the event loop),
then
`celix_serviceTracker_useHighestRankingService`/`celix_serviceTracker_useHighestRankingService`
can help us achieving optimal performance. Since no event loop is involved in
`celix_serviceTracker_useHighestRankingService`, its recursive usage (using a
service itself calling useService) together with async API may not hurt us any
more. IIRC, the intention of original OSGi serv
ice tracker is to avoid external synchronization implemented by user. By
re-exposing `celix_serviceTracker_useHighestRankingService` as public API, we
can recovery its original design purpose.
I planed to discuss it with you after the release happens, since it involves
public API change bigger than this PR.
PS: Currently I'm working on conan support. Hopefully, a PR will arrive this
week. @pnoltes
--
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
