You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@celix.apache.org by GitBox <gi...@apache.org> on 2020/10/06 18:57:06 UTC
[GitHub] [celix] pnoltes opened a new pull request #286: Feature/async svc registration
pnoltes opened a new pull request #286:
URL: https://github.com/apache/celix/pull/286
This PR **adds a async API for service registration, use services, service tracker, service tracker tracker (listener hook) and bundle trackers**.
This can be used to - for example - directly get a service id, but let the actual service registration be done at a later moment. This should make it easier to break long chains of registration events which can easily run into dead locks.
(So register a service async in a lock and at the service id to a list/map without worrying that a register service event can chain back to the same object and run into a dead lock)
An other big change is that all service registration, use service and trackers creations/destructions are done on the one the same event loop, event the non async API calls. This means that with this PR **all events for trackers and use service callbacks are done on the event loop**.
The event loop is also updated to first use a ring buffer structure and if this fills fallback to dynamicly allocated events.
Also several thread issues are solved, found through the added async and event stress unit tests.
The findService(s) functionality is moved from the service tracker to the service registry to ensure that this can be called without the needing/using the event loop thread.
Lastly the log_admin is updated as an example how the async api can be used.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] Oipo commented on pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
Oipo commented on pull request #286:
URL: https://github.com/apache/celix/pull/286#issuecomment-753642764
FYI, I ran `` through callgrind, and this is the result. The .txt file can be opened with kcachegrind.
[callgrind.out.90753.txt](https://github.com/apache/celix/files/5761879/callgrind.out.90753.txt)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] Oipo commented on pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
Oipo commented on pull request #286:
URL: https://github.com/apache/celix/pull/286#issuecomment-706998638
The address sanitizer finds the following:
[stackuseafterfree.txt](https://github.com/apache/celix/files/5364178/stackuseafterfree.txt)
The test uses the count variable asynchronously, leading to situations where the address to count is used after the test has quit.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] codecov-io edited a comment on pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #286:
URL: https://github.com/apache/celix/pull/286#issuecomment-711176467
# [Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=h1) Report
> Merging [#286](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=desc) (a32487d) into [master](https://codecov.io/gh/apache/celix/commit/7a26aea42fd33849e54a33d69969e42d096b9dcd?el=desc) (7a26aea) will **increase** coverage by `0.58%`.
> The diff coverage is `81.45%`.
[](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #286 +/- ##
==========================================
+ Coverage 66.69% 67.28% +0.58%
==========================================
Files 147 147
Lines 29947 30412 +465
==========================================
+ Hits 19974 20462 +488
+ Misses 9973 9950 -23
```
| [Impacted Files](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [libs/framework/src/celix\_framework\_factory.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2NlbGl4X2ZyYW1ld29ya19mYWN0b3J5LmM=) | `87.50% <ø> (-0.74%)` | :arrow_down: |
| [bundles/pubsub/pubsub\_utils/src/pubsub\_matching.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3V0aWxzL3NyYy9wdWJzdWJfbWF0Y2hpbmcuYw==) | `78.62% <50.00%> (+3.29%)` | :arrow_up: |
| [bundles/http\_admin/http\_admin/src/http\_admin.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9odHRwX2FkbWluL2h0dHBfYWRtaW4vc3JjL2h0dHBfYWRtaW4uYw==) | `68.04% <66.66%> (+2.63%)` | :arrow_up: |
| [libs/framework/src/service\_tracker.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfdHJhY2tlci5j) | `79.50% <76.47%> (+2.36%)` | :arrow_up: |
| [libs/framework/src/bundle.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2J1bmRsZS5j) | `68.31% <76.92%> (+0.08%)` | :arrow_up: |
| [libs/framework/src/bundle\_context.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2J1bmRsZV9jb250ZXh0LmM=) | `69.93% <78.48%> (-0.19%)` | :arrow_down: |
| [libs/framework/src/framework.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2ZyYW1ld29yay5j) | `74.66% <83.54%> (-0.66%)` | :arrow_down: |
| [libs/framework/src/service\_registry.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfcmVnaXN0cnkuYw==) | `73.35% <86.90%> (+1.66%)` | :arrow_up: |
| [...dles/pubsub/pubsub\_spi/src/pubsub\_endpoint\_match.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3NwaS9zcmMvcHVic3ViX2VuZHBvaW50X21hdGNoLmM=) | `56.37% <87.50%> (-2.23%)` | :arrow_down: |
| [libs/utils/src/utils.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy91dGlscy9zcmMvdXRpbHMuYw==) | `87.39% <90.90%> (+4.66%)` | :arrow_up: |
| ... and [29 more](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=footer). Last update [7a26aea...a32487d](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] Oipo commented on a change in pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
Oipo commented on a change in pull request #286:
URL: https://github.com/apache/celix/pull/286#discussion_r545940469
##########
File path: libs/framework/src/bundle_context.c
##########
@@ -836,20 +1316,111 @@ long celix_bundleContext_trackServices(
opts.callbackHandle = callbackHandle;
opts.add = add;
opts.remove = remove;
- return celix_bundleContext_trackServicesWithOptions(ctx, &opts);
+ return celix_bundleContext_trackServicesWithOptionsInternal(ctx, &opts, false);
}
+long celix_bundleContext_trackServicesAsync(
+ celix_bundle_context_t* ctx,
+ const char* serviceName,
+ void* callbackHandle,
+ void (*add)(void* handle, void* svc),
+ void (*remove)(void* handle, void* svc)) {
+ celix_service_tracking_options_t opts = CELIX_EMPTY_SERVICE_TRACKING_OPTIONS;
+ opts.filter.serviceName = serviceName;
+ opts.callbackHandle = callbackHandle;
+ opts.add = add;
+ opts.remove = remove;
+ return celix_bundleContext_trackServicesWithOptionsInternal(ctx, &opts, true);
+}
-long celix_bundleContext_trackServicesWithOptions(bundle_context_t *ctx, const celix_service_tracking_options_t *opts) {
- long trackerId = -1;
- celix_service_tracker_t *tracker = celix_serviceTracker_createWithOptions(ctx, opts);
+
+static void celix_bundleContext_createTrackerOnEventLoop(void *data) {
+ celix_bundle_context_service_tracker_entry_t* entry = data;
+ assert(celix_framework_isCurrentThreadTheEventLoop(entry->ctx->framework));
+ celix_service_tracker_t *tracker = celix_serviceTracker_createWithOptions(entry->ctx, &entry->opts);
if (tracker != NULL) {
+ celixThreadMutex_lock(&entry->ctx->mutex);
+ entry->tracker = tracker;
+ celixThreadMutex_unlock(&entry->ctx->mutex);
+ } else {
+ fw_log(entry->ctx->framework->logger, CELIX_LOG_LEVEL_ERROR, "Cannot create tracker for bnd %s (%li)", celix_bundle_getSymbolicName(entry->ctx->bundle), celix_bundle_getId(entry->ctx->bundle));
+ }
+}
+
+static void celix_bundleContext_doneCreatingTrackerOnEventLoop(void *data) {
+ celix_bundle_context_service_tracker_entry_t* entry = data;
+ if (entry->trackerCreatedCallback != NULL) {
+ entry->trackerCreatedCallback(entry->trackerCreatedCallbackData);
+ }
+}
+
+
+
+static long celix_bundleContext_trackServicesWithOptionsInternal(celix_bundle_context_t *ctx, const celix_service_tracking_options_t *opts, bool async) {
+ if (ctx == NULL) {
+ return -1L;
+ } else if (opts == NULL) {
+ fw_log(ctx->framework->logger, CELIX_LOG_LEVEL_ERROR, "Cannot track services with a NULL service tracking options argument");
+ return -1L;
+ }
+
+ if (opts->filter.serviceName == NULL) {
+ fw_log(ctx->framework->logger, CELIX_LOG_LEVEL_DEBUG, "Starting a tracker for any services");
+ }
+
+ if (!async && celix_framework_isCurrentThreadTheEventLoop(ctx->framework)) {
+ //already in event loop thread. To keep the old behavior just create the tracker traditionally (chained in the current thread).
+ celix_service_tracker_t *tracker = celix_serviceTracker_createWithOptions(ctx, opts);
+ long trackerId = -1L;
+ if (tracker != NULL) {
+ celix_bundle_context_service_tracker_entry_t* entry = calloc(1, sizeof(*entry));
+ entry->ctx = ctx;
+ entry->tracker = tracker;
+ entry->opts = *opts;
+ entry->createEventId = -1;
+ celixThreadMutex_lock(&ctx->mutex);
+ entry->trackerId = ctx->nextTrackerId++;
+ trackerId = entry->trackerId;
+ hashMap_put(ctx->serviceTrackers, (void *) trackerId, entry);
+ celixThreadMutex_unlock(&ctx->mutex);
+ }
+ return trackerId;
+ } else {
+ if (!async) {
Review comment:
:do_not_litter:
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] codecov-io edited a comment on pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #286:
URL: https://github.com/apache/celix/pull/286#issuecomment-711176467
# [Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=h1) Report
> Merging [#286](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=desc) (ba24349) into [master](https://codecov.io/gh/apache/celix/commit/dd4dca1d52c36a794dfac71867f0abc3fd81241c?el=desc) (dd4dca1) will **increase** coverage by `1.03%`.
> The diff coverage is `82.43%`.
[](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #286 +/- ##
==========================================
+ Coverage 68.17% 69.20% +1.03%
==========================================
Files 148 149 +1
Lines 30088 31622 +1534
==========================================
+ Hits 20511 21885 +1374
- Misses 9577 9737 +160
```
| [Impacted Files](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [libs/framework/src/celix\_framework\_factory.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2NlbGl4X2ZyYW1ld29ya19mYWN0b3J5LmM=) | `87.50% <ø> (-0.74%)` | :arrow_down: |
| [bundles/pubsub/pubsub\_utils/src/pubsub\_matching.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3V0aWxzL3NyYy9wdWJzdWJfbWF0Y2hpbmcuYw==) | `78.62% <50.00%> (+3.29%)` | :arrow_up: |
| [bundles/http\_admin/http\_admin/src/http\_admin.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9odHRwX2FkbWluL2h0dHBfYWRtaW4vc3JjL2h0dHBfYWRtaW4uYw==) | `68.04% <66.66%> (+2.63%)` | :arrow_up: |
| [libs/framework/src/bundle.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2J1bmRsZS5j) | `68.31% <76.92%> (+0.08%)` | :arrow_up: |
| [libs/framework/src/service\_tracker.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfdHJhY2tlci5j) | `81.98% <78.82%> (+1.12%)` | :arrow_up: |
| [libs/framework/src/bundle\_context.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2J1bmRsZV9jb250ZXh0LmM=) | `70.32% <79.22%> (+0.20%)` | :arrow_up: |
| [libs/framework/src/framework.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2ZyYW1ld29yay5j) | `74.66% <83.54%> (-0.88%)` | :arrow_down: |
| [...dles/pubsub/pubsub\_spi/src/pubsub\_endpoint\_match.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3NwaS9zcmMvcHVic3ViX2VuZHBvaW50X21hdGNoLmM=) | `56.37% <87.50%> (-2.23%)` | :arrow_down: |
| [libs/framework/src/service\_registry.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfcmVnaXN0cnkuYw==) | `73.87% <89.28%> (+2.17%)` | :arrow_up: |
| [libs/utils/src/utils.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy91dGlscy9zcmMvdXRpbHMuYw==) | `87.39% <90.90%> (+4.66%)` | :arrow_up: |
| ... and [38 more](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=footer). Last update [dd4dca1...ba24349](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] Oipo commented on a change in pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
Oipo commented on a change in pull request #286:
URL: https://github.com/apache/celix/pull/286#discussion_r503153562
##########
File path: libs/framework/src/framework.c
##########
@@ -2528,13 +2631,108 @@ bool celix_framework_startBundle(celix_framework_t *fw, long bndId) {
}
void celix_framework_waitForEmptyEventQueue(celix_framework_t *fw) {
+ assert(!celix_framework_isCurrentThreadTheEventLoop(fw));
+
celixThreadMutex_lock(&fw->dispatcher.mutex);
- while ((celix_arrayList_size(fw->dispatcher.requests) + fw->dispatcher.nrOfLocalRequest) != 0) {
+ while (fw->dispatcher.eventQueueSize > 0 || celix_arrayList_size(fw->dispatcher.dynamicEventQueue) > 0) {
celixThreadCondition_wait(&fw->dispatcher.cond, &fw->dispatcher.mutex);
}
celixThreadMutex_unlock(&fw->dispatcher.mutex);
}
+void celix_framework_waitForEvents(celix_framework_t* fw, long bndId) {
+ assert(!celix_framework_isCurrentThreadTheEventLoop(fw));
+
+ celixThreadMutex_lock(&fw->dispatcher.mutex);
+ bool eventInProgress = true;
+ while (eventInProgress) {
+ eventInProgress = false;
+ for (int i = 0; i < fw->dispatcher.eventQueueSize; ++i) {
+ int index = (fw->dispatcher.eventQueueFirstEntry + i) % CELIX_FRAMEWORK_STATIC_EVENT_QUEUE_SIZE;
+ celix_framework_event_t* e = &fw->dispatcher.eventQueue[index];
+ if (e->bndEntry != NULL && e->bndEntry->bndId == bndId) {
+ eventInProgress = true;
+ break;
+ }
+ }
+ for (int i = 0; !eventInProgress && i < celix_arrayList_size(fw->dispatcher.dynamicEventQueue); ++i) {
+ celix_framework_event_t* e = celix_arrayList_get(fw->dispatcher.dynamicEventQueue, i);
+ if (e->bndEntry != NULL && e->bndEntry->bndId == bndId) {
+ eventInProgress = true;
+ break;
+ }
+ }
+ if (eventInProgress) {
+ celixThreadCondition_timedwaitRelative(&fw->dispatcher.cond, &fw->dispatcher.mutex, 5, 0);
+ }
+ }
+ celixThreadMutex_unlock(&fw->dispatcher.mutex);
+}
+
+
void celix_framework_setLogCallback(celix_framework_t* fw, void* logHandle, void (*logFunction)(void* handle, celix_log_level_e level, const char* file, const char *function, int line, const char *format, va_list formatArgs)) {
celix_frameworkLogger_setLogCallback(fw->logger, logHandle, logFunction);
+}
+
+
+long celix_framework_fireGenericEvent(framework_t* fw, long eventId, long bndId, const char *eventName, void* processData, void (*processCallback)(void *data), void* doneData, void (*doneCallback)(void* doneData)) {
+ celix_framework_bundle_entry_t* bndEntry = NULL;
+ if (bndId >=0) {
+ bndEntry = fw_bundleEntry_getBundleEntryAndIncreaseUseCount(fw, bndId);
+ if (bndEntry == NULL) {
+ fw_log(fw->logger, CELIX_LOG_LEVEL_ERROR, "Cannot find bundle for id %li", bndId);
+ return -1L;
+ }
+ }
+
+ if (eventId < 0) {
+ eventId = celix_framework_nextEventId(fw);
+ }
+
+ celix_framework_event_t event;
+ memset(&event, 0, sizeof(event));
+ event.type = CELIX_GENERIC_EVENT;
+ event.bndEntry = bndEntry;
+ event.genericEventId = eventId;
+ event.genericEventName = eventName;
+ event.genericProcessData = processData;
+ event.genericProcess = processCallback;
+ event.doneData = doneData;
+ event.doneCallback = doneCallback;
+ celix_framework_addToEventQueue(fw, &event);
+
+ return eventId;
+}
+
+long celix_framework_nextEventId(framework_t *fw) {
+ return __atomic_fetch_add(&fw->nextGenericEventId, 1, __ATOMIC_SEQ_CST);;
Review comment:
Same goes for `service_registry.c`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] codecov-io edited a comment on pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #286:
URL: https://github.com/apache/celix/pull/286#issuecomment-711176467
# [Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=h1) Report
> Merging [#286](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=desc) (10b5234) into [master](https://codecov.io/gh/apache/celix/commit/dd4dca1d52c36a794dfac71867f0abc3fd81241c?el=desc) (dd4dca1) will **increase** coverage by `0.51%`.
> The diff coverage is `82.46%`.
[](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #286 +/- ##
==========================================
+ Coverage 68.17% 68.68% +0.51%
==========================================
Files 148 149 +1
Lines 30088 30640 +552
==========================================
+ Hits 20511 21046 +535
- Misses 9577 9594 +17
```
| [Impacted Files](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [libs/framework/src/celix\_framework\_factory.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2NlbGl4X2ZyYW1ld29ya19mYWN0b3J5LmM=) | `87.50% <ø> (-0.74%)` | :arrow_down: |
| [bundles/pubsub/pubsub\_utils/src/pubsub\_matching.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3V0aWxzL3NyYy9wdWJzdWJfbWF0Y2hpbmcuYw==) | `78.62% <50.00%> (+3.29%)` | :arrow_up: |
| [bundles/http\_admin/http\_admin/src/http\_admin.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9odHRwX2FkbWluL2h0dHBfYWRtaW4vc3JjL2h0dHBfYWRtaW4uYw==) | `68.04% <66.66%> (+2.63%)` | :arrow_up: |
| [libs/framework/src/bundle.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2J1bmRsZS5j) | `68.31% <76.92%> (+0.08%)` | :arrow_up: |
| [libs/framework/src/service\_tracker.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfdHJhY2tlci5j) | `81.98% <78.82%> (+1.12%)` | :arrow_up: |
| [libs/framework/src/bundle\_context.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2J1bmRsZV9jb250ZXh0LmM=) | `70.35% <79.31%> (+0.23%)` | :arrow_up: |
| [libs/framework/src/framework.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2ZyYW1ld29yay5j) | `74.66% <83.54%> (-0.88%)` | :arrow_down: |
| [...dles/pubsub/pubsub\_spi/src/pubsub\_endpoint\_match.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3NwaS9zcmMvcHVic3ViX2VuZHBvaW50X21hdGNoLmM=) | `56.37% <87.50%> (-2.23%)` | :arrow_down: |
| [libs/framework/src/service\_registry.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfcmVnaXN0cnkuYw==) | `73.87% <89.28%> (+2.17%)` | :arrow_up: |
| [libs/utils/src/utils.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy91dGlscy9zcmMvdXRpbHMuYw==) | `87.39% <90.90%> (+4.66%)` | :arrow_up: |
| ... and [36 more](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=footer). Last update [dd4dca1...10b5234](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] codecov-io edited a comment on pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #286:
URL: https://github.com/apache/celix/pull/286#issuecomment-711176467
# [Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=h1) Report
> Merging [#286](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=desc) (1a0a236) into [master](https://codecov.io/gh/apache/celix/commit/ab13f6da69e42851f5b065cddfe59ab14eb2b02c?el=desc) (ab13f6d) will **increase** coverage by `0.21%`.
> The diff coverage is `77.88%`.
[](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #286 +/- ##
==========================================
+ Coverage 66.98% 67.19% +0.21%
==========================================
Files 147 147
Lines 29946 30343 +397
==========================================
+ Hits 20058 20390 +332
- Misses 9888 9953 +65
```
| [Impacted Files](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [libs/framework/src/celix\_framework\_factory.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2NlbGl4X2ZyYW1ld29ya19mYWN0b3J5LmM=) | `87.50% <ø> (-0.74%)` | :arrow_down: |
| [bundles/http\_admin/http\_admin/src/http\_admin.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9odHRwX2FkbWluL2h0dHBfYWRtaW4vc3JjL2h0dHBfYWRtaW4uYw==) | `68.04% <66.66%> (+2.63%)` | :arrow_up: |
| [libs/framework/src/bundle\_context.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2J1bmRsZV9jb250ZXh0LmM=) | `66.51% <72.51%> (-3.61%)` | :arrow_down: |
| [libs/framework/src/service\_tracker.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfdHJhY2tlci5j) | `79.50% <76.47%> (+2.18%)` | :arrow_up: |
| [libs/framework/src/bundle.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2J1bmRsZS5j) | `68.31% <76.92%> (+0.08%)` | :arrow_up: |
| [libs/framework/src/framework.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2ZyYW1ld29yay5j) | `73.38% <78.53%> (-2.15%)` | :arrow_down: |
| [libs/framework/src/service\_registry.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfcmVnaXN0cnkuYw==) | `73.35% <86.90%> (+1.66%)` | :arrow_up: |
| [...dles/pubsub/pubsub\_spi/src/pubsub\_endpoint\_match.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3NwaS9zcmMvcHVic3ViX2VuZHBvaW50X21hdGNoLmM=) | `56.37% <87.50%> (-2.23%)` | :arrow_down: |
| [bundles/logging/log\_admin/src/celix\_log\_admin.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9sb2dnaW5nL2xvZ19hZG1pbi9zcmMvY2VsaXhfbG9nX2FkbWluLmM=) | `99.73% <100.00%> (-0.01%)` | :arrow_down: |
| [bundles/pubsub/pubsub\_spi/src/pubsub\_endpoint.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3NwaS9zcmMvcHVic3ViX2VuZHBvaW50LmM=) | `75.00% <100.00%> (-2.15%)` | :arrow_down: |
| ... and [23 more](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=footer). Last update [ab13f6d...1a0a236](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] dhbfischer commented on a change in pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
dhbfischer commented on a change in pull request #286:
URL: https://github.com/apache/celix/pull/286#discussion_r545154911
##########
File path: libs/framework/src/bundle_context.c
##########
@@ -461,52 +505,117 @@ long celix_bundleContext_registerServiceWithOptions(bundle_context_t *ctx, const
}
const char *lang = opts->serviceLanguage != NULL && strncmp("", opts->serviceLanguage, 1) != 0 ? opts->serviceLanguage : CELIX_FRAMEWORK_SERVICE_C_LANGUAGE;
celix_properties_set(props, CELIX_FRAMEWORK_SERVICE_LANGUAGE, lang);
- if (opts->serviceName != NULL && strncmp("", opts->serviceName, 1) != 0) {
- if (opts->factory != NULL) {
- reg = celix_framework_registerServiceFactory(ctx->framework, ctx->bundle, opts->serviceName, opts->factory, props);
- } else {
- bundleContext_registerService(ctx, opts->serviceName, opts->svc, props, ®);
- }
- svcId = serviceRegistration_getServiceId(reg); //save to call with NULL
+
+ long svcId = -1;
+ if (!async && celix_framework_isCurrentThreadTheEventLoop(ctx->framework)) {
+ /*
+ * Note already on event loop, cannot register the service async, because we cannot wait a future event (the
+ * service registration) the event loop.
+ *
+ * So in this case we handle the service registration the "traditional way" and call the sync fw service
+ * registrations versions on the event loop thread
+ */
+
+ svcId = celix_framework_registerService(ctx->framework, ctx->bundle, opts->serviceName, opts->svc, opts->factory, props);
} else {
- framework_logIfError(ctx->framework->logger, CELIX_ILLEGAL_ARGUMENT, NULL, "Required serviceName argument is NULL");
+ svcId = celix_framework_registerServiceAsync(ctx->framework, ctx->bundle, opts->serviceName, opts->svc, opts->factory, props, opts->asyncData, opts->asyncCallback, NULL, NULL);
+ if (!async && svcId >= 0) {
+ //note on event loop thread, but in a sync call, so waiting till service registration is concluded
+ celix_bundleContext_waitForAsyncRegistration(ctx, svcId);
+ }
}
+
+
if (svcId < 0) {
properties_destroy(props);
} else {
celixThreadMutex_lock(&ctx->mutex);
- arrayList_add(ctx->svcRegistrations, reg);
+ celix_arrayList_addLong(ctx->svcRegistrations, svcId);
celixThreadMutex_unlock(&ctx->mutex);
+ if (!async) {
Review comment:
Why calling an async callback in a non async function?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] Oipo commented on a change in pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
Oipo commented on a change in pull request #286:
URL: https://github.com/apache/celix/pull/286#discussion_r501061104
##########
File path: bundles/pubsub/pubsub_spi/src/pubsub_endpoint.c
##########
@@ -170,8 +170,9 @@ celix_properties_t* pubsubEndpoint_createFromPublisherTrackerInfo(bundle_context
data.topic = topic;
celix_bundleContext_useBundle(ctx, bundleId, &data, retrieveTopicProperties);
+ pubsubEndpoint_setFields(ep, fwUUID, scope, topic, PUBSUB_PUBLISHER_ENDPOINT_TYPE, NULL, NULL, NULL, data.props);
+
if (data.props != NULL) {
Review comment:
You can just remove this if statement, celix_properties_destroy also checks for NULL.
##########
File path: libs/framework/gtest/src/bundle_context_bundles_tests.cpp
##########
@@ -378,6 +378,90 @@ TEST_F(CelixBundleContextBundlesTests, trackBundlesTest) {
celix_bundleContext_stopTracker(ctx, trackerId);
};
+
+TEST_F(CelixBundleContextBundlesTests, trackBundlesTestAsync) {
+ struct data {
+ std::atomic<int> installedCount{0};
+ std::atomic<int> startedCount{0};
+ std::atomic<int> stoppedCount{0};
+ };
+ struct data data;
+
+ auto installed = [](void *handle, const bundle_t *bnd) {
+ auto *d = static_cast<struct data*>(handle);
+ EXPECT_TRUE(bnd != nullptr);
+ d->installedCount.fetch_add(1);
Review comment:
Stylistic choice: `installedCount.fetch_add(1)` is semantically the same as `installedCount += 1`. It's only when you want to specifiy memory order that the former is the only option.
##########
File path: libs/framework/gtest/src/bundle_context_bundles_tests.cpp
##########
@@ -378,6 +378,90 @@ TEST_F(CelixBundleContextBundlesTests, trackBundlesTest) {
celix_bundleContext_stopTracker(ctx, trackerId);
};
+
+TEST_F(CelixBundleContextBundlesTests, trackBundlesTestAsync) {
+ struct data {
+ std::atomic<int> installedCount{0};
+ std::atomic<int> startedCount{0};
+ std::atomic<int> stoppedCount{0};
+ };
+ struct data data;
+
+ auto installed = [](void *handle, const bundle_t *bnd) {
+ auto *d = static_cast<struct data*>(handle);
+ EXPECT_TRUE(bnd != nullptr);
+ d->installedCount.fetch_add(1);
+ };
+
+ auto started = [](void *handle, const bundle_t *bnd) {
+ auto *d = static_cast<struct data*>(handle);
+ EXPECT_TRUE(bnd != nullptr);
+ d->startedCount.fetch_add(1);
+ };
+
+ auto stopped = [](void *handle, const bundle_t *bnd) {
+ auto *d = static_cast<struct data*>(handle);
+ if (bnd == nullptr) {
+ celix_logUtils_logToStdout("test", CELIX_LOG_LEVEL_ERROR, "bnd should not be null");
+ }
+ EXPECT_TRUE(bnd != nullptr);
+ d->stoppedCount.fetch_add(1);
+ };
+
+ celix_bundle_tracking_options_t opts{};
+ opts.callbackHandle = static_cast<void*>(&data);
+ opts.onInstalled = installed;
+ opts.onStarted = started;
+ opts.onStopped = stopped;
+
+ long bundleId1 = celix_bundleContext_installBundle(ctx, TEST_BND1_LOC, true);
+ celix_framework_waitForEmptyEventQueue(fw);
+ EXPECT_TRUE(bundleId1 >= 0);
+
+ /*
+ * NOTE for bundles already installed (TEST_BND1) the callbacks are called on the
+ * thread of celix_bundleContext_trackBundlesWithOptions.
+ * For Bundles installed after the celix_bundleContext_trackBundlesWithOptions function
+ * the called are called on the Celix framework event queue thread.
+ */
+ long trackerId = celix_bundleContext_trackBundlesWithOptionsAsync(ctx, &opts);
+ celix_bundleContext_waitForAsyncTracker(ctx, trackerId);
+ EXPECT_EQ(1, data.installedCount.load());
Review comment:
Same stylistic choice here, the implicit conversion to int is the same as `.load()`.
##########
File path: libs/utils/src/utils.c
##########
@@ -150,9 +150,9 @@ int utils_compareServiceIdsAndRanking(unsigned long servId, long servRank, unsig
if (servId == otherServId) {
result = 0;
} else if (servRank != otherServRank) {
- result = servRank < otherServRank ? -1 : 1;
Review comment:
Is this a bug or a breaking change?
##########
File path: libs/framework/include/celix_framework.h
##########
@@ -156,6 +156,12 @@ void celix_framework_waitForEmptyEventQueue(celix_framework_t *fw);
void celix_framework_setLogCallback(celix_framework_t* fw, void* logHandle, void (*logFunction)(void* handle, celix_log_level_e level, const char* file, const char *function, int line, const char *format, va_list formatArgs));
+/**
+ * wait till all events for the bundle identified by the bndId are processed.
+ */
+void celix_framework_waitForEvents(celix_framework_t* fw, long bndId);
Review comment:
A better name would probably be `celix_framework_waitUntilNoEventsForBnd` or something
##########
File path: libs/framework/include/celix_bundle_context.h
##########
@@ -382,30 +546,74 @@ typedef struct celix_service_tracking_options {
.removeWithProperties = NULL, \
.setWithOwner = NULL, \
.addWithOwner = NULL, \
- .removeWithOwner = NULL}
+ .removeWithOwner = NULL, \
+ .trackerCreatedCallbackData = NULL, \
+ .trackerCreatedCallback = NULL }
#endif
/**
* Tracks services using the provided tracker options.
* The tracker options are only using during this call and can safely be freed/reused after this call returns.
*
+ * The service tracker will be created async on the Celix event loop thread. This means that the function can return
+ * before the tracker is created.
+ *
+ * @param ctx The bundle context.
+ * @param opts The pointer to the tracker options.
+ * @return the tracker id (>=0) or < 0 if unsuccessful.
+ */
+long celix_bundleContext_trackServicesWithOptionsAsync(celix_bundle_context_t *ctx, const celix_service_tracking_options_t *opts);
Review comment:
Does `celix_service_tracking_options_t` contain a completion handler? Would be useful here as well.
##########
File path: libs/framework/include/celix_bundle_context.h
##########
@@ -248,15 +353,39 @@ long celix_bundleContext_findServiceWithOptions(celix_bundle_context_t *ctx, con
*/
celix_array_list_t* celix_bundleContext_findServicesWithOptions(celix_bundle_context_t *ctx, const celix_service_filter_options_t *opts);
+/**
+ * track the highest ranking service with the provided serviceName.
+ * The highest ranking services will used for the callback.
+ * If a new and higher ranking services the callback with be called again with the new service.
+ * If a service is removed a the callback with be called with next highest ranking service or NULL as service.
+ *
+ * The service tracker will be created async on the Celix event loop thread. This means that the function can return
+ * before the tracker is created.
+ *
+ * @param ctx The bundle context.
+ * @param serviceName The required service name to track.
+ * If NULL is all service are tracked.
+ * @param callbackHandle The data pointer, which will be used in the callbacks
+ * @param set is a required callback, which will be called when a new highest ranking service is set.
+ * @return the tracker id (>=0) or < 0 if unsuccessful.
+ */
+long celix_bundleContext_trackServiceAsync(
+ celix_bundle_context_t* ctx,
+ const char* serviceName,
+ void* callbackHandle,
+ void (*set)(void* handle, void* svc)
Review comment:
A completion handler would be useful here as well
##########
File path: libs/framework/include/celix_bundle_context.h
##########
@@ -137,18 +190,50 @@ typedef struct celix_service_registration_options {
.serviceName = NULL, \
.properties = NULL, \
.serviceLanguage = NULL, \
- .serviceVersion = NULL }
+ .serviceVersion = NULL, \
+ .asyncData = NULL, \
+ .asyncCallback = NULL }
#endif
+/**
+ * Register a service to the Celix framework using the provided service registration options.
+ *
+ * The service will be registered async on the Celix event loop thread. This means that service registration is (probably)
+ * not yet concluded when this function returns, but is added to the event loop..
+ * Use celix_bundleContext_waitForAsyncRegistration to synchronise with the
+ * actual service registration in the framework's service registry.
+ *
+ * @param ctx The bundle context
+ * @param opts The pointer to the registration options. The options are only in the during registration call.
+ * @return The serviceId (>= 0) or -1 if the registration was unsuccessful and -2 if the registration was cancelled (@see celix_bundleContext_reserveSvcId).
+ */
+long celix_bundleContext_registerServiceWithOptionsAsync(celix_bundle_context_t *ctx, const celix_service_registration_options_t *opts);
Review comment:
Does `celix_service_registration_options_t` contain a completion handler? Would be useful here as well.
##########
File path: libs/framework/include/celix_bundle_context.h
##########
@@ -40,21 +40,63 @@ extern "C" {
#define OPTS_INIT
#endif
+
+/**
+ * Register a service to the Celix framework.
+ *
+ * The service will be registered async on the Celix event loop thread. This means that service registration is (probably)
+ * not yet concluded when this function returns, but is added to the event loop.
+ * Use celix_bundleContext_waitForAsyncRegistration to synchronise with the
Review comment:
What would probably fit the event loop idiom better is a completion handler. Though `celix_bundleContext_waitForAsyncRegistration` could still be allowed for when sleeping is fine.
##########
File path: libs/framework/include/celix_bundle_context.h
##########
@@ -266,13 +395,37 @@ long celix_bundleContext_trackService(
const char* serviceName,
void* callbackHandle,
void (*set)(void* handle, void* svc)
+); //__attribute__((deprecated("Use celix_bundleContext_trackServiceSync instead!")));
+
+/**
+ * track services with the provided serviceName.
+ *
+ * The service tracker will be created async on the Celix event loop thread. This means that the function can return
+ * before the tracker is created.
+ *
+ * @param ctx The bundle context.
+ * @param serviceName The required service name to track
+ * If NULL is all service are tracked.
+ * @param callbackHandle The data pointer, which will be used in the callbacks
+ * @param add is a required callback, which will be called when a service is added and initially for the existing service.
+ * @param remove is a required callback, which will be called when a service is removed
+ * @return the tracker id (>=0) or < 0 if unsuccessful.
+ */
+long celix_bundleContext_trackServicesAsync(
+ celix_bundle_context_t* ctx,
+ const char* serviceName,
+ void* callbackHandle,
+ void (*add)(void* handle, void* svc),
+ void (*remove)(void* handle, void* svc)
Review comment:
A completion handler would be useful here as well
##########
File path: libs/framework/include/service_registry.h
##########
@@ -134,6 +144,37 @@ bool celix_serviceRegistry_getServiceInfo(
long celix_serviceRegistry_nextSvcId(celix_service_registry_t* registry);
+/**
+ * Unregister service for the provided service id (owned by bnd).
+ * Will print an error if the service id is invalid
+ */
+void celix_serviceRegistry_unregisterService(celix_service_registry_t* registry, celix_bundle_t* bnd, long serviceId);
+
+
+/**
+ * Create a LDAP filter for the provided filter parts.
+ * @param serviceName The optional service name
+ * @param versionRange The optional version range
+ * @param filter The optional filter
Review comment:
name typo, should be `additionalFilter`. Also probably missing is why it is additional?
##########
File path: libs/framework/include/celix_bundle_context.h
##########
@@ -655,23 +863,46 @@ bool celix_bundleContext_startBundle(celix_bundle_context_t *ctx, long bndId);
*/
char* celix_bundleContext_getBundleSymbolicName(celix_bundle_context_t *ctx, long bndId);
+
/**
* track bundles
* The add bundle callback will also be called for already installed bundles.
Review comment:
add bundle callback?
##########
File path: libs/framework/include/celix_bundle_context.h
##########
@@ -655,23 +863,46 @@ bool celix_bundleContext_startBundle(celix_bundle_context_t *ctx, long bndId);
*/
char* celix_bundleContext_getBundleSymbolicName(celix_bundle_context_t *ctx, long bndId);
+
/**
* track bundles
* The add bundle callback will also be called for already installed bundles.
*
+ * The bundle tracker will be created async on the Celix event loop thread. This means that the function can return
+ * before the tracker is created.
+ *
* @param ctx The bundle context.
* @param callbackHandle The data pointer, which will be used in the callbacks
* @param add The callback which will be called for started bundles.
* @param remove The callback which will be called when bundles are stopped.
* @return The bundle tracker id or < 0 if unsuccessful.
*/
-long celix_bundleContext_trackBundles(
+long celix_bundleContext_trackBundlesAsync(
celix_bundle_context_t* ctx,
void* callbackHandle,
void (*onStarted)(void* handle, const celix_bundle_t *bundle),
void (*onStopped)(void *handle, const celix_bundle_t *bundle)
);
+/**
+ * track bundles
+ * The add bundle callback will also be called for already installed bundles.
+ *
+ * Note: please use celix_bundleContext_trackBundlesAsync instead.
+ *
+ * @param ctx The bundle context.
+ * @param callbackHandle The data pointer, which will be used in the callbacks
+ * @param add The callback which will be called for started bundles.
Review comment:
Yeah, probably onStarted/onStopped.
##########
File path: libs/framework/src/framework.c
##########
@@ -341,7 +331,7 @@ celix_status_t framework_destroy(framework_pt framework) {
const char *bndName = celix_bundle_getSymbolicName(bnd);
fw_log(framework->logger, CELIX_LOG_LEVEL_FATAL, "Cannot destroy framework. The use count of bundle %s (bnd id %li) is not 0, but %u.", bndName, entry->bndId, count);
celixThreadMutex_lock(&framework->dispatcher.mutex);
- int nrOfRequests = celix_arrayList_size(framework->dispatcher.requests);
+ int nrOfRequests = framework->dispatcher.eventQueueSize + celix_arrayList_size(framework->dispatcher.dynamicEventQueue);
Review comment:
Can't comment on line 369, but because it's destroying a locked mutex, thread sanitizer crashes.
##########
File path: libs/framework/src/service_registry.c
##########
@@ -1046,6 +920,137 @@ static inline void celix_waitAndDestroyServiceListener(celix_service_registry_se
free(entry);
}
+/**
+ * Create a LDAP filter for the provided filter parts.
Review comment:
This documentation should only be available in the header file, no?
##########
File path: libs/framework/include/celix_bundle_context.h
##########
@@ -655,23 +863,46 @@ bool celix_bundleContext_startBundle(celix_bundle_context_t *ctx, long bndId);
*/
char* celix_bundleContext_getBundleSymbolicName(celix_bundle_context_t *ctx, long bndId);
+
/**
* track bundles
* The add bundle callback will also be called for already installed bundles.
*
+ * The bundle tracker will be created async on the Celix event loop thread. This means that the function can return
+ * before the tracker is created.
+ *
* @param ctx The bundle context.
* @param callbackHandle The data pointer, which will be used in the callbacks
* @param add The callback which will be called for started bundles.
Review comment:
Probably onStarted/onStopped?
##########
File path: libs/framework/src/framework.c
##########
@@ -2528,13 +2631,108 @@ bool celix_framework_startBundle(celix_framework_t *fw, long bndId) {
}
void celix_framework_waitForEmptyEventQueue(celix_framework_t *fw) {
+ assert(!celix_framework_isCurrentThreadTheEventLoop(fw));
+
celixThreadMutex_lock(&fw->dispatcher.mutex);
- while ((celix_arrayList_size(fw->dispatcher.requests) + fw->dispatcher.nrOfLocalRequest) != 0) {
+ while (fw->dispatcher.eventQueueSize > 0 || celix_arrayList_size(fw->dispatcher.dynamicEventQueue) > 0) {
celixThreadCondition_wait(&fw->dispatcher.cond, &fw->dispatcher.mutex);
}
celixThreadMutex_unlock(&fw->dispatcher.mutex);
}
+void celix_framework_waitForEvents(celix_framework_t* fw, long bndId) {
+ assert(!celix_framework_isCurrentThreadTheEventLoop(fw));
+
+ celixThreadMutex_lock(&fw->dispatcher.mutex);
+ bool eventInProgress = true;
+ while (eventInProgress) {
+ eventInProgress = false;
+ for (int i = 0; i < fw->dispatcher.eventQueueSize; ++i) {
+ int index = (fw->dispatcher.eventQueueFirstEntry + i) % CELIX_FRAMEWORK_STATIC_EVENT_QUEUE_SIZE;
+ celix_framework_event_t* e = &fw->dispatcher.eventQueue[index];
+ if (e->bndEntry != NULL && e->bndEntry->bndId == bndId) {
+ eventInProgress = true;
+ break;
+ }
+ }
+ for (int i = 0; !eventInProgress && i < celix_arrayList_size(fw->dispatcher.dynamicEventQueue); ++i) {
+ celix_framework_event_t* e = celix_arrayList_get(fw->dispatcher.dynamicEventQueue, i);
+ if (e->bndEntry != NULL && e->bndEntry->bndId == bndId) {
+ eventInProgress = true;
+ break;
+ }
+ }
+ if (eventInProgress) {
+ celixThreadCondition_timedwaitRelative(&fw->dispatcher.cond, &fw->dispatcher.mutex, 5, 0);
+ }
+ }
+ celixThreadMutex_unlock(&fw->dispatcher.mutex);
+}
+
+
void celix_framework_setLogCallback(celix_framework_t* fw, void* logHandle, void (*logFunction)(void* handle, celix_log_level_e level, const char* file, const char *function, int line, const char *format, va_list formatArgs)) {
celix_frameworkLogger_setLogCallback(fw->logger, logHandle, logFunction);
+}
+
+
+long celix_framework_fireGenericEvent(framework_t* fw, long eventId, long bndId, const char *eventName, void* processData, void (*processCallback)(void *data), void* doneData, void (*doneCallback)(void* doneData)) {
+ celix_framework_bundle_entry_t* bndEntry = NULL;
+ if (bndId >=0) {
+ bndEntry = fw_bundleEntry_getBundleEntryAndIncreaseUseCount(fw, bndId);
+ if (bndEntry == NULL) {
+ fw_log(fw->logger, CELIX_LOG_LEVEL_ERROR, "Cannot find bundle for id %li", bndId);
+ return -1L;
+ }
+ }
+
+ if (eventId < 0) {
+ eventId = celix_framework_nextEventId(fw);
+ }
+
+ celix_framework_event_t event;
+ memset(&event, 0, sizeof(event));
+ event.type = CELIX_GENERIC_EVENT;
+ event.bndEntry = bndEntry;
+ event.genericEventId = eventId;
+ event.genericEventName = eventName;
+ event.genericProcessData = processData;
+ event.genericProcess = processCallback;
+ event.doneData = doneData;
+ event.doneCallback = doneCallback;
+ celix_framework_addToEventQueue(fw, &event);
+
+ return eventId;
+}
+
+long celix_framework_nextEventId(framework_t *fw) {
+ return __atomic_fetch_add(&fw->nextGenericEventId, 1, __ATOMIC_SEQ_CST);;
Review comment:
Technically, this can even be `__ATOMIC_RELAXED`, because it's a simple counter. See https://stackoverflow.com/a/48148318/1460998 for a more detailed answer.
##########
File path: libs/framework/src/framework.c
##########
@@ -2015,67 +1934,112 @@ static void fw_handleEventRequest(celix_framework_t *framework, request_t* reque
for (int i = 0; i < celix_arrayList_size(localListeners); ++i) {
fw_bundle_listener_pt listener = arrayList_get(localListeners, i);
- bundle_event_t event;
- memset(&event, 0, sizeof(event));
- event.bnd = request->bndEntry->bnd;
- event.type = request->eventType;
- fw_invokeBundleListener(framework, listener->listener, &event, listener->bundle);
+ bundle_event_t bEvent;
+ memset(&bEvent, 0, sizeof(bEvent));
+ bEvent.bnd = event->bndEntry->bnd;
+ bEvent.type = event->bundleEvent;
+ fw_invokeBundleListener(framework, listener->listener, &bEvent, listener->bundle);
fw_bundleListener_decreaseUseCount(listener);
}
celix_arrayList_destroy(localListeners);
- } else if (request->type == FRAMEWORK_EVENT_TYPE) {
+ } else if (event->type == CELIX_FRAMEWORK_EVENT_TYPE) {
+ //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Handling fw event");
celixThreadMutex_lock(&framework->frameworkListenersLock);
- //TODO refactor use of framework listeners to use a useCount + conditition.
for (int i = 0; i < celix_arrayList_size(framework->frameworkListeners); ++i) {
fw_framework_listener_pt listener = celix_arrayList_get(framework->frameworkListeners, i);
- framework_event_t event;
- memset(&event, 0, sizeof(event));
- event.type = request->eventType;
- event.error = request->error;
- event.errorCode = request->errorCode;
+ framework_event_t fEvent;
+ memset(&fEvent, 0, sizeof(fEvent));
+ fEvent.type = event->fwEvent;
+ fEvent.error = event->error;
+ fEvent.errorCode = event->errorCode;
- fw_invokeFrameworkListener(framework, listener->listener, &event, listener->bundle);
+ fw_invokeFrameworkListener(framework, listener->listener, &fEvent, listener->bundle);
}
celixThreadMutex_unlock(&framework->frameworkListenersLock);
+ } else if (event->type == CELIX_REGISTER_SERVICE_EVENT) {
+ //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Handling fw service registration event for service '%s' with svc id %li", event->serviceName, event->registerServiceId);
+ service_registration_t* reg = NULL;
+ celix_status_t status;
+ if (event->factory != NULL) {
+ status = celix_serviceRegistry_registerServiceFactory(framework->registry, event->bndEntry->bnd, event->serviceName, event->factory, event->properties, event->registerServiceId, ®);
+ } else {
+ status = celix_serviceRegistry_registerService(framework->registry, event->bndEntry->bnd, event->serviceName, event->svc, event->properties, event->registerServiceId, ®);
+ }
+ if (status != CELIX_SUCCESS) {
+ fw_log(framework->logger, CELIX_LOG_LEVEL_ERROR, "Could not register service async. svc name is %s, error is %s", event->serviceName, celix_strerror(status));
+ } else if (event->registerCallback != NULL) {
+ event->registerCallback(event->registerData, serviceRegistration_getServiceId(reg));
+ }
+ } else if (event->type == CELIX_UNREGISTER_SERVICE_EVENT) {
+ //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Handling fw service unregister event for service id %li", event->unregisterServiceId);
+ celix_serviceRegistry_unregisterService(framework->registry, event->bndEntry->bnd, event->unregisterServiceId);
+ } else if (event->type == CELIX_GENERIC_EVENT) {
+ //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Handling event %s", event->eventName);
+ if (event->genericProcess != NULL) {
+ event->genericProcess(event->genericProcessData);
+ }
+ }
+
+ if (event->doneCallback != NULL) {
+ event->doneCallback(event->doneData);
}
}
-static inline void fw_handleEvents(celix_framework_t* framework, celix_array_list_t* localRequests) {
+static inline celix_framework_event_t* fw_topEventFromQueue(celix_framework_t* fw) {
+ celix_framework_event_t* e = NULL;
+ celixThreadMutex_lock(&fw->dispatcher.mutex);
+ if (fw->dispatcher.eventQueueSize > 0) {
+ e = &fw->dispatcher.eventQueue[fw->dispatcher.eventQueueFirstEntry];
+ } else if (celix_arrayList_size(fw->dispatcher.dynamicEventQueue) > 0) {
+ e = celix_arrayList_get(fw->dispatcher.dynamicEventQueue, 0);
+ }
+ celixThreadMutex_unlock(&fw->dispatcher.mutex);
+ return e;
+}
+
+static inline bool fw_removeTopEventFromQueue(celix_framework_t* fw) {
+ bool dynamicallyAllocated = false;
+ celixThreadMutex_lock(&fw->dispatcher.mutex);
+ if (fw->dispatcher.eventQueueSize > 0) {
+ fw->dispatcher.eventQueueFirstEntry = (fw->dispatcher.eventQueueFirstEntry+1) % CELIX_FRAMEWORK_STATIC_EVENT_QUEUE_SIZE;
+ fw->dispatcher.eventQueueSize -= 1;
Review comment:
Hmm, I think a dynamically sized ring buffer solves this quirk of having two separate internal event queues.
Basically, have one `void *data`, one `uint64_t size`, one `uint64_t beginIndex` and one `uint64_t endIndex`. The range between begin/end signify the relevant events. If `beginIndex == endIndex` the ringbuffer is empty and when inserting, `endIndex < beginIndex && endIndex + 1 == beginIndex || endIndex + 1 == size && beginIndex == 0` signifies it's full. In the latter case, allocate a new `void*` with twice the size and memcpy everything between begin/end. Insert a new event is also simply a memcpy.
##########
File path: libs/framework/include/service_registry.h
##########
@@ -95,6 +93,17 @@ celix_status_t celix_serviceRegistry_addServiceListener(celix_service_registry_t
celix_status_t celix_serviceRegistry_removeServiceListener(celix_service_registry_t *reg, celix_service_listener_t *listener);
+bool celix_serviceRegistry_isServiceRegistered(celix_service_registry_t* reg, long serviceId);
Review comment:
Missing doxygen-style documentation here. One of the questions I have, for example, is what does `reserveId` do?
##########
File path: libs/framework/src/bundle_context.c
##########
@@ -80,15 +86,19 @@ celix_status_t bundleContext_destroy(bundle_context_pt context) {
celixThreadMutex_lock(&context->mutex);
- bundleContext_cleanupBundleTrackers(context);
- bundleContext_cleanupServiceTrackers(context);
- bundleContext_cleanupServiceTrackerTrackers(context);
+ assert(hashMap_size(context->bundleTrackers) == 0);
+ hashMap_destroy(context->bundleTrackers, false, false);
+ assert(hashMap_size(context->serviceTrackers) == 0);
+ hashMap_destroy(context->serviceTrackers, false, false);
+ assert(hashMap_size(context->metaTrackers) == 0);
+ hashMap_destroy(context->metaTrackers, false, false);
+ assert(celix_arrayList_size(context->svcRegistrations) == 0);
+ celix_arrayList_destroy(context->svcRegistrations);
- //NOTE still present service registrations will be cleared during bundle stop in the
+ //NOTE still present service registrations will be cleared during bundle stop in the
Review comment:
But the context->svcRegistrations assert only 3 lines above says otherwise?
##########
File path: libs/framework/include/celix_bundle_context.h
##########
@@ -655,23 +863,46 @@ bool celix_bundleContext_startBundle(celix_bundle_context_t *ctx, long bndId);
*/
char* celix_bundleContext_getBundleSymbolicName(celix_bundle_context_t *ctx, long bndId);
+
/**
* track bundles
* The add bundle callback will also be called for already installed bundles.
*
+ * The bundle tracker will be created async on the Celix event loop thread. This means that the function can return
+ * before the tracker is created.
+ *
* @param ctx The bundle context.
* @param callbackHandle The data pointer, which will be used in the callbacks
* @param add The callback which will be called for started bundles.
* @param remove The callback which will be called when bundles are stopped.
* @return The bundle tracker id or < 0 if unsuccessful.
*/
-long celix_bundleContext_trackBundles(
+long celix_bundleContext_trackBundlesAsync(
celix_bundle_context_t* ctx,
void* callbackHandle,
void (*onStarted)(void* handle, const celix_bundle_t *bundle),
void (*onStopped)(void *handle, const celix_bundle_t *bundle)
);
+/**
+ * track bundles
+ * The add bundle callback will also be called for already installed bundles.
Review comment:
add bundle callback? Do you mean onStarted callback here?
##########
File path: libs/framework/src/framework.c
##########
@@ -1922,88 +1861,68 @@ bundle_context_t* framework_getContext(const_framework_pt framework) {
return result;
}
-celix_status_t fw_fireBundleEvent(framework_pt framework, bundle_event_type_e eventType, celix_framework_bundle_entry_t* entry) {
- celix_status_t status = CELIX_SUCCESS;
-
+void fw_fireBundleEvent(framework_pt framework, bundle_event_type_e eventType, celix_framework_bundle_entry_t* entry) {
if (eventType == OSGI_FRAMEWORK_BUNDLE_EVENT_STOPPING || eventType == OSGI_FRAMEWORK_BUNDLE_EVENT_UNINSTALLED || eventType == OSGI_FRAMEWORK_BUNDLE_EVENT_STOPPED) {
if (entry->bndId == framework->bundleId) {
//NOTE for framework bundle not triggering events while framework is stopped (and as result in use)
- return CELIX_SUCCESS;
+ return;
}
}
- request_t* request = (request_t*) calloc(1, sizeof(*request));
- if (!request) {
- status = CELIX_ENOMEM;
- } else {
- fw_bundleEntry_increaseUseCount(entry);
+ fw_bundleEntry_increaseUseCount(entry);
- request->eventType = eventType;
- request->filter = NULL;
- request->type = BUNDLE_EVENT_TYPE;
- request->error = NULL;
- request->bndEntry = entry;
+ celix_framework_event_t event;
+ memset(&event, 0, sizeof(event));
+ event.type = CELIX_BUNDLE_EVENT_TYPE;
+ event.bndEntry = entry;
+ event.bundleEvent = eventType;
+ celix_framework_addToEventQueue(framework, &event);
+}
- celixThreadMutex_lock(&framework->dispatcher.mutex);
- if (framework->dispatcher.active) {
- //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Adding dispatcher bundle event request for bnd id %li with event type %i", entry->bndId, eventType);
- celix_arrayList_add(framework->dispatcher.requests, request);
- celixThreadCondition_broadcast(&framework->dispatcher.cond);
- } else {
- /*
- * NOTE because stopping the framework is done through stopping the framework bundle,
- * most bundle stopping / stopped events cannot be fired.
- *
- * TBD if this needs to addressed.
- */
- fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Cannot fire event dispatcher not active. Event is %x for bundle %s", eventType, celix_bundle_getSymbolicName(entry->bnd));
- fw_bundleEntry_decreaseUseCount(entry);
- free(request);
- }
- celixThreadMutex_unlock(&framework->dispatcher.mutex);
+void fw_fireFrameworkEvent(framework_pt framework, framework_event_type_e eventType, celix_status_t errorCode) {
+ celix_framework_event_t event;
+ memset(&event, 0, sizeof(event));
+ event.type = CELIX_FRAMEWORK_EVENT_TYPE;
+ event.fwEvent = eventType;
+ event.errorCode = errorCode;
+ event.error = "";
+ if (errorCode != CELIX_SUCCESS) {
+ event.error = celix_strerror(errorCode);
}
- framework_logIfError(framework->logger, status, NULL, "Failed to fire bundle event");
-
- return status;
+ celix_framework_addToEventQueue(framework, &event);
}
-celix_status_t fw_fireFrameworkEvent(framework_pt framework, framework_event_type_e eventType, celix_status_t errorCode) {
- celix_status_t status = CELIX_SUCCESS;
-
- request_t* request = calloc(1, sizeof(*request));
- if (!request) {
- status = CELIX_ENOMEM;
+static void celix_framework_addToEventQueue(celix_framework_t *fw, const celix_framework_event_t* event) {
+ celixThreadMutex_lock(&fw->dispatcher.mutex);
+ //try to add to static queue
+ if (celix_arrayList_size(fw->dispatcher.dynamicEventQueue) > 0) { //always to dynamic queue if not empty (to ensure order)
+ celix_framework_event_t *e = malloc(sizeof(*e));
+ *e = *event; //shallow copy
+ celix_arrayList_add(fw->dispatcher.dynamicEventQueue, e);
+ if (celix_arrayList_size(fw->dispatcher.dynamicEventQueue) % 100 == 0) {
+ fw_log(fw->logger, CELIX_LOG_LEVEL_WARNING, "dynamic event queue size is %i. Is there a bundle blocking on the event loop thread?", celix_arrayList_size(fw->dispatcher.dynamicEventQueue));
+ }
+ } else if (fw->dispatcher.eventQueueSize < CELIX_FRAMEWORK_STATIC_EVENT_QUEUE_SIZE) {
+ size_t index = (fw->dispatcher.eventQueueFirstEntry + fw->dispatcher.eventQueueSize) %
+ CELIX_FRAMEWORK_STATIC_EVENT_QUEUE_SIZE;
+ fw->dispatcher.eventQueue[index] = *event; //shallow copy
+ fw->dispatcher.eventQueueSize += 1;
} else {
- request->eventType = eventType;
- request->filter = NULL;
- request->type = FRAMEWORK_EVENT_TYPE;
- request->errorCode = errorCode;
- request->error = "";
-
- if (errorCode != CELIX_SUCCESS) {
- request->error = celix_strerror(errorCode);
- }
-
- celixThreadMutex_lock(&framework->dispatcher.mutex);
- if (framework->dispatcher.active) {
- //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Adding dispatcher framework event request for event type %i", eventType);
- celix_arrayList_add(framework->dispatcher.requests, request);
- celixThreadCondition_broadcast(&framework->dispatcher.cond);
- } else {
- free(request);
- }
- celixThreadMutex_unlock(&framework->dispatcher.mutex);
- }
-
- framework_logIfError(framework->logger, status, NULL, "Failed to fire framework event");
-
- return status;
+ //static queue is full, dynamics queue is empty. Add first entry to dynamic queue
+ fw_log(fw->logger, CELIX_LOG_LEVEL_WARNING,
+ "Static event queue for celix framework is full, falling back to dynamic allocated events. Increase static event queue size, current size is %i", CELIX_FRAMEWORK_STATIC_EVENT_QUEUE_SIZE);
+ celix_framework_event_t *e = malloc(sizeof(*e));
+ *e = *event; //shallow copy
+ celix_arrayList_add(fw->dispatcher.dynamicEventQueue, e);
+ }
+ celixThreadCondition_broadcast(&fw->dispatcher.cond);
+ celixThreadMutex_unlock(&fw->dispatcher.mutex);
}
-
-static void fw_handleEventRequest(celix_framework_t *framework, request_t* request) {
- if (request->type == BUNDLE_EVENT_TYPE) {
+static void fw_handleEventRequest(celix_framework_t *framework, celix_framework_event_t* event) {
+ if (event->type == CELIX_BUNDLE_EVENT_TYPE) {
Review comment:
This if is best done with a switch/case: https://gcc.godbolt.org/z/oKKnEY
##########
File path: libs/framework/src/framework.c
##########
@@ -2015,67 +1934,112 @@ static void fw_handleEventRequest(celix_framework_t *framework, request_t* reque
for (int i = 0; i < celix_arrayList_size(localListeners); ++i) {
fw_bundle_listener_pt listener = arrayList_get(localListeners, i);
- bundle_event_t event;
- memset(&event, 0, sizeof(event));
- event.bnd = request->bndEntry->bnd;
- event.type = request->eventType;
- fw_invokeBundleListener(framework, listener->listener, &event, listener->bundle);
+ bundle_event_t bEvent;
+ memset(&bEvent, 0, sizeof(bEvent));
+ bEvent.bnd = event->bndEntry->bnd;
+ bEvent.type = event->bundleEvent;
+ fw_invokeBundleListener(framework, listener->listener, &bEvent, listener->bundle);
fw_bundleListener_decreaseUseCount(listener);
}
celix_arrayList_destroy(localListeners);
- } else if (request->type == FRAMEWORK_EVENT_TYPE) {
+ } else if (event->type == CELIX_FRAMEWORK_EVENT_TYPE) {
+ //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Handling fw event");
celixThreadMutex_lock(&framework->frameworkListenersLock);
- //TODO refactor use of framework listeners to use a useCount + conditition.
for (int i = 0; i < celix_arrayList_size(framework->frameworkListeners); ++i) {
fw_framework_listener_pt listener = celix_arrayList_get(framework->frameworkListeners, i);
- framework_event_t event;
- memset(&event, 0, sizeof(event));
- event.type = request->eventType;
- event.error = request->error;
- event.errorCode = request->errorCode;
+ framework_event_t fEvent;
+ memset(&fEvent, 0, sizeof(fEvent));
+ fEvent.type = event->fwEvent;
+ fEvent.error = event->error;
+ fEvent.errorCode = event->errorCode;
- fw_invokeFrameworkListener(framework, listener->listener, &event, listener->bundle);
+ fw_invokeFrameworkListener(framework, listener->listener, &fEvent, listener->bundle);
}
celixThreadMutex_unlock(&framework->frameworkListenersLock);
+ } else if (event->type == CELIX_REGISTER_SERVICE_EVENT) {
+ //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Handling fw service registration event for service '%s' with svc id %li", event->serviceName, event->registerServiceId);
+ service_registration_t* reg = NULL;
+ celix_status_t status;
+ if (event->factory != NULL) {
+ status = celix_serviceRegistry_registerServiceFactory(framework->registry, event->bndEntry->bnd, event->serviceName, event->factory, event->properties, event->registerServiceId, ®);
+ } else {
+ status = celix_serviceRegistry_registerService(framework->registry, event->bndEntry->bnd, event->serviceName, event->svc, event->properties, event->registerServiceId, ®);
+ }
+ if (status != CELIX_SUCCESS) {
+ fw_log(framework->logger, CELIX_LOG_LEVEL_ERROR, "Could not register service async. svc name is %s, error is %s", event->serviceName, celix_strerror(status));
+ } else if (event->registerCallback != NULL) {
+ event->registerCallback(event->registerData, serviceRegistration_getServiceId(reg));
+ }
+ } else if (event->type == CELIX_UNREGISTER_SERVICE_EVENT) {
+ //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Handling fw service unregister event for service id %li", event->unregisterServiceId);
+ celix_serviceRegistry_unregisterService(framework->registry, event->bndEntry->bnd, event->unregisterServiceId);
+ } else if (event->type == CELIX_GENERIC_EVENT) {
+ //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Handling event %s", event->eventName);
+ if (event->genericProcess != NULL) {
+ event->genericProcess(event->genericProcessData);
+ }
+ }
+
+ if (event->doneCallback != NULL) {
+ event->doneCallback(event->doneData);
}
}
-static inline void fw_handleEvents(celix_framework_t* framework, celix_array_list_t* localRequests) {
+static inline celix_framework_event_t* fw_topEventFromQueue(celix_framework_t* fw) {
+ celix_framework_event_t* e = NULL;
+ celixThreadMutex_lock(&fw->dispatcher.mutex);
+ if (fw->dispatcher.eventQueueSize > 0) {
+ e = &fw->dispatcher.eventQueue[fw->dispatcher.eventQueueFirstEntry];
+ } else if (celix_arrayList_size(fw->dispatcher.dynamicEventQueue) > 0) {
+ e = celix_arrayList_get(fw->dispatcher.dynamicEventQueue, 0);
+ }
+ celixThreadMutex_unlock(&fw->dispatcher.mutex);
+ return e;
+}
+
+static inline bool fw_removeTopEventFromQueue(celix_framework_t* fw) {
+ bool dynamicallyAllocated = false;
+ celixThreadMutex_lock(&fw->dispatcher.mutex);
+ if (fw->dispatcher.eventQueueSize > 0) {
+ fw->dispatcher.eventQueueFirstEntry = (fw->dispatcher.eventQueueFirstEntry+1) % CELIX_FRAMEWORK_STATIC_EVENT_QUEUE_SIZE;
+ fw->dispatcher.eventQueueSize -= 1;
Review comment:
Am I correct in saying that if the ring buffer is overflowed and dynamic events have been allocated, that the FIFO principle gets violated? As in, if an event gets inserted into the ring buffer (once at least one event has been handled) before the ring buffer is empty, the dynamically allocated events have to wait?
##########
File path: libs/framework/src/framework.c
##########
@@ -1922,88 +1861,68 @@ bundle_context_t* framework_getContext(const_framework_pt framework) {
return result;
}
-celix_status_t fw_fireBundleEvent(framework_pt framework, bundle_event_type_e eventType, celix_framework_bundle_entry_t* entry) {
- celix_status_t status = CELIX_SUCCESS;
-
+void fw_fireBundleEvent(framework_pt framework, bundle_event_type_e eventType, celix_framework_bundle_entry_t* entry) {
if (eventType == OSGI_FRAMEWORK_BUNDLE_EVENT_STOPPING || eventType == OSGI_FRAMEWORK_BUNDLE_EVENT_UNINSTALLED || eventType == OSGI_FRAMEWORK_BUNDLE_EVENT_STOPPED) {
if (entry->bndId == framework->bundleId) {
//NOTE for framework bundle not triggering events while framework is stopped (and as result in use)
- return CELIX_SUCCESS;
+ return;
}
}
- request_t* request = (request_t*) calloc(1, sizeof(*request));
- if (!request) {
- status = CELIX_ENOMEM;
- } else {
- fw_bundleEntry_increaseUseCount(entry);
+ fw_bundleEntry_increaseUseCount(entry);
- request->eventType = eventType;
- request->filter = NULL;
- request->type = BUNDLE_EVENT_TYPE;
- request->error = NULL;
- request->bndEntry = entry;
+ celix_framework_event_t event;
+ memset(&event, 0, sizeof(event));
+ event.type = CELIX_BUNDLE_EVENT_TYPE;
+ event.bndEntry = entry;
+ event.bundleEvent = eventType;
+ celix_framework_addToEventQueue(framework, &event);
+}
- celixThreadMutex_lock(&framework->dispatcher.mutex);
- if (framework->dispatcher.active) {
- //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Adding dispatcher bundle event request for bnd id %li with event type %i", entry->bndId, eventType);
- celix_arrayList_add(framework->dispatcher.requests, request);
- celixThreadCondition_broadcast(&framework->dispatcher.cond);
- } else {
- /*
- * NOTE because stopping the framework is done through stopping the framework bundle,
- * most bundle stopping / stopped events cannot be fired.
- *
- * TBD if this needs to addressed.
- */
- fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Cannot fire event dispatcher not active. Event is %x for bundle %s", eventType, celix_bundle_getSymbolicName(entry->bnd));
- fw_bundleEntry_decreaseUseCount(entry);
- free(request);
- }
- celixThreadMutex_unlock(&framework->dispatcher.mutex);
+void fw_fireFrameworkEvent(framework_pt framework, framework_event_type_e eventType, celix_status_t errorCode) {
+ celix_framework_event_t event;
+ memset(&event, 0, sizeof(event));
+ event.type = CELIX_FRAMEWORK_EVENT_TYPE;
+ event.fwEvent = eventType;
+ event.errorCode = errorCode;
+ event.error = "";
+ if (errorCode != CELIX_SUCCESS) {
+ event.error = celix_strerror(errorCode);
}
- framework_logIfError(framework->logger, status, NULL, "Failed to fire bundle event");
-
- return status;
+ celix_framework_addToEventQueue(framework, &event);
}
-celix_status_t fw_fireFrameworkEvent(framework_pt framework, framework_event_type_e eventType, celix_status_t errorCode) {
- celix_status_t status = CELIX_SUCCESS;
-
- request_t* request = calloc(1, sizeof(*request));
- if (!request) {
- status = CELIX_ENOMEM;
+static void celix_framework_addToEventQueue(celix_framework_t *fw, const celix_framework_event_t* event) {
+ celixThreadMutex_lock(&fw->dispatcher.mutex);
+ //try to add to static queue
+ if (celix_arrayList_size(fw->dispatcher.dynamicEventQueue) > 0) { //always to dynamic queue if not empty (to ensure order)
+ celix_framework_event_t *e = malloc(sizeof(*e));
+ *e = *event; //shallow copy
+ celix_arrayList_add(fw->dispatcher.dynamicEventQueue, e);
+ if (celix_arrayList_size(fw->dispatcher.dynamicEventQueue) % 100 == 0) {
+ fw_log(fw->logger, CELIX_LOG_LEVEL_WARNING, "dynamic event queue size is %i. Is there a bundle blocking on the event loop thread?", celix_arrayList_size(fw->dispatcher.dynamicEventQueue));
+ }
+ } else if (fw->dispatcher.eventQueueSize < CELIX_FRAMEWORK_STATIC_EVENT_QUEUE_SIZE) {
+ size_t index = (fw->dispatcher.eventQueueFirstEntry + fw->dispatcher.eventQueueSize) %
+ CELIX_FRAMEWORK_STATIC_EVENT_QUEUE_SIZE;
+ fw->dispatcher.eventQueue[index] = *event; //shallow copy
+ fw->dispatcher.eventQueueSize += 1;
} else {
- request->eventType = eventType;
- request->filter = NULL;
- request->type = FRAMEWORK_EVENT_TYPE;
- request->errorCode = errorCode;
- request->error = "";
-
- if (errorCode != CELIX_SUCCESS) {
- request->error = celix_strerror(errorCode);
- }
-
- celixThreadMutex_lock(&framework->dispatcher.mutex);
- if (framework->dispatcher.active) {
- //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Adding dispatcher framework event request for event type %i", eventType);
- celix_arrayList_add(framework->dispatcher.requests, request);
- celixThreadCondition_broadcast(&framework->dispatcher.cond);
- } else {
- free(request);
- }
- celixThreadMutex_unlock(&framework->dispatcher.mutex);
- }
-
- framework_logIfError(framework->logger, status, NULL, "Failed to fire framework event");
-
- return status;
+ //static queue is full, dynamics queue is empty. Add first entry to dynamic queue
+ fw_log(fw->logger, CELIX_LOG_LEVEL_WARNING,
+ "Static event queue for celix framework is full, falling back to dynamic allocated events. Increase static event queue size, current size is %i", CELIX_FRAMEWORK_STATIC_EVENT_QUEUE_SIZE);
+ celix_framework_event_t *e = malloc(sizeof(*e));
+ *e = *event; //shallow copy
+ celix_arrayList_add(fw->dispatcher.dynamicEventQueue, e);
+ }
+ celixThreadCondition_broadcast(&fw->dispatcher.cond);
+ celixThreadMutex_unlock(&fw->dispatcher.mutex);
}
-
-static void fw_handleEventRequest(celix_framework_t *framework, request_t* request) {
- if (request->type == BUNDLE_EVENT_TYPE) {
+static void fw_handleEventRequest(celix_framework_t *framework, celix_framework_event_t* event) {
+ if (event->type == CELIX_BUNDLE_EVENT_TYPE) {
+ //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Handling fw bundle event for bundle %s", event->bndEntry->bnd->symbolicName);
Review comment:
Is this statement still needed?
##########
File path: libs/framework/src/service_tracker.c
##########
@@ -128,345 +111,285 @@ celix_status_t serviceTracker_create(bundle_context_pt context, const char * ser
return status;
}
-celix_status_t serviceTracker_createWithFilter(bundle_context_pt context, const char * filter, service_tracker_customizer_pt customizer, service_tracker_pt *tracker) {
- celix_status_t status = CELIX_SUCCESS;
- *tracker = (service_tracker_pt) calloc(1, sizeof(**tracker));
- if (!*tracker) {
- status = CELIX_ENOMEM;
- } else {
- (*tracker)->context = context;
- (*tracker)->filter = strdup(filter);
- (*tracker)->customizer = customizer;
- }
+celix_status_t serviceTracker_createWithFilter(bundle_context_pt context, const char * filter, service_tracker_customizer_pt customizer, service_tracker_pt *out) {
+ service_tracker_t* tracker = calloc(1, sizeof(*tracker));
+ *out = tracker;
+ tracker->context = context;
+ tracker->filter = celix_utils_strdup(filter);
+ tracker->customizer = *customizer;
+ free(customizer);
- framework_logIfError(celix_frameworkLogger_globalLogger(), status, NULL, "Cannot create service tracker [filter=%s]", filter);
+ celixThreadMutex_create(&tracker->closeSync.mutex, NULL);
+ celixThreadCondition_init(&tracker->closeSync.cond, NULL);
- return status;
+ celixThreadMutex_create(&tracker->mutex, NULL);
+ celixThreadCondition_init(&tracker->cond, NULL);
+ tracker->trackedServices = celix_arrayList_create();
+ tracker->untrackingServices = celix_arrayList_create();
+
+ celixThreadMutex_create(&tracker->mutex, NULL);
+ tracker->currentHighestServiceId = -1;
+
+ tracker->listener.handle = tracker;
+ tracker->listener.serviceChanged = (void *) serviceTracker_serviceChanged;
+
+ tracker->callbackHandle = tracker->callbackHandle;
+
+ tracker->add = tracker->add;
+ tracker->addWithProperties = tracker->addWithProperties;
+ tracker->addWithOwner = tracker->addWithOwner;
+ tracker->set = tracker->set;
+ tracker->setWithProperties = tracker->setWithProperties;
+ tracker->setWithOwner = tracker->setWithOwner;
+ tracker->remove = tracker->remove;
+ tracker->removeWithProperties = tracker->removeWithProperties;
+ tracker->removeWithOwner = tracker->removeWithOwner;
+
+ return CELIX_SUCCESS;
}
celix_status_t serviceTracker_destroy(service_tracker_pt tracker) {
- if (tracker->customizer != NULL) {
- serviceTrackerCustomizer_destroy(tracker->customizer);
- }
-
free(tracker->serviceName);
free(tracker->filter);
- free(tracker);
-
+ celixThreadMutex_destroy(&tracker->closeSync.mutex);
+ celixThreadCondition_destroy(&tracker->closeSync.cond);
+ celixThreadMutex_destroy(&tracker->mutex);
+ celixThreadCondition_destroy(&tracker->cond);
+ celix_arrayList_destroy(tracker->trackedServices);
+ celix_arrayList_destroy(tracker->untrackingServices);
+ free(tracker);
return CELIX_SUCCESS;
}
celix_status_t serviceTracker_open(service_tracker_pt tracker) {
- celix_service_listener_t *listener = NULL;
- celix_service_tracker_instance_t *instance = NULL;
- celix_status_t status = CELIX_SUCCESS;
-
- bool addListener = false;
-
- celixThreadRwlock_writeLock(&tracker->instanceLock);
- if (tracker->instance == NULL) {
- instance = calloc(1, sizeof(*instance));
- instance->context = tracker->context;
-
- instance->closing = false;
- instance->activeServiceChangeCalls = 0;
- celixThreadMutex_create(&instance->closingLock, NULL);
- celixThreadCondition_init(&instance->activeServiceChangeCallsCond, NULL);
-
-
- celixThreadRwlock_create(&instance->lock, NULL);
- instance->trackedServices = celix_arrayList_create();
-
- celixThreadMutex_create(&instance->mutex, NULL);
- instance->currentHighestServiceId = -1;
-
- instance->listener.handle = instance;
- instance->listener.serviceChanged = (void *) serviceTracker_serviceChanged;
- listener = &instance->listener;
-
- instance->callbackHandle = tracker->callbackHandle;
- instance->filter = strdup(tracker->filter);
- if (tracker->customizer != NULL) {
- memcpy(&instance->customizer, tracker->customizer, sizeof(instance->customizer));
- }
- instance->add = tracker->add;
- instance->addWithProperties = tracker->addWithProperties;
- instance->addWithOwner = tracker->addWithOwner;
- instance->set = tracker->set;
- instance->setWithProperties = tracker->setWithProperties;
- instance->setWithOwner = tracker->setWithOwner;
- instance->remove = tracker->remove;
- instance->removeWithProperties = tracker->removeWithProperties;
- instance->removeWithOwner = tracker->removeWithOwner;
-
- tracker->instance = instance;
-
- addListener = true;
- } else {
- //already open
- framework_logIfError(tracker->context->framework->logger, status, NULL, "Tracker already open");
+ celixThreadMutex_lock(&tracker->mutex);
+ bool alreadyOpen = tracker->open;
+ tracker->open = true;
+ celixThreadMutex_unlock(&tracker->mutex);
+ if (!alreadyOpen) {
+ bundleContext_addServiceListener(tracker->context, &tracker->listener, tracker->filter);
}
- celixThreadRwlock_unlock(&tracker->instanceLock);
-
- if (addListener) {
- bundleContext_addServiceListener(tracker->context, listener, tracker->filter);
- }
return CELIX_SUCCESS;
}
-celix_status_t serviceTracker_close(service_tracker_pt tracker) {
+celix_status_t serviceTracker_close(service_tracker_t* tracker) {
//put all tracked entries in tmp array list, so that the untrack (etc) calls are not blocked.
//set state to close to prevent service listener events
- celixThreadRwlock_writeLock(&tracker->instanceLock);
- celix_service_tracker_instance_t *instance = tracker->instance;
- tracker->instance = NULL;
- if (instance != NULL) {
- celixThreadMutex_lock(&instance->closingLock);
- //prevent service listener events
- instance->closing = true;
- celixThreadMutex_unlock(&instance->closingLock);
- }
- celixThreadRwlock_unlock(&tracker->instanceLock);
-
- if (instance != NULL) {
- celixThreadRwlock_writeLock(&instance->lock);
- unsigned int size = celix_arrayList_size(instance->trackedServices);
- if(size > 0) {
- celix_tracked_entry_t *trackedEntries[size];
- for (unsigned int i = 0u; i < size; i++) {
- trackedEntries[i] = (celix_tracked_entry_t *) arrayList_get(instance->trackedServices, i);
- }
- arrayList_clear(instance->trackedServices);
- celixThreadRwlock_unlock(&instance->lock);
+ celixThreadMutex_lock(&tracker->mutex);
+ bool open = tracker->open;
+ tracker->open = false;
+ celixThreadMutex_unlock(&tracker->mutex);
- //loop trough tracked entries an untrack
- for (unsigned int i = 0u; i < size; i++) {
- serviceTracker_untrackTracked(instance, trackedEntries[i]);
+ if (!open) {
+ return CELIX_SUCCESS;
+ }
+
+
+ //indicate that the service tracking is closing and wait for the still pending service registration events.
+ celixThreadMutex_lock(&tracker->closeSync.mutex);
+ tracker->closeSync.closing = true;
+ while (tracker->closeSync.activeCalls > 0) {
+ celixThreadCondition_wait(&tracker->closeSync.cond, &tracker->closeSync.mutex);
+ }
+ celixThreadMutex_unlock(&tracker->closeSync.mutex);
+
+ int nrOfTrackedEntries;
+ do {
+ celixThreadMutex_lock(&tracker->mutex);
+ celix_tracked_entry_t* tracked = NULL;
+ nrOfTrackedEntries = celix_arrayList_size(tracker->trackedServices);
+ if (nrOfTrackedEntries > 0) {
+ tracked = celix_arrayList_get(tracker->trackedServices, 0);
+ celix_arrayList_removeAt(tracker->trackedServices, 0);
+ celix_arrayList_add(tracker->untrackingServices, tracked);
+ }
+ celixThreadMutex_unlock(&tracker->mutex);
+
+ if (tracked != NULL) {
+ int currentSize = nrOfTrackedEntries - 1;
+ if (currentSize == 0) {
+ serviceTracker_checkAndInvokeSetService(tracker, NULL, NULL, NULL);
+ } else {
+ celix_serviceTracker_useHighestRankingService(tracker, tracked->serviceName, tracker, NULL, NULL, serviceTracker_checkAndInvokeSetService);
}
- } else {
- celixThreadRwlock_unlock(&instance->lock);
- }
- celixThreadMutex_lock(&instance->closingLock);
- while (instance->activeServiceChangeCalls > 0) {
- celixThreadCondition_wait(&instance->activeServiceChangeCallsCond, &instance->closingLock);
+ serviceTracker_untrackTracked(tracker, tracked);
+ celixThreadMutex_lock(&tracker->mutex);
+ celix_arrayList_remove(tracker->untrackingServices, tracked);
+ celixThreadCondition_broadcast(&tracker->cond);
+ celixThreadMutex_unlock(&tracker->mutex);
}
- celixThreadMutex_unlock(&instance->closingLock);
-#ifdef CELIX_SERVICE_TRACKER_USE_SHUTDOWN_THREAD
- //NOTE Separate thread is needed to prevent deadlock where closing is triggered from a serviceChange event and the
- // untrack -> removeServiceListener will try to remove a service listener which is being invoked and is the
- // actual thread calling the removeServiceListener.
- //
- // This can be detached -> because service listener events are ignored (closing=true) and so no callbacks
- // are made back to the celix framework / tracker owner.
- serviceTracker_addInstanceFromShutdownList(instance);
- celix_thread_t localThread;
- celixThread_create(&localThread, NULL, shutdownServiceTrackerInstanceHandler, instance);
- celixThread_detach(localThread);
-#else
- fw_removeServiceListener(instance->context->framework, instance->context->bundle, &instance->listener);
-
- celixThreadMutex_destroy(&instance->closingLock);
- celixThreadCondition_destroy(&instance->activeServiceChangeCallsCond);
- celixThreadMutex_destroy(&instance->mutex);
- celixThreadRwlock_destroy(&instance->lock);
- celix_arrayList_destroy(instance->trackedServices);
- free(instance->filter);
- free(instance);
-#endif
- }
+ celixThreadMutex_lock(&tracker->mutex);
+ nrOfTrackedEntries = celix_arrayList_size(tracker->trackedServices);
+ celixThreadMutex_unlock(&tracker->mutex);
+ } while (nrOfTrackedEntries > 0);
+
+
+ fw_removeServiceListener(tracker->context->framework, tracker->context->bundle, &tracker->listener);
return CELIX_SUCCESS;
}
-service_reference_pt serviceTracker_getServiceReference(service_tracker_pt tracker) {
+service_reference_pt serviceTracker_getServiceReference(service_tracker_t* tracker) {
//TODO deprecated warning -> not locked
- celix_tracked_entry_t *tracked;
+
service_reference_pt result = NULL;
- unsigned int i;
-
- celixThreadRwlock_readLock(&tracker->instanceLock);
- celix_service_tracker_instance_t *instance = tracker->instance;
- if (instance != NULL) {
- celixThreadRwlock_readLock(&instance->lock);
- for (i = 0; i < arrayList_size(instance->trackedServices); ++i) {
- tracked = (celix_tracked_entry_t *) arrayList_get(instance->trackedServices, i);
- result = tracked->reference;
- break;
- }
- celixThreadRwlock_unlock(&instance->lock);
+
+ celixThreadMutex_lock(&tracker->mutex);
+ for (int i = 0; i < celix_arrayList_size(tracker->trackedServices); ++i) {
+ celix_tracked_entry_t *tracked = celix_arrayList_get(tracker->trackedServices, i);
+ result = tracked->reference;
+ break;
}
- celixThreadRwlock_unlock(&tracker->instanceLock);
+ celixThreadMutex_unlock(&tracker->mutex);
return result;
}
-array_list_pt serviceTracker_getServiceReferences(service_tracker_pt tracker) {
+array_list_pt serviceTracker_getServiceReferences(service_tracker_t* tracker) {
//TODO deprecated warning -> not locked
Review comment:
Is this comment still relevant?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] Oipo commented on pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
Oipo commented on pull request #286:
URL: https://github.com/apache/celix/pull/286#issuecomment-712099123
> > ThreadSanitizer still reports a couple of issues. See attachments.
> > Moreover, one of the tests in test_framework takes about 20 seconds to stop:
> > ```
> > [2020-10-12T11:11:19] [ trace] [celix_framework] [framework_stop:582] Celix framework stopped for uuid 379123f6-387f-4e2a-a5f3-f6c7264c64b1
> > [2020-10-12T11:11:25] [warning] [celix_framework] [fw_bundleEntry_waitTillUseCountIs:79] Bundle Framework (0) still in use. Use count is 800, desired is 1
> > [2020-10-12T11:11:48] [ trace] [celix_framework] [framework_shutdown:1833] Joined event loop thread for framework 379123f6-387f-4e2a-a5f3-f6c7264c64b1
> > ```
> >
> >
> > [potentialdeadlock.txt](https://github.com/apache/celix/files/5364118/potentialdeadlock.txt)
> > [threadsleaked.txt](https://github.com/apache/celix/files/5364119/threadsleaked.txt)
>
> How can I reproduce the thread sanitizer outputs? Just setting the ENABLE_THREAD_SANITIZER CMake option?
>
> I cannot reproduce the test_framework test which takes 20 seconds. It this a structural issue on your system?
This happens 90% of the time when using `ENABLE_THREAD_SANITIZER`, yes. I can run it again with your latest changes and see if anything different happens.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes commented on a change in pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
pnoltes commented on a change in pull request #286:
URL: https://github.com/apache/celix/pull/286#discussion_r507166662
##########
File path: libs/framework/src/service_tracker.c
##########
@@ -128,345 +111,285 @@ celix_status_t serviceTracker_create(bundle_context_pt context, const char * ser
return status;
}
-celix_status_t serviceTracker_createWithFilter(bundle_context_pt context, const char * filter, service_tracker_customizer_pt customizer, service_tracker_pt *tracker) {
- celix_status_t status = CELIX_SUCCESS;
- *tracker = (service_tracker_pt) calloc(1, sizeof(**tracker));
- if (!*tracker) {
- status = CELIX_ENOMEM;
- } else {
- (*tracker)->context = context;
- (*tracker)->filter = strdup(filter);
- (*tracker)->customizer = customizer;
- }
+celix_status_t serviceTracker_createWithFilter(bundle_context_pt context, const char * filter, service_tracker_customizer_pt customizer, service_tracker_pt *out) {
+ service_tracker_t* tracker = calloc(1, sizeof(*tracker));
+ *out = tracker;
+ tracker->context = context;
+ tracker->filter = celix_utils_strdup(filter);
+ tracker->customizer = *customizer;
+ free(customizer);
- framework_logIfError(celix_frameworkLogger_globalLogger(), status, NULL, "Cannot create service tracker [filter=%s]", filter);
+ celixThreadMutex_create(&tracker->closeSync.mutex, NULL);
+ celixThreadCondition_init(&tracker->closeSync.cond, NULL);
- return status;
+ celixThreadMutex_create(&tracker->mutex, NULL);
+ celixThreadCondition_init(&tracker->cond, NULL);
+ tracker->trackedServices = celix_arrayList_create();
+ tracker->untrackingServices = celix_arrayList_create();
+
+ celixThreadMutex_create(&tracker->mutex, NULL);
+ tracker->currentHighestServiceId = -1;
+
+ tracker->listener.handle = tracker;
+ tracker->listener.serviceChanged = (void *) serviceTracker_serviceChanged;
+
+ tracker->callbackHandle = tracker->callbackHandle;
+
+ tracker->add = tracker->add;
+ tracker->addWithProperties = tracker->addWithProperties;
+ tracker->addWithOwner = tracker->addWithOwner;
+ tracker->set = tracker->set;
+ tracker->setWithProperties = tracker->setWithProperties;
+ tracker->setWithOwner = tracker->setWithOwner;
+ tracker->remove = tracker->remove;
+ tracker->removeWithProperties = tracker->removeWithProperties;
+ tracker->removeWithOwner = tracker->removeWithOwner;
+
+ return CELIX_SUCCESS;
}
celix_status_t serviceTracker_destroy(service_tracker_pt tracker) {
- if (tracker->customizer != NULL) {
- serviceTrackerCustomizer_destroy(tracker->customizer);
- }
-
free(tracker->serviceName);
free(tracker->filter);
- free(tracker);
-
+ celixThreadMutex_destroy(&tracker->closeSync.mutex);
+ celixThreadCondition_destroy(&tracker->closeSync.cond);
+ celixThreadMutex_destroy(&tracker->mutex);
+ celixThreadCondition_destroy(&tracker->cond);
+ celix_arrayList_destroy(tracker->trackedServices);
+ celix_arrayList_destroy(tracker->untrackingServices);
+ free(tracker);
return CELIX_SUCCESS;
}
celix_status_t serviceTracker_open(service_tracker_pt tracker) {
- celix_service_listener_t *listener = NULL;
- celix_service_tracker_instance_t *instance = NULL;
- celix_status_t status = CELIX_SUCCESS;
-
- bool addListener = false;
-
- celixThreadRwlock_writeLock(&tracker->instanceLock);
- if (tracker->instance == NULL) {
- instance = calloc(1, sizeof(*instance));
- instance->context = tracker->context;
-
- instance->closing = false;
- instance->activeServiceChangeCalls = 0;
- celixThreadMutex_create(&instance->closingLock, NULL);
- celixThreadCondition_init(&instance->activeServiceChangeCallsCond, NULL);
-
-
- celixThreadRwlock_create(&instance->lock, NULL);
- instance->trackedServices = celix_arrayList_create();
-
- celixThreadMutex_create(&instance->mutex, NULL);
- instance->currentHighestServiceId = -1;
-
- instance->listener.handle = instance;
- instance->listener.serviceChanged = (void *) serviceTracker_serviceChanged;
- listener = &instance->listener;
-
- instance->callbackHandle = tracker->callbackHandle;
- instance->filter = strdup(tracker->filter);
- if (tracker->customizer != NULL) {
- memcpy(&instance->customizer, tracker->customizer, sizeof(instance->customizer));
- }
- instance->add = tracker->add;
- instance->addWithProperties = tracker->addWithProperties;
- instance->addWithOwner = tracker->addWithOwner;
- instance->set = tracker->set;
- instance->setWithProperties = tracker->setWithProperties;
- instance->setWithOwner = tracker->setWithOwner;
- instance->remove = tracker->remove;
- instance->removeWithProperties = tracker->removeWithProperties;
- instance->removeWithOwner = tracker->removeWithOwner;
-
- tracker->instance = instance;
-
- addListener = true;
- } else {
- //already open
- framework_logIfError(tracker->context->framework->logger, status, NULL, "Tracker already open");
+ celixThreadMutex_lock(&tracker->mutex);
+ bool alreadyOpen = tracker->open;
+ tracker->open = true;
+ celixThreadMutex_unlock(&tracker->mutex);
+ if (!alreadyOpen) {
+ bundleContext_addServiceListener(tracker->context, &tracker->listener, tracker->filter);
}
- celixThreadRwlock_unlock(&tracker->instanceLock);
-
- if (addListener) {
- bundleContext_addServiceListener(tracker->context, listener, tracker->filter);
- }
return CELIX_SUCCESS;
}
-celix_status_t serviceTracker_close(service_tracker_pt tracker) {
+celix_status_t serviceTracker_close(service_tracker_t* tracker) {
//put all tracked entries in tmp array list, so that the untrack (etc) calls are not blocked.
//set state to close to prevent service listener events
- celixThreadRwlock_writeLock(&tracker->instanceLock);
- celix_service_tracker_instance_t *instance = tracker->instance;
- tracker->instance = NULL;
- if (instance != NULL) {
- celixThreadMutex_lock(&instance->closingLock);
- //prevent service listener events
- instance->closing = true;
- celixThreadMutex_unlock(&instance->closingLock);
- }
- celixThreadRwlock_unlock(&tracker->instanceLock);
-
- if (instance != NULL) {
- celixThreadRwlock_writeLock(&instance->lock);
- unsigned int size = celix_arrayList_size(instance->trackedServices);
- if(size > 0) {
- celix_tracked_entry_t *trackedEntries[size];
- for (unsigned int i = 0u; i < size; i++) {
- trackedEntries[i] = (celix_tracked_entry_t *) arrayList_get(instance->trackedServices, i);
- }
- arrayList_clear(instance->trackedServices);
- celixThreadRwlock_unlock(&instance->lock);
+ celixThreadMutex_lock(&tracker->mutex);
+ bool open = tracker->open;
+ tracker->open = false;
+ celixThreadMutex_unlock(&tracker->mutex);
- //loop trough tracked entries an untrack
- for (unsigned int i = 0u; i < size; i++) {
- serviceTracker_untrackTracked(instance, trackedEntries[i]);
+ if (!open) {
+ return CELIX_SUCCESS;
+ }
+
+
+ //indicate that the service tracking is closing and wait for the still pending service registration events.
+ celixThreadMutex_lock(&tracker->closeSync.mutex);
+ tracker->closeSync.closing = true;
+ while (tracker->closeSync.activeCalls > 0) {
+ celixThreadCondition_wait(&tracker->closeSync.cond, &tracker->closeSync.mutex);
+ }
+ celixThreadMutex_unlock(&tracker->closeSync.mutex);
+
+ int nrOfTrackedEntries;
+ do {
+ celixThreadMutex_lock(&tracker->mutex);
+ celix_tracked_entry_t* tracked = NULL;
+ nrOfTrackedEntries = celix_arrayList_size(tracker->trackedServices);
+ if (nrOfTrackedEntries > 0) {
+ tracked = celix_arrayList_get(tracker->trackedServices, 0);
+ celix_arrayList_removeAt(tracker->trackedServices, 0);
+ celix_arrayList_add(tracker->untrackingServices, tracked);
+ }
+ celixThreadMutex_unlock(&tracker->mutex);
+
+ if (tracked != NULL) {
+ int currentSize = nrOfTrackedEntries - 1;
+ if (currentSize == 0) {
+ serviceTracker_checkAndInvokeSetService(tracker, NULL, NULL, NULL);
+ } else {
+ celix_serviceTracker_useHighestRankingService(tracker, tracked->serviceName, tracker, NULL, NULL, serviceTracker_checkAndInvokeSetService);
}
- } else {
- celixThreadRwlock_unlock(&instance->lock);
- }
- celixThreadMutex_lock(&instance->closingLock);
- while (instance->activeServiceChangeCalls > 0) {
- celixThreadCondition_wait(&instance->activeServiceChangeCallsCond, &instance->closingLock);
+ serviceTracker_untrackTracked(tracker, tracked);
+ celixThreadMutex_lock(&tracker->mutex);
+ celix_arrayList_remove(tracker->untrackingServices, tracked);
+ celixThreadCondition_broadcast(&tracker->cond);
+ celixThreadMutex_unlock(&tracker->mutex);
}
- celixThreadMutex_unlock(&instance->closingLock);
-#ifdef CELIX_SERVICE_TRACKER_USE_SHUTDOWN_THREAD
- //NOTE Separate thread is needed to prevent deadlock where closing is triggered from a serviceChange event and the
- // untrack -> removeServiceListener will try to remove a service listener which is being invoked and is the
- // actual thread calling the removeServiceListener.
- //
- // This can be detached -> because service listener events are ignored (closing=true) and so no callbacks
- // are made back to the celix framework / tracker owner.
- serviceTracker_addInstanceFromShutdownList(instance);
- celix_thread_t localThread;
- celixThread_create(&localThread, NULL, shutdownServiceTrackerInstanceHandler, instance);
- celixThread_detach(localThread);
-#else
- fw_removeServiceListener(instance->context->framework, instance->context->bundle, &instance->listener);
-
- celixThreadMutex_destroy(&instance->closingLock);
- celixThreadCondition_destroy(&instance->activeServiceChangeCallsCond);
- celixThreadMutex_destroy(&instance->mutex);
- celixThreadRwlock_destroy(&instance->lock);
- celix_arrayList_destroy(instance->trackedServices);
- free(instance->filter);
- free(instance);
-#endif
- }
+ celixThreadMutex_lock(&tracker->mutex);
+ nrOfTrackedEntries = celix_arrayList_size(tracker->trackedServices);
+ celixThreadMutex_unlock(&tracker->mutex);
+ } while (nrOfTrackedEntries > 0);
+
+
+ fw_removeServiceListener(tracker->context->framework, tracker->context->bundle, &tracker->listener);
return CELIX_SUCCESS;
}
-service_reference_pt serviceTracker_getServiceReference(service_tracker_pt tracker) {
+service_reference_pt serviceTracker_getServiceReference(service_tracker_t* tracker) {
//TODO deprecated warning -> not locked
- celix_tracked_entry_t *tracked;
+
service_reference_pt result = NULL;
- unsigned int i;
-
- celixThreadRwlock_readLock(&tracker->instanceLock);
- celix_service_tracker_instance_t *instance = tracker->instance;
- if (instance != NULL) {
- celixThreadRwlock_readLock(&instance->lock);
- for (i = 0; i < arrayList_size(instance->trackedServices); ++i) {
- tracked = (celix_tracked_entry_t *) arrayList_get(instance->trackedServices, i);
- result = tracked->reference;
- break;
- }
- celixThreadRwlock_unlock(&instance->lock);
+
+ celixThreadMutex_lock(&tracker->mutex);
+ for (int i = 0; i < celix_arrayList_size(tracker->trackedServices); ++i) {
+ celix_tracked_entry_t *tracked = celix_arrayList_get(tracker->trackedServices, i);
+ result = tracked->reference;
+ break;
}
- celixThreadRwlock_unlock(&tracker->instanceLock);
+ celixThreadMutex_unlock(&tracker->mutex);
return result;
}
-array_list_pt serviceTracker_getServiceReferences(service_tracker_pt tracker) {
+array_list_pt serviceTracker_getServiceReferences(service_tracker_t* tracker) {
//TODO deprecated warning -> not locked
Review comment:
yes most of the non celix_ prefix are deprecated and ideally these function get a deprecated attribute
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes edited a comment on pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
pnoltes edited a comment on pull request #286:
URL: https://github.com/apache/celix/pull/286#issuecomment-711172582
> ThreadSanitizer still reports a couple of issues. See attachments.
>
> Moreover, one of the tests in test_framework takes about 20 seconds to stop:
>
> ```
> [2020-10-12T11:11:19] [ trace] [celix_framework] [framework_stop:582] Celix framework stopped for uuid 379123f6-387f-4e2a-a5f3-f6c7264c64b1
> [2020-10-12T11:11:25] [warning] [celix_framework] [fw_bundleEntry_waitTillUseCountIs:79] Bundle Framework (0) still in use. Use count is 800, desired is 1
> [2020-10-12T11:11:48] [ trace] [celix_framework] [framework_shutdown:1833] Joined event loop thread for framework 379123f6-387f-4e2a-a5f3-f6c7264c64b1
> ```
>
> [potentialdeadlock.txt](https://github.com/apache/celix/files/5364118/potentialdeadlock.txt)
> [threadsleaked.txt](https://github.com/apache/celix/files/5364119/threadsleaked.txt)
How can I reproduce the thread sanitizer outputs? Just setting the ENABLE_THREAD_SANITIZER CMake option?
I cannot reproduce the test_framework test which takes 20 seconds. It this a structural issue on your system?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] codecov-io edited a comment on pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #286:
URL: https://github.com/apache/celix/pull/286#issuecomment-711176467
# [Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=h1) Report
> Merging [#286](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=desc) (3776a46) into [master](https://codecov.io/gh/apache/celix/commit/7a26aea42fd33849e54a33d69969e42d096b9dcd?el=desc) (7a26aea) will **increase** coverage by `0.56%`.
> The diff coverage is `81.17%`.
[](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #286 +/- ##
==========================================
+ Coverage 66.69% 67.26% +0.56%
==========================================
Files 147 147
Lines 29947 30411 +464
==========================================
+ Hits 19974 20455 +481
+ Misses 9973 9956 -17
```
| [Impacted Files](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [libs/framework/src/celix\_framework\_factory.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2NlbGl4X2ZyYW1ld29ya19mYWN0b3J5LmM=) | `87.50% <ø> (-0.74%)` | :arrow_down: |
| [bundles/pubsub/pubsub\_utils/src/pubsub\_matching.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3V0aWxzL3NyYy9wdWJzdWJfbWF0Y2hpbmcuYw==) | `78.62% <50.00%> (+3.29%)` | :arrow_up: |
| [bundles/http\_admin/http\_admin/src/http\_admin.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9odHRwX2FkbWluL2h0dHBfYWRtaW4vc3JjL2h0dHBfYWRtaW4uYw==) | `68.04% <66.66%> (+2.63%)` | :arrow_up: |
| [libs/framework/src/service\_tracker.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfdHJhY2tlci5j) | `79.50% <76.47%> (+2.36%)` | :arrow_up: |
| [libs/framework/src/bundle.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2J1bmRsZS5j) | `68.31% <76.92%> (+0.08%)` | :arrow_up: |
| [libs/framework/src/bundle\_context.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2J1bmRsZV9jb250ZXh0LmM=) | `69.93% <78.48%> (-0.19%)` | :arrow_down: |
| [libs/framework/src/framework.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2ZyYW1ld29yay5j) | `74.66% <83.54%> (-0.66%)` | :arrow_down: |
| [libs/framework/src/service\_registry.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfcmVnaXN0cnkuYw==) | `73.35% <86.90%> (+1.66%)` | :arrow_up: |
| [...dles/pubsub/pubsub\_spi/src/pubsub\_endpoint\_match.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3NwaS9zcmMvcHVic3ViX2VuZHBvaW50X21hdGNoLmM=) | `56.37% <87.50%> (-2.23%)` | :arrow_down: |
| [libs/utils/src/utils.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy91dGlscy9zcmMvdXRpbHMuYw==) | `87.39% <90.90%> (+4.66%)` | :arrow_up: |
| ... and [27 more](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=footer). Last update [7a26aea...3776a46](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes merged pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
pnoltes merged pull request #286:
URL: https://github.com/apache/celix/pull/286
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] codecov-io edited a comment on pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #286:
URL: https://github.com/apache/celix/pull/286#issuecomment-711176467
# [Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=h1) Report
> Merging [#286](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=desc) (ba24349) into [master](https://codecov.io/gh/apache/celix/commit/dd4dca1d52c36a794dfac71867f0abc3fd81241c?el=desc) (dd4dca1) will **increase** coverage by `0.48%`.
> The diff coverage is `82.43%`.
[](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #286 +/- ##
==========================================
+ Coverage 68.17% 68.65% +0.48%
==========================================
Files 148 148
Lines 30088 30556 +468
==========================================
+ Hits 20511 20978 +467
- Misses 9577 9578 +1
```
| [Impacted Files](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [libs/framework/src/celix\_framework\_factory.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2NlbGl4X2ZyYW1ld29ya19mYWN0b3J5LmM=) | `87.50% <ø> (-0.74%)` | :arrow_down: |
| [bundles/pubsub/pubsub\_utils/src/pubsub\_matching.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3V0aWxzL3NyYy9wdWJzdWJfbWF0Y2hpbmcuYw==) | `78.62% <50.00%> (+3.29%)` | :arrow_up: |
| [bundles/http\_admin/http\_admin/src/http\_admin.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9odHRwX2FkbWluL2h0dHBfYWRtaW4vc3JjL2h0dHBfYWRtaW4uYw==) | `68.04% <66.66%> (+2.63%)` | :arrow_up: |
| [libs/framework/src/bundle.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2J1bmRsZS5j) | `68.31% <76.92%> (+0.08%)` | :arrow_up: |
| [libs/framework/src/service\_tracker.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfdHJhY2tlci5j) | `81.98% <78.82%> (+1.12%)` | :arrow_up: |
| [libs/framework/src/bundle\_context.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2J1bmRsZV9jb250ZXh0LmM=) | `70.32% <79.22%> (+0.20%)` | :arrow_up: |
| [libs/framework/src/framework.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2ZyYW1ld29yay5j) | `74.66% <83.54%> (-0.88%)` | :arrow_down: |
| [...dles/pubsub/pubsub\_spi/src/pubsub\_endpoint\_match.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3NwaS9zcmMvcHVic3ViX2VuZHBvaW50X21hdGNoLmM=) | `56.37% <87.50%> (-2.23%)` | :arrow_down: |
| [libs/framework/src/service\_registry.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfcmVnaXN0cnkuYw==) | `73.87% <89.28%> (+2.17%)` | :arrow_up: |
| [libs/utils/src/utils.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy91dGlscy9zcmMvdXRpbHMuYw==) | `87.39% <90.90%> (+4.66%)` | :arrow_up: |
| ... and [28 more](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=footer). Last update [dd4dca1...ba24349](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes commented on pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
pnoltes commented on pull request #286:
URL: https://github.com/apache/celix/pull/286#issuecomment-711172582
> ThreadSanitizer still reports a couple of issues. See attachments.
>
> Moreover, one of the tests in test_framework takes about 20 seconds to stop:
>
> ```
> [2020-10-12T11:11:19] [ trace] [celix_framework] [framework_stop:582] Celix framework stopped for uuid 379123f6-387f-4e2a-a5f3-f6c7264c64b1
> [2020-10-12T11:11:25] [warning] [celix_framework] [fw_bundleEntry_waitTillUseCountIs:79] Bundle Framework (0) still in use. Use count is 800, desired is 1
> [2020-10-12T11:11:48] [ trace] [celix_framework] [framework_shutdown:1833] Joined event loop thread for framework 379123f6-387f-4e2a-a5f3-f6c7264c64b1
> ```
>
> [potentialdeadlock.txt](https://github.com/apache/celix/files/5364118/potentialdeadlock.txt)
> [threadsleaked.txt](https://github.com/apache/celix/files/5364119/threadsleaked.txt)
How can I reproduce these outputs? Just setting the ENABLE_THREAD_SANITIZER CMake option?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] Oipo commented on a change in pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
Oipo commented on a change in pull request #286:
URL: https://github.com/apache/celix/pull/286#discussion_r511644501
##########
File path: libs/framework/src/framework.c
##########
@@ -2015,67 +1934,112 @@ static void fw_handleEventRequest(celix_framework_t *framework, request_t* reque
for (int i = 0; i < celix_arrayList_size(localListeners); ++i) {
fw_bundle_listener_pt listener = arrayList_get(localListeners, i);
- bundle_event_t event;
- memset(&event, 0, sizeof(event));
- event.bnd = request->bndEntry->bnd;
- event.type = request->eventType;
- fw_invokeBundleListener(framework, listener->listener, &event, listener->bundle);
+ bundle_event_t bEvent;
+ memset(&bEvent, 0, sizeof(bEvent));
+ bEvent.bnd = event->bndEntry->bnd;
+ bEvent.type = event->bundleEvent;
+ fw_invokeBundleListener(framework, listener->listener, &bEvent, listener->bundle);
fw_bundleListener_decreaseUseCount(listener);
}
celix_arrayList_destroy(localListeners);
- } else if (request->type == FRAMEWORK_EVENT_TYPE) {
+ } else if (event->type == CELIX_FRAMEWORK_EVENT_TYPE) {
+ //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Handling fw event");
celixThreadMutex_lock(&framework->frameworkListenersLock);
- //TODO refactor use of framework listeners to use a useCount + conditition.
for (int i = 0; i < celix_arrayList_size(framework->frameworkListeners); ++i) {
fw_framework_listener_pt listener = celix_arrayList_get(framework->frameworkListeners, i);
- framework_event_t event;
- memset(&event, 0, sizeof(event));
- event.type = request->eventType;
- event.error = request->error;
- event.errorCode = request->errorCode;
+ framework_event_t fEvent;
+ memset(&fEvent, 0, sizeof(fEvent));
+ fEvent.type = event->fwEvent;
+ fEvent.error = event->error;
+ fEvent.errorCode = event->errorCode;
- fw_invokeFrameworkListener(framework, listener->listener, &event, listener->bundle);
+ fw_invokeFrameworkListener(framework, listener->listener, &fEvent, listener->bundle);
}
celixThreadMutex_unlock(&framework->frameworkListenersLock);
+ } else if (event->type == CELIX_REGISTER_SERVICE_EVENT) {
+ //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Handling fw service registration event for service '%s' with svc id %li", event->serviceName, event->registerServiceId);
+ service_registration_t* reg = NULL;
+ celix_status_t status;
+ if (event->factory != NULL) {
+ status = celix_serviceRegistry_registerServiceFactory(framework->registry, event->bndEntry->bnd, event->serviceName, event->factory, event->properties, event->registerServiceId, ®);
+ } else {
+ status = celix_serviceRegistry_registerService(framework->registry, event->bndEntry->bnd, event->serviceName, event->svc, event->properties, event->registerServiceId, ®);
+ }
+ if (status != CELIX_SUCCESS) {
+ fw_log(framework->logger, CELIX_LOG_LEVEL_ERROR, "Could not register service async. svc name is %s, error is %s", event->serviceName, celix_strerror(status));
+ } else if (event->registerCallback != NULL) {
+ event->registerCallback(event->registerData, serviceRegistration_getServiceId(reg));
+ }
+ } else if (event->type == CELIX_UNREGISTER_SERVICE_EVENT) {
+ //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Handling fw service unregister event for service id %li", event->unregisterServiceId);
+ celix_serviceRegistry_unregisterService(framework->registry, event->bndEntry->bnd, event->unregisterServiceId);
+ } else if (event->type == CELIX_GENERIC_EVENT) {
+ //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Handling event %s", event->eventName);
+ if (event->genericProcess != NULL) {
+ event->genericProcess(event->genericProcessData);
+ }
+ }
+
+ if (event->doneCallback != NULL) {
+ event->doneCallback(event->doneData);
}
}
-static inline void fw_handleEvents(celix_framework_t* framework, celix_array_list_t* localRequests) {
+static inline celix_framework_event_t* fw_topEventFromQueue(celix_framework_t* fw) {
+ celix_framework_event_t* e = NULL;
+ celixThreadMutex_lock(&fw->dispatcher.mutex);
+ if (fw->dispatcher.eventQueueSize > 0) {
+ e = &fw->dispatcher.eventQueue[fw->dispatcher.eventQueueFirstEntry];
+ } else if (celix_arrayList_size(fw->dispatcher.dynamicEventQueue) > 0) {
+ e = celix_arrayList_get(fw->dispatcher.dynamicEventQueue, 0);
+ }
+ celixThreadMutex_unlock(&fw->dispatcher.mutex);
+ return e;
+}
+
+static inline bool fw_removeTopEventFromQueue(celix_framework_t* fw) {
+ bool dynamicallyAllocated = false;
+ celixThreadMutex_lock(&fw->dispatcher.mutex);
+ if (fw->dispatcher.eventQueueSize > 0) {
+ fw->dispatcher.eventQueueFirstEntry = (fw->dispatcher.eventQueueFirstEntry+1) % CELIX_FRAMEWORK_STATIC_EVENT_QUEUE_SIZE;
+ fw->dispatcher.eventQueueSize -= 1;
Review comment:
What exactly do you mean with "simpler approach"? As far as simplicity for reading/understanding the code, this is a complex approach. It took me more than 15 minutes to understand the code related to adding/retrieving events. The difficulty comes from understanding two mechanisms instead of one, but also increases cyclomatic complexity more than necessary.
Or do you mean simple in terms of implementation? As for that, using something like https://github.com/dhess/c-ringbuf would arguably be simpler, no? I think one could even argue that making a celix-specific dynamically sized ringbuffer implementation would also be easier to test/maintain than the current code. Speaking of which, are the `celix_framework_addToEventQueue`, `fw_removeTopEventFromQueue` etc functions tested?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] Oipo commented on a change in pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
Oipo commented on a change in pull request #286:
URL: https://github.com/apache/celix/pull/286#discussion_r545940628
##########
File path: libs/framework/src/bundle_context.c
##########
@@ -80,15 +86,19 @@ celix_status_t bundleContext_destroy(bundle_context_pt context) {
celixThreadMutex_lock(&context->mutex);
- bundleContext_cleanupBundleTrackers(context);
- bundleContext_cleanupServiceTrackers(context);
- bundleContext_cleanupServiceTrackerTrackers(context);
+ assert(hashMap_size(context->bundleTrackers) == 0);
+ hashMap_destroy(context->bundleTrackers, false, false);
+ assert(hashMap_size(context->serviceTrackers) == 0);
+ hashMap_destroy(context->serviceTrackers, false, false);
+ assert(hashMap_size(context->metaTrackers) == 0);
+ hashMap_destroy(context->metaTrackers, false, false);
+ assert(celix_arrayList_size(context->svcRegistrations) == 0);
+ celix_arrayList_destroy(context->svcRegistrations);
- //NOTE still present service registrations will be cleared during bundle stop in the
+ //NOTE still present service registrations will be cleared during bundle stop in the
Review comment:
Is the comment still relevant?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] codecov-io edited a comment on pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #286:
URL: https://github.com/apache/celix/pull/286#issuecomment-711176467
# [Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=h1) Report
> Merging [#286](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=desc) (a32487d) into [master](https://codecov.io/gh/apache/celix/commit/7a26aea42fd33849e54a33d69969e42d096b9dcd?el=desc) (7a26aea) will **increase** coverage by `0.88%`.
> The diff coverage is `81.45%`.
[](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #286 +/- ##
==========================================
+ Coverage 66.69% 67.57% +0.88%
==========================================
Files 147 147
Lines 29947 30412 +465
==========================================
+ Hits 19974 20552 +578
+ Misses 9973 9860 -113
```
| [Impacted Files](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [libs/framework/src/celix\_framework\_factory.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2NlbGl4X2ZyYW1ld29ya19mYWN0b3J5LmM=) | `87.50% <ø> (-0.74%)` | :arrow_down: |
| [bundles/pubsub/pubsub\_utils/src/pubsub\_matching.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3V0aWxzL3NyYy9wdWJzdWJfbWF0Y2hpbmcuYw==) | `78.62% <50.00%> (+3.29%)` | :arrow_up: |
| [bundles/http\_admin/http\_admin/src/http\_admin.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9odHRwX2FkbWluL2h0dHBfYWRtaW4vc3JjL2h0dHBfYWRtaW4uYw==) | `68.04% <66.66%> (+2.63%)` | :arrow_up: |
| [libs/framework/src/service\_tracker.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfdHJhY2tlci5j) | `79.50% <76.47%> (+2.36%)` | :arrow_up: |
| [libs/framework/src/bundle.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2J1bmRsZS5j) | `68.31% <76.92%> (+0.08%)` | :arrow_up: |
| [libs/framework/src/bundle\_context.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2J1bmRsZV9jb250ZXh0LmM=) | `69.93% <78.48%> (-0.19%)` | :arrow_down: |
| [libs/framework/src/framework.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2ZyYW1ld29yay5j) | `74.66% <83.54%> (-0.66%)` | :arrow_down: |
| [libs/framework/src/service\_registry.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfcmVnaXN0cnkuYw==) | `73.35% <86.90%> (+1.66%)` | :arrow_up: |
| [...dles/pubsub/pubsub\_spi/src/pubsub\_endpoint\_match.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3NwaS9zcmMvcHVic3ViX2VuZHBvaW50X21hdGNoLmM=) | `56.37% <87.50%> (-2.23%)` | :arrow_down: |
| [libs/utils/src/utils.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy91dGlscy9zcmMvdXRpbHMuYw==) | `87.39% <90.90%> (+4.66%)` | :arrow_up: |
| ... and [29 more](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=footer). Last update [7a26aea...a32487d](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes commented on a change in pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
pnoltes commented on a change in pull request #286:
URL: https://github.com/apache/celix/pull/286#discussion_r507163568
##########
File path: libs/utils/src/utils.c
##########
@@ -150,9 +150,9 @@ int utils_compareServiceIdsAndRanking(unsigned long servId, long servRank, unsig
if (servId == otherServId) {
result = 0;
} else if (servRank != otherServRank) {
- result = servRank < otherServRank ? -1 : 1;
Review comment:
its a bug. the compare ranking function combined with sort resulted in the wrong ordering.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] codecov-io edited a comment on pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #286:
URL: https://github.com/apache/celix/pull/286#issuecomment-711176467
# [Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=h1) Report
> Merging [#286](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=desc) (6ebe5b9) into [master](https://codecov.io/gh/apache/celix/commit/dd4dca1d52c36a794dfac71867f0abc3fd81241c?el=desc) (dd4dca1) will **increase** coverage by `0.48%`.
> The diff coverage is `82.42%`.
[](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #286 +/- ##
==========================================
+ Coverage 68.17% 68.65% +0.48%
==========================================
Files 148 148
Lines 30088 30556 +468
==========================================
+ Hits 20511 20979 +468
Misses 9577 9577
```
| [Impacted Files](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [libs/framework/src/celix\_framework\_factory.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2NlbGl4X2ZyYW1ld29ya19mYWN0b3J5LmM=) | `87.50% <ø> (-0.74%)` | :arrow_down: |
| [bundles/pubsub/pubsub\_utils/src/pubsub\_matching.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3V0aWxzL3NyYy9wdWJzdWJfbWF0Y2hpbmcuYw==) | `78.62% <50.00%> (+3.29%)` | :arrow_up: |
| [bundles/http\_admin/http\_admin/src/http\_admin.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9odHRwX2FkbWluL2h0dHBfYWRtaW4vc3JjL2h0dHBfYWRtaW4uYw==) | `68.04% <66.66%> (+2.63%)` | :arrow_up: |
| [libs/framework/src/bundle.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2J1bmRsZS5j) | `68.31% <76.92%> (+0.08%)` | :arrow_up: |
| [libs/framework/src/service\_tracker.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfdHJhY2tlci5j) | `81.98% <78.82%> (+1.12%)` | :arrow_up: |
| [libs/framework/src/bundle\_context.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2J1bmRsZV9jb250ZXh0LmM=) | `70.35% <79.31%> (+0.23%)` | :arrow_up: |
| [libs/framework/src/framework.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2ZyYW1ld29yay5j) | `74.66% <83.54%> (-0.88%)` | :arrow_down: |
| [...dles/pubsub/pubsub\_spi/src/pubsub\_endpoint\_match.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3NwaS9zcmMvcHVic3ViX2VuZHBvaW50X21hdGNoLmM=) | `56.37% <87.50%> (-2.23%)` | :arrow_down: |
| [libs/framework/src/service\_registry.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfcmVnaXN0cnkuYw==) | `73.87% <89.28%> (+2.17%)` | :arrow_up: |
| [libs/utils/src/utils.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy91dGlscy9zcmMvdXRpbHMuYw==) | `87.39% <90.90%> (+4.66%)` | :arrow_up: |
| ... and [28 more](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=footer). Last update [dd4dca1...6ebe5b9](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes commented on a change in pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
pnoltes commented on a change in pull request #286:
URL: https://github.com/apache/celix/pull/286#discussion_r507160980
##########
File path: libs/framework/gtest/src/bundle_context_bundles_tests.cpp
##########
@@ -378,6 +378,90 @@ TEST_F(CelixBundleContextBundlesTests, trackBundlesTest) {
celix_bundleContext_stopTracker(ctx, trackerId);
};
+
+TEST_F(CelixBundleContextBundlesTests, trackBundlesTestAsync) {
+ struct data {
+ std::atomic<int> installedCount{0};
+ std::atomic<int> startedCount{0};
+ std::atomic<int> stoppedCount{0};
+ };
+ struct data data;
+
+ auto installed = [](void *handle, const bundle_t *bnd) {
+ auto *d = static_cast<struct data*>(handle);
+ EXPECT_TRUE(bnd != nullptr);
+ d->installedCount.fetch_add(1);
Review comment:
true, but for atomics I prefer the more explicit functions.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes commented on a change in pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
pnoltes commented on a change in pull request #286:
URL: https://github.com/apache/celix/pull/286#discussion_r520042933
##########
File path: libs/framework/src/framework.c
##########
@@ -2015,67 +1934,112 @@ static void fw_handleEventRequest(celix_framework_t *framework, request_t* reque
for (int i = 0; i < celix_arrayList_size(localListeners); ++i) {
fw_bundle_listener_pt listener = arrayList_get(localListeners, i);
- bundle_event_t event;
- memset(&event, 0, sizeof(event));
- event.bnd = request->bndEntry->bnd;
- event.type = request->eventType;
- fw_invokeBundleListener(framework, listener->listener, &event, listener->bundle);
+ bundle_event_t bEvent;
+ memset(&bEvent, 0, sizeof(bEvent));
+ bEvent.bnd = event->bndEntry->bnd;
+ bEvent.type = event->bundleEvent;
+ fw_invokeBundleListener(framework, listener->listener, &bEvent, listener->bundle);
fw_bundleListener_decreaseUseCount(listener);
}
celix_arrayList_destroy(localListeners);
- } else if (request->type == FRAMEWORK_EVENT_TYPE) {
+ } else if (event->type == CELIX_FRAMEWORK_EVENT_TYPE) {
+ //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Handling fw event");
celixThreadMutex_lock(&framework->frameworkListenersLock);
- //TODO refactor use of framework listeners to use a useCount + conditition.
for (int i = 0; i < celix_arrayList_size(framework->frameworkListeners); ++i) {
fw_framework_listener_pt listener = celix_arrayList_get(framework->frameworkListeners, i);
- framework_event_t event;
- memset(&event, 0, sizeof(event));
- event.type = request->eventType;
- event.error = request->error;
- event.errorCode = request->errorCode;
+ framework_event_t fEvent;
+ memset(&fEvent, 0, sizeof(fEvent));
+ fEvent.type = event->fwEvent;
+ fEvent.error = event->error;
+ fEvent.errorCode = event->errorCode;
- fw_invokeFrameworkListener(framework, listener->listener, &event, listener->bundle);
+ fw_invokeFrameworkListener(framework, listener->listener, &fEvent, listener->bundle);
}
celixThreadMutex_unlock(&framework->frameworkListenersLock);
+ } else if (event->type == CELIX_REGISTER_SERVICE_EVENT) {
+ //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Handling fw service registration event for service '%s' with svc id %li", event->serviceName, event->registerServiceId);
+ service_registration_t* reg = NULL;
+ celix_status_t status;
+ if (event->factory != NULL) {
+ status = celix_serviceRegistry_registerServiceFactory(framework->registry, event->bndEntry->bnd, event->serviceName, event->factory, event->properties, event->registerServiceId, ®);
+ } else {
+ status = celix_serviceRegistry_registerService(framework->registry, event->bndEntry->bnd, event->serviceName, event->svc, event->properties, event->registerServiceId, ®);
+ }
+ if (status != CELIX_SUCCESS) {
+ fw_log(framework->logger, CELIX_LOG_LEVEL_ERROR, "Could not register service async. svc name is %s, error is %s", event->serviceName, celix_strerror(status));
+ } else if (event->registerCallback != NULL) {
+ event->registerCallback(event->registerData, serviceRegistration_getServiceId(reg));
+ }
+ } else if (event->type == CELIX_UNREGISTER_SERVICE_EVENT) {
+ //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Handling fw service unregister event for service id %li", event->unregisterServiceId);
+ celix_serviceRegistry_unregisterService(framework->registry, event->bndEntry->bnd, event->unregisterServiceId);
+ } else if (event->type == CELIX_GENERIC_EVENT) {
+ //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Handling event %s", event->eventName);
+ if (event->genericProcess != NULL) {
+ event->genericProcess(event->genericProcessData);
+ }
+ }
+
+ if (event->doneCallback != NULL) {
+ event->doneCallback(event->doneData);
}
}
-static inline void fw_handleEvents(celix_framework_t* framework, celix_array_list_t* localRequests) {
+static inline celix_framework_event_t* fw_topEventFromQueue(celix_framework_t* fw) {
+ celix_framework_event_t* e = NULL;
+ celixThreadMutex_lock(&fw->dispatcher.mutex);
+ if (fw->dispatcher.eventQueueSize > 0) {
+ e = &fw->dispatcher.eventQueue[fw->dispatcher.eventQueueFirstEntry];
+ } else if (celix_arrayList_size(fw->dispatcher.dynamicEventQueue) > 0) {
+ e = celix_arrayList_get(fw->dispatcher.dynamicEventQueue, 0);
+ }
+ celixThreadMutex_unlock(&fw->dispatcher.mutex);
+ return e;
+}
+
+static inline bool fw_removeTopEventFromQueue(celix_framework_t* fw) {
+ bool dynamicallyAllocated = false;
+ celixThreadMutex_lock(&fw->dispatcher.mutex);
+ if (fw->dispatcher.eventQueueSize > 0) {
+ fw->dispatcher.eventQueueFirstEntry = (fw->dispatcher.eventQueueFirstEntry+1) % CELIX_FRAMEWORK_STATIC_EVENT_QUEUE_SIZE;
+ fw->dispatcher.eventQueueSize -= 1;
Review comment:
I meant only using a ring buffer instead of a static ring buffer and a fallback to a dynamic array is a simpler approach.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes commented on a change in pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
pnoltes commented on a change in pull request #286:
URL: https://github.com/apache/celix/pull/286#discussion_r520093545
##########
File path: libs/framework/src/framework.c
##########
@@ -2015,67 +1934,112 @@ static void fw_handleEventRequest(celix_framework_t *framework, request_t* reque
for (int i = 0; i < celix_arrayList_size(localListeners); ++i) {
fw_bundle_listener_pt listener = arrayList_get(localListeners, i);
- bundle_event_t event;
- memset(&event, 0, sizeof(event));
- event.bnd = request->bndEntry->bnd;
- event.type = request->eventType;
- fw_invokeBundleListener(framework, listener->listener, &event, listener->bundle);
+ bundle_event_t bEvent;
+ memset(&bEvent, 0, sizeof(bEvent));
+ bEvent.bnd = event->bndEntry->bnd;
+ bEvent.type = event->bundleEvent;
+ fw_invokeBundleListener(framework, listener->listener, &bEvent, listener->bundle);
fw_bundleListener_decreaseUseCount(listener);
}
celix_arrayList_destroy(localListeners);
- } else if (request->type == FRAMEWORK_EVENT_TYPE) {
+ } else if (event->type == CELIX_FRAMEWORK_EVENT_TYPE) {
+ //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Handling fw event");
celixThreadMutex_lock(&framework->frameworkListenersLock);
- //TODO refactor use of framework listeners to use a useCount + conditition.
for (int i = 0; i < celix_arrayList_size(framework->frameworkListeners); ++i) {
fw_framework_listener_pt listener = celix_arrayList_get(framework->frameworkListeners, i);
- framework_event_t event;
- memset(&event, 0, sizeof(event));
- event.type = request->eventType;
- event.error = request->error;
- event.errorCode = request->errorCode;
+ framework_event_t fEvent;
+ memset(&fEvent, 0, sizeof(fEvent));
+ fEvent.type = event->fwEvent;
+ fEvent.error = event->error;
+ fEvent.errorCode = event->errorCode;
- fw_invokeFrameworkListener(framework, listener->listener, &event, listener->bundle);
+ fw_invokeFrameworkListener(framework, listener->listener, &fEvent, listener->bundle);
}
celixThreadMutex_unlock(&framework->frameworkListenersLock);
+ } else if (event->type == CELIX_REGISTER_SERVICE_EVENT) {
+ //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Handling fw service registration event for service '%s' with svc id %li", event->serviceName, event->registerServiceId);
+ service_registration_t* reg = NULL;
+ celix_status_t status;
+ if (event->factory != NULL) {
+ status = celix_serviceRegistry_registerServiceFactory(framework->registry, event->bndEntry->bnd, event->serviceName, event->factory, event->properties, event->registerServiceId, ®);
+ } else {
+ status = celix_serviceRegistry_registerService(framework->registry, event->bndEntry->bnd, event->serviceName, event->svc, event->properties, event->registerServiceId, ®);
+ }
+ if (status != CELIX_SUCCESS) {
+ fw_log(framework->logger, CELIX_LOG_LEVEL_ERROR, "Could not register service async. svc name is %s, error is %s", event->serviceName, celix_strerror(status));
+ } else if (event->registerCallback != NULL) {
+ event->registerCallback(event->registerData, serviceRegistration_getServiceId(reg));
+ }
+ } else if (event->type == CELIX_UNREGISTER_SERVICE_EVENT) {
+ //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Handling fw service unregister event for service id %li", event->unregisterServiceId);
+ celix_serviceRegistry_unregisterService(framework->registry, event->bndEntry->bnd, event->unregisterServiceId);
+ } else if (event->type == CELIX_GENERIC_EVENT) {
+ //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Handling event %s", event->eventName);
+ if (event->genericProcess != NULL) {
+ event->genericProcess(event->genericProcessData);
+ }
+ }
+
+ if (event->doneCallback != NULL) {
+ event->doneCallback(event->doneData);
}
}
-static inline void fw_handleEvents(celix_framework_t* framework, celix_array_list_t* localRequests) {
+static inline celix_framework_event_t* fw_topEventFromQueue(celix_framework_t* fw) {
+ celix_framework_event_t* e = NULL;
+ celixThreadMutex_lock(&fw->dispatcher.mutex);
+ if (fw->dispatcher.eventQueueSize > 0) {
+ e = &fw->dispatcher.eventQueue[fw->dispatcher.eventQueueFirstEntry];
+ } else if (celix_arrayList_size(fw->dispatcher.dynamicEventQueue) > 0) {
+ e = celix_arrayList_get(fw->dispatcher.dynamicEventQueue, 0);
+ }
+ celixThreadMutex_unlock(&fw->dispatcher.mutex);
+ return e;
+}
+
+static inline bool fw_removeTopEventFromQueue(celix_framework_t* fw) {
+ bool dynamicallyAllocated = false;
+ celixThreadMutex_lock(&fw->dispatcher.mutex);
+ if (fw->dispatcher.eventQueueSize > 0) {
+ fw->dispatcher.eventQueueFirstEntry = (fw->dispatcher.eventQueueFirstEntry+1) % CELIX_FRAMEWORK_STATIC_EVENT_QUEUE_SIZE;
+ fw->dispatcher.eventQueueSize -= 1;
Review comment:
I reverted the changes for only using a ring buffer, which then can increase in size.
This is more difficult than I initial expected.
Currently all the waitFor call (e.g. waitForAsyncRegistration) assume that if an event is not in the event queue, the event is completely finished.
This is implemented by first getting the top event from the queue, handle the event (with no lock on the event queue needed) and then removing the event.
If during this process the events are moved (i.e. the ring buffer size is increased), this will crash.
Maybe this should be designed and implemented differently, but I would prefer a new PR for this.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] codecov-io edited a comment on pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #286:
URL: https://github.com/apache/celix/pull/286#issuecomment-711176467
# [Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=h1) Report
> Merging [#286](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=desc) (c9600a4) into [master](https://codecov.io/gh/apache/celix/commit/7a26aea42fd33849e54a33d69969e42d096b9dcd?el=desc) (7a26aea) will **increase** coverage by `0.58%`.
> The diff coverage is `81.45%`.
[](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #286 +/- ##
==========================================
+ Coverage 66.69% 67.28% +0.58%
==========================================
Files 147 147
Lines 29947 30412 +465
==========================================
+ Hits 19974 20462 +488
+ Misses 9973 9950 -23
```
| [Impacted Files](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [libs/framework/src/celix\_framework\_factory.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2NlbGl4X2ZyYW1ld29ya19mYWN0b3J5LmM=) | `87.50% <ø> (-0.74%)` | :arrow_down: |
| [bundles/pubsub/pubsub\_utils/src/pubsub\_matching.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3V0aWxzL3NyYy9wdWJzdWJfbWF0Y2hpbmcuYw==) | `78.62% <50.00%> (+3.29%)` | :arrow_up: |
| [bundles/http\_admin/http\_admin/src/http\_admin.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9odHRwX2FkbWluL2h0dHBfYWRtaW4vc3JjL2h0dHBfYWRtaW4uYw==) | `68.04% <66.66%> (+2.63%)` | :arrow_up: |
| [libs/framework/src/service\_tracker.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfdHJhY2tlci5j) | `79.50% <76.47%> (+2.36%)` | :arrow_up: |
| [libs/framework/src/bundle.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2J1bmRsZS5j) | `68.31% <76.92%> (+0.08%)` | :arrow_up: |
| [libs/framework/src/bundle\_context.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2J1bmRsZV9jb250ZXh0LmM=) | `69.93% <78.48%> (-0.19%)` | :arrow_down: |
| [libs/framework/src/framework.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2ZyYW1ld29yay5j) | `74.66% <83.54%> (-0.66%)` | :arrow_down: |
| [libs/framework/src/service\_registry.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfcmVnaXN0cnkuYw==) | `73.35% <86.90%> (+1.66%)` | :arrow_up: |
| [...dles/pubsub/pubsub\_spi/src/pubsub\_endpoint\_match.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3NwaS9zcmMvcHVic3ViX2VuZHBvaW50X21hdGNoLmM=) | `56.37% <87.50%> (-2.23%)` | :arrow_down: |
| [libs/utils/src/utils.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy91dGlscy9zcmMvdXRpbHMuYw==) | `87.39% <90.90%> (+4.66%)` | :arrow_up: |
| ... and [29 more](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=footer). Last update [7a26aea...c9600a4](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] codecov-io edited a comment on pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #286:
URL: https://github.com/apache/celix/pull/286#issuecomment-711176467
# [Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=h1) Report
> Merging [#286](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=desc) (b57c48c) into [master](https://codecov.io/gh/apache/celix/commit/4ebde14e5cd089223339f5a21c07c0cea9ab34f8?el=desc) (4ebde14) will **increase** coverage by `0.12%`.
> The diff coverage is `77.82%`.
[](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #286 +/- ##
==========================================
+ Coverage 68.29% 68.42% +0.12%
==========================================
Files 136 136
Lines 27379 27784 +405
==========================================
+ Hits 18698 19010 +312
- Misses 8681 8774 +93
```
| [Impacted Files](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [libs/framework/src/celix\_framework\_factory.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2NlbGl4X2ZyYW1ld29ya19mYWN0b3J5LmM=) | `87.50% <ø> (-0.74%)` | :arrow_down: |
| [bundles/http\_admin/http\_admin/src/http\_admin.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9odHRwX2FkbWluL2h0dHBfYWRtaW4vc3JjL2h0dHBfYWRtaW4uYw==) | `68.04% <66.66%> (+2.63%)` | :arrow_up: |
| [libs/framework/src/bundle\_context.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2J1bmRsZV9jb250ZXh0LmM=) | `66.51% <72.51%> (-3.61%)` | :arrow_down: |
| [libs/framework/src/service\_tracker.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfdHJhY2tlci5j) | `79.50% <76.47%> (+2.18%)` | :arrow_up: |
| [libs/framework/src/bundle.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2J1bmRsZS5j) | `68.31% <76.92%> (+0.08%)` | :arrow_up: |
| [libs/framework/src/framework.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2ZyYW1ld29yay5j) | `73.38% <78.53%> (-2.15%)` | :arrow_down: |
| [libs/framework/src/service\_registry.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfcmVnaXN0cnkuYw==) | `73.35% <86.90%> (+1.66%)` | :arrow_up: |
| [bundles/logging/log\_admin/src/celix\_log\_admin.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9sb2dnaW5nL2xvZ19hZG1pbi9zcmMvY2VsaXhfbG9nX2FkbWluLmM=) | `99.73% <100.00%> (-0.01%)` | :arrow_down: |
| [bundles/pubsub/pubsub\_spi/src/pubsub\_endpoint.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3NwaS9zcmMvcHVic3ViX2VuZHBvaW50LmM=) | `72.11% <100.00%> (-3.13%)` | :arrow_down: |
| [...gy\_manager/tms\_tst/disc\_mock/disc\_mock\_activator.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9yZW1vdGVfc2VydmljZXMvdG9wb2xvZ3lfbWFuYWdlci90bXNfdHN0L2Rpc2NfbW9jay9kaXNjX21vY2tfYWN0aXZhdG9yLmM=) | `91.30% <100.00%> (+0.12%)` | :arrow_up: |
| ... and [18 more](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=footer). Last update [4ebde14...b57c48c](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes commented on a change in pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
pnoltes commented on a change in pull request #286:
URL: https://github.com/apache/celix/pull/286#discussion_r507166263
##########
File path: libs/framework/src/service_registry.c
##########
@@ -1046,6 +920,137 @@ static inline void celix_waitAndDestroyServiceListener(celix_service_registry_se
free(entry);
}
+/**
+ * Create a LDAP filter for the provided filter parts.
Review comment:
correct and removed documentation
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] codecov-io edited a comment on pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #286:
URL: https://github.com/apache/celix/pull/286#issuecomment-711176467
# [Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=h1) Report
> Merging [#286](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=desc) (042ea62) into [master](https://codecov.io/gh/apache/celix/commit/7a26aea42fd33849e54a33d69969e42d096b9dcd?el=desc) (7a26aea) will **increase** coverage by `0.58%`.
> The diff coverage is `81.45%`.
[](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #286 +/- ##
==========================================
+ Coverage 66.69% 67.28% +0.58%
==========================================
Files 147 147
Lines 29947 30410 +463
==========================================
+ Hits 19974 20460 +486
+ Misses 9973 9950 -23
```
| [Impacted Files](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [libs/framework/src/celix\_framework\_factory.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2NlbGl4X2ZyYW1ld29ya19mYWN0b3J5LmM=) | `87.50% <ø> (-0.74%)` | :arrow_down: |
| [bundles/pubsub/pubsub\_utils/src/pubsub\_matching.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3V0aWxzL3NyYy9wdWJzdWJfbWF0Y2hpbmcuYw==) | `78.62% <50.00%> (+3.29%)` | :arrow_up: |
| [bundles/http\_admin/http\_admin/src/http\_admin.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9odHRwX2FkbWluL2h0dHBfYWRtaW4vc3JjL2h0dHBfYWRtaW4uYw==) | `68.04% <66.66%> (+2.63%)` | :arrow_up: |
| [libs/framework/src/service\_tracker.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfdHJhY2tlci5j) | `79.50% <76.47%> (+2.36%)` | :arrow_up: |
| [libs/framework/src/bundle.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2J1bmRsZS5j) | `68.31% <76.92%> (+0.08%)` | :arrow_up: |
| [libs/framework/src/bundle\_context.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2J1bmRsZV9jb250ZXh0LmM=) | `69.87% <78.48%> (-0.26%)` | :arrow_down: |
| [libs/framework/src/framework.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2ZyYW1ld29yay5j) | `74.66% <83.54%> (-0.66%)` | :arrow_down: |
| [libs/framework/src/service\_registry.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfcmVnaXN0cnkuYw==) | `73.35% <86.90%> (+1.66%)` | :arrow_up: |
| [...dles/pubsub/pubsub\_spi/src/pubsub\_endpoint\_match.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3NwaS9zcmMvcHVic3ViX2VuZHBvaW50X21hdGNoLmM=) | `56.37% <87.50%> (-2.23%)` | :arrow_down: |
| [libs/utils/src/utils.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy91dGlscy9zcmMvdXRpbHMuYw==) | `87.39% <90.90%> (+4.66%)` | :arrow_up: |
| ... and [29 more](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=footer). Last update [7a26aea...042ea62](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] codecov-io edited a comment on pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #286:
URL: https://github.com/apache/celix/pull/286#issuecomment-711176467
# [Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=h1) Report
> Merging [#286](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=desc) (3776a46) into [master](https://codecov.io/gh/apache/celix/commit/7a26aea42fd33849e54a33d69969e42d096b9dcd?el=desc) (7a26aea) will **increase** coverage by `0.57%`.
> The diff coverage is `81.17%`.
[](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #286 +/- ##
==========================================
+ Coverage 66.69% 67.27% +0.57%
==========================================
Files 147 147
Lines 29947 30411 +464
==========================================
+ Hits 19974 20458 +484
+ Misses 9973 9953 -20
```
| [Impacted Files](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [libs/framework/src/celix\_framework\_factory.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2NlbGl4X2ZyYW1ld29ya19mYWN0b3J5LmM=) | `87.50% <ø> (-0.74%)` | :arrow_down: |
| [bundles/pubsub/pubsub\_utils/src/pubsub\_matching.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3V0aWxzL3NyYy9wdWJzdWJfbWF0Y2hpbmcuYw==) | `78.62% <50.00%> (+3.29%)` | :arrow_up: |
| [bundles/http\_admin/http\_admin/src/http\_admin.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9odHRwX2FkbWluL2h0dHBfYWRtaW4vc3JjL2h0dHBfYWRtaW4uYw==) | `68.04% <66.66%> (+2.63%)` | :arrow_up: |
| [libs/framework/src/service\_tracker.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfdHJhY2tlci5j) | `79.50% <76.47%> (+2.36%)` | :arrow_up: |
| [libs/framework/src/bundle.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2J1bmRsZS5j) | `68.31% <76.92%> (+0.08%)` | :arrow_up: |
| [libs/framework/src/bundle\_context.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2J1bmRsZV9jb250ZXh0LmM=) | `69.93% <78.48%> (-0.19%)` | :arrow_down: |
| [libs/framework/src/framework.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2ZyYW1ld29yay5j) | `74.66% <83.54%> (-0.66%)` | :arrow_down: |
| [libs/framework/src/service\_registry.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfcmVnaXN0cnkuYw==) | `73.35% <86.90%> (+1.66%)` | :arrow_up: |
| [...dles/pubsub/pubsub\_spi/src/pubsub\_endpoint\_match.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3NwaS9zcmMvcHVic3ViX2VuZHBvaW50X21hdGNoLmM=) | `56.37% <87.50%> (-2.23%)` | :arrow_down: |
| [libs/utils/src/utils.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy91dGlscy9zcmMvdXRpbHMuYw==) | `87.39% <90.90%> (+4.66%)` | :arrow_up: |
| ... and [28 more](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=footer). Last update [7a26aea...3776a46](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] codecov-io edited a comment on pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #286:
URL: https://github.com/apache/celix/pull/286#issuecomment-711176467
# [Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=h1) Report
> Merging [#286](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=desc) (87ba7ff) into [master](https://codecov.io/gh/apache/celix/commit/ab13f6da69e42851f5b065cddfe59ab14eb2b02c?el=desc) (ab13f6d) will **increase** coverage by `0.22%`.
> The diff coverage is `79.94%`.
[](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #286 +/- ##
==========================================
+ Coverage 66.98% 67.20% +0.22%
==========================================
Files 147 147
Lines 29946 30403 +457
==========================================
+ Hits 20058 20432 +374
- Misses 9888 9971 +83
```
| [Impacted Files](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [libs/framework/src/celix\_framework\_factory.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2NlbGl4X2ZyYW1ld29ya19mYWN0b3J5LmM=) | `87.50% <ø> (-0.74%)` | :arrow_down: |
| [bundles/pubsub/pubsub\_utils/src/pubsub\_matching.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3V0aWxzL3NyYy9wdWJzdWJfbWF0Y2hpbmcuYw==) | `78.62% <50.00%> (+3.29%)` | :arrow_up: |
| [bundles/http\_admin/http\_admin/src/http\_admin.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9odHRwX2FkbWluL2h0dHBfYWRtaW4vc3JjL2h0dHBfYWRtaW4uYw==) | `68.04% <66.66%> (+2.63%)` | :arrow_up: |
| [libs/framework/src/service\_tracker.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfdHJhY2tlci5j) | `79.50% <76.47%> (+2.18%)` | :arrow_up: |
| [libs/framework/src/bundle.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2J1bmRsZS5j) | `68.31% <76.92%> (+0.08%)` | :arrow_up: |
| [libs/framework/src/bundle\_context.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2J1bmRsZV9jb250ZXh0LmM=) | `69.93% <78.48%> (-0.19%)` | :arrow_down: |
| [libs/framework/src/framework.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2ZyYW1ld29yay5j) | `73.50% <79.05%> (-2.02%)` | :arrow_down: |
| [libs/framework/src/service\_registry.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfcmVnaXN0cnkuYw==) | `73.35% <86.90%> (+1.66%)` | :arrow_up: |
| [...dles/pubsub/pubsub\_spi/src/pubsub\_endpoint\_match.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3NwaS9zcmMvcHVic3ViX2VuZHBvaW50X21hdGNoLmM=) | `56.37% <87.50%> (-2.23%)` | :arrow_down: |
| [libs/utils/src/utils.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy91dGlscy9zcmMvdXRpbHMuYw==) | `87.39% <90.90%> (+4.66%)` | :arrow_up: |
| ... and [24 more](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=footer). Last update [ab13f6d...87ba7ff](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes commented on a change in pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
pnoltes commented on a change in pull request #286:
URL: https://github.com/apache/celix/pull/286#discussion_r507163568
##########
File path: libs/utils/src/utils.c
##########
@@ -150,9 +150,9 @@ int utils_compareServiceIdsAndRanking(unsigned long servId, long servRank, unsig
if (servId == otherServId) {
result = 0;
} else if (servRank != otherServRank) {
- result = servRank < otherServRank ? -1 : 1;
Review comment:
is a bug. the compare ranking function combined with sort resulted in the wrong ordering.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes commented on pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
pnoltes commented on pull request #286:
URL: https://github.com/apache/celix/pull/286#issuecomment-741983089
> The address sanitizer finds the following:
>
> [stackuseafterfree.txt](https://github.com/apache/celix/files/5364178/stackuseafterfree.txt)
>
> The test uses the count variable asynchronously, leading to situations where the address to count is used after the test has quit.
should be solved in commit 3776a46
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes commented on a change in pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
pnoltes commented on a change in pull request #286:
URL: https://github.com/apache/celix/pull/286#discussion_r507165905
##########
File path: libs/framework/src/framework.c
##########
@@ -2528,13 +2631,108 @@ bool celix_framework_startBundle(celix_framework_t *fw, long bndId) {
}
void celix_framework_waitForEmptyEventQueue(celix_framework_t *fw) {
+ assert(!celix_framework_isCurrentThreadTheEventLoop(fw));
+
celixThreadMutex_lock(&fw->dispatcher.mutex);
- while ((celix_arrayList_size(fw->dispatcher.requests) + fw->dispatcher.nrOfLocalRequest) != 0) {
+ while (fw->dispatcher.eventQueueSize > 0 || celix_arrayList_size(fw->dispatcher.dynamicEventQueue) > 0) {
celixThreadCondition_wait(&fw->dispatcher.cond, &fw->dispatcher.mutex);
}
celixThreadMutex_unlock(&fw->dispatcher.mutex);
}
+void celix_framework_waitForEvents(celix_framework_t* fw, long bndId) {
+ assert(!celix_framework_isCurrentThreadTheEventLoop(fw));
+
+ celixThreadMutex_lock(&fw->dispatcher.mutex);
+ bool eventInProgress = true;
+ while (eventInProgress) {
+ eventInProgress = false;
+ for (int i = 0; i < fw->dispatcher.eventQueueSize; ++i) {
+ int index = (fw->dispatcher.eventQueueFirstEntry + i) % CELIX_FRAMEWORK_STATIC_EVENT_QUEUE_SIZE;
+ celix_framework_event_t* e = &fw->dispatcher.eventQueue[index];
+ if (e->bndEntry != NULL && e->bndEntry->bndId == bndId) {
+ eventInProgress = true;
+ break;
+ }
+ }
+ for (int i = 0; !eventInProgress && i < celix_arrayList_size(fw->dispatcher.dynamicEventQueue); ++i) {
+ celix_framework_event_t* e = celix_arrayList_get(fw->dispatcher.dynamicEventQueue, i);
+ if (e->bndEntry != NULL && e->bndEntry->bndId == bndId) {
+ eventInProgress = true;
+ break;
+ }
+ }
+ if (eventInProgress) {
+ celixThreadCondition_timedwaitRelative(&fw->dispatcher.cond, &fw->dispatcher.mutex, 5, 0);
+ }
+ }
+ celixThreadMutex_unlock(&fw->dispatcher.mutex);
+}
+
+
void celix_framework_setLogCallback(celix_framework_t* fw, void* logHandle, void (*logFunction)(void* handle, celix_log_level_e level, const char* file, const char *function, int line, const char *format, va_list formatArgs)) {
celix_frameworkLogger_setLogCallback(fw->logger, logHandle, logFunction);
+}
+
+
+long celix_framework_fireGenericEvent(framework_t* fw, long eventId, long bndId, const char *eventName, void* processData, void (*processCallback)(void *data), void* doneData, void (*doneCallback)(void* doneData)) {
+ celix_framework_bundle_entry_t* bndEntry = NULL;
+ if (bndId >=0) {
+ bndEntry = fw_bundleEntry_getBundleEntryAndIncreaseUseCount(fw, bndId);
+ if (bndEntry == NULL) {
+ fw_log(fw->logger, CELIX_LOG_LEVEL_ERROR, "Cannot find bundle for id %li", bndId);
+ return -1L;
+ }
+ }
+
+ if (eventId < 0) {
+ eventId = celix_framework_nextEventId(fw);
+ }
+
+ celix_framework_event_t event;
+ memset(&event, 0, sizeof(event));
+ event.type = CELIX_GENERIC_EVENT;
+ event.bndEntry = bndEntry;
+ event.genericEventId = eventId;
+ event.genericEventName = eventName;
+ event.genericProcessData = processData;
+ event.genericProcess = processCallback;
+ event.doneData = doneData;
+ event.doneCallback = doneCallback;
+ celix_framework_addToEventQueue(fw, &event);
+
+ return eventId;
+}
+
+long celix_framework_nextEventId(framework_t *fw) {
+ return __atomic_fetch_add(&fw->nextGenericEventId, 1, __ATOMIC_SEQ_CST);;
Review comment:
done
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes commented on a change in pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
pnoltes commented on a change in pull request #286:
URL: https://github.com/apache/celix/pull/286#discussion_r507161044
##########
File path: bundles/pubsub/pubsub_spi/src/pubsub_endpoint.c
##########
@@ -170,8 +170,9 @@ celix_properties_t* pubsubEndpoint_createFromPublisherTrackerInfo(bundle_context
data.topic = topic;
celix_bundleContext_useBundle(ctx, bundleId, &data, retrieveTopicProperties);
+ pubsubEndpoint_setFields(ep, fwUUID, scope, topic, PUBSUB_PUBLISHER_ENDPOINT_TYPE, NULL, NULL, NULL, data.props);
+
if (data.props != NULL) {
Review comment:
done
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] codecov-io edited a comment on pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #286:
URL: https://github.com/apache/celix/pull/286#issuecomment-711176467
# [Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=h1) Report
> Merging [#286](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=desc) (10b5234) into [master](https://codecov.io/gh/apache/celix/commit/dd4dca1d52c36a794dfac71867f0abc3fd81241c?el=desc) (dd4dca1) will **increase** coverage by `1.03%`.
> The diff coverage is `82.46%`.
[](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #286 +/- ##
==========================================
+ Coverage 68.17% 69.20% +1.03%
==========================================
Files 148 149 +1
Lines 30088 31623 +1535
==========================================
+ Hits 20511 21884 +1373
- Misses 9577 9739 +162
```
| [Impacted Files](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [libs/framework/src/celix\_framework\_factory.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2NlbGl4X2ZyYW1ld29ya19mYWN0b3J5LmM=) | `87.50% <ø> (-0.74%)` | :arrow_down: |
| [bundles/pubsub/pubsub\_utils/src/pubsub\_matching.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3V0aWxzL3NyYy9wdWJzdWJfbWF0Y2hpbmcuYw==) | `78.62% <50.00%> (+3.29%)` | :arrow_up: |
| [bundles/http\_admin/http\_admin/src/http\_admin.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9odHRwX2FkbWluL2h0dHBfYWRtaW4vc3JjL2h0dHBfYWRtaW4uYw==) | `68.04% <66.66%> (+2.63%)` | :arrow_up: |
| [libs/framework/src/bundle.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2J1bmRsZS5j) | `68.31% <76.92%> (+0.08%)` | :arrow_up: |
| [libs/framework/src/service\_tracker.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfdHJhY2tlci5j) | `81.98% <78.82%> (+1.12%)` | :arrow_up: |
| [libs/framework/src/bundle\_context.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2J1bmRsZV9jb250ZXh0LmM=) | `70.35% <79.31%> (+0.23%)` | :arrow_up: |
| [libs/framework/src/framework.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2ZyYW1ld29yay5j) | `74.66% <83.54%> (-0.88%)` | :arrow_down: |
| [...dles/pubsub/pubsub\_spi/src/pubsub\_endpoint\_match.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3NwaS9zcmMvcHVic3ViX2VuZHBvaW50X21hdGNoLmM=) | `56.37% <87.50%> (-2.23%)` | :arrow_down: |
| [libs/framework/src/service\_registry.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfcmVnaXN0cnkuYw==) | `73.87% <89.28%> (+2.17%)` | :arrow_up: |
| [libs/utils/src/utils.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy91dGlscy9zcmMvdXRpbHMuYw==) | `87.39% <90.90%> (+4.66%)` | :arrow_up: |
| ... and [38 more](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=footer). Last update [dd4dca1...10b5234](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes commented on a change in pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
pnoltes commented on a change in pull request #286:
URL: https://github.com/apache/celix/pull/286#discussion_r507164570
##########
File path: libs/framework/src/framework.c
##########
@@ -2015,67 +1934,112 @@ static void fw_handleEventRequest(celix_framework_t *framework, request_t* reque
for (int i = 0; i < celix_arrayList_size(localListeners); ++i) {
fw_bundle_listener_pt listener = arrayList_get(localListeners, i);
- bundle_event_t event;
- memset(&event, 0, sizeof(event));
- event.bnd = request->bndEntry->bnd;
- event.type = request->eventType;
- fw_invokeBundleListener(framework, listener->listener, &event, listener->bundle);
+ bundle_event_t bEvent;
+ memset(&bEvent, 0, sizeof(bEvent));
+ bEvent.bnd = event->bndEntry->bnd;
+ bEvent.type = event->bundleEvent;
+ fw_invokeBundleListener(framework, listener->listener, &bEvent, listener->bundle);
fw_bundleListener_decreaseUseCount(listener);
}
celix_arrayList_destroy(localListeners);
- } else if (request->type == FRAMEWORK_EVENT_TYPE) {
+ } else if (event->type == CELIX_FRAMEWORK_EVENT_TYPE) {
+ //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Handling fw event");
celixThreadMutex_lock(&framework->frameworkListenersLock);
- //TODO refactor use of framework listeners to use a useCount + conditition.
for (int i = 0; i < celix_arrayList_size(framework->frameworkListeners); ++i) {
fw_framework_listener_pt listener = celix_arrayList_get(framework->frameworkListeners, i);
- framework_event_t event;
- memset(&event, 0, sizeof(event));
- event.type = request->eventType;
- event.error = request->error;
- event.errorCode = request->errorCode;
+ framework_event_t fEvent;
+ memset(&fEvent, 0, sizeof(fEvent));
+ fEvent.type = event->fwEvent;
+ fEvent.error = event->error;
+ fEvent.errorCode = event->errorCode;
- fw_invokeFrameworkListener(framework, listener->listener, &event, listener->bundle);
+ fw_invokeFrameworkListener(framework, listener->listener, &fEvent, listener->bundle);
}
celixThreadMutex_unlock(&framework->frameworkListenersLock);
+ } else if (event->type == CELIX_REGISTER_SERVICE_EVENT) {
+ //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Handling fw service registration event for service '%s' with svc id %li", event->serviceName, event->registerServiceId);
+ service_registration_t* reg = NULL;
+ celix_status_t status;
+ if (event->factory != NULL) {
+ status = celix_serviceRegistry_registerServiceFactory(framework->registry, event->bndEntry->bnd, event->serviceName, event->factory, event->properties, event->registerServiceId, ®);
+ } else {
+ status = celix_serviceRegistry_registerService(framework->registry, event->bndEntry->bnd, event->serviceName, event->svc, event->properties, event->registerServiceId, ®);
+ }
+ if (status != CELIX_SUCCESS) {
+ fw_log(framework->logger, CELIX_LOG_LEVEL_ERROR, "Could not register service async. svc name is %s, error is %s", event->serviceName, celix_strerror(status));
+ } else if (event->registerCallback != NULL) {
+ event->registerCallback(event->registerData, serviceRegistration_getServiceId(reg));
+ }
+ } else if (event->type == CELIX_UNREGISTER_SERVICE_EVENT) {
+ //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Handling fw service unregister event for service id %li", event->unregisterServiceId);
+ celix_serviceRegistry_unregisterService(framework->registry, event->bndEntry->bnd, event->unregisterServiceId);
+ } else if (event->type == CELIX_GENERIC_EVENT) {
+ //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Handling event %s", event->eventName);
+ if (event->genericProcess != NULL) {
+ event->genericProcess(event->genericProcessData);
+ }
+ }
+
+ if (event->doneCallback != NULL) {
+ event->doneCallback(event->doneData);
}
}
-static inline void fw_handleEvents(celix_framework_t* framework, celix_array_list_t* localRequests) {
+static inline celix_framework_event_t* fw_topEventFromQueue(celix_framework_t* fw) {
+ celix_framework_event_t* e = NULL;
+ celixThreadMutex_lock(&fw->dispatcher.mutex);
+ if (fw->dispatcher.eventQueueSize > 0) {
+ e = &fw->dispatcher.eventQueue[fw->dispatcher.eventQueueFirstEntry];
+ } else if (celix_arrayList_size(fw->dispatcher.dynamicEventQueue) > 0) {
+ e = celix_arrayList_get(fw->dispatcher.dynamicEventQueue, 0);
+ }
+ celixThreadMutex_unlock(&fw->dispatcher.mutex);
+ return e;
+}
+
+static inline bool fw_removeTopEventFromQueue(celix_framework_t* fw) {
+ bool dynamicallyAllocated = false;
+ celixThreadMutex_lock(&fw->dispatcher.mutex);
+ if (fw->dispatcher.eventQueueSize > 0) {
+ fw->dispatcher.eventQueueFirstEntry = (fw->dispatcher.eventQueueFirstEntry+1) % CELIX_FRAMEWORK_STATIC_EVENT_QUEUE_SIZE;
+ fw->dispatcher.eventQueueSize -= 1;
Review comment:
The dynamic event queue will be empty before continuing on the ring buffer, but I agree it is better to increase the ring buffer size instead.
This is a simpler approach.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] codecov-io edited a comment on pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #286:
URL: https://github.com/apache/celix/pull/286#issuecomment-711176467
# [Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=h1) Report
> Merging [#286](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=desc) (c9600a4) into [master](https://codecov.io/gh/apache/celix/commit/7a26aea42fd33849e54a33d69969e42d096b9dcd?el=desc) (7a26aea) will **increase** coverage by `0.58%`.
> The diff coverage is `81.45%`.
[](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #286 +/- ##
==========================================
+ Coverage 66.69% 67.27% +0.58%
==========================================
Files 147 147
Lines 29947 30412 +465
==========================================
+ Hits 19974 20461 +487
+ Misses 9973 9951 -22
```
| [Impacted Files](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [libs/framework/src/celix\_framework\_factory.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2NlbGl4X2ZyYW1ld29ya19mYWN0b3J5LmM=) | `87.50% <ø> (-0.74%)` | :arrow_down: |
| [bundles/pubsub/pubsub\_utils/src/pubsub\_matching.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3V0aWxzL3NyYy9wdWJzdWJfbWF0Y2hpbmcuYw==) | `78.62% <50.00%> (+3.29%)` | :arrow_up: |
| [bundles/http\_admin/http\_admin/src/http\_admin.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9odHRwX2FkbWluL2h0dHBfYWRtaW4vc3JjL2h0dHBfYWRtaW4uYw==) | `68.04% <66.66%> (+2.63%)` | :arrow_up: |
| [libs/framework/src/service\_tracker.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfdHJhY2tlci5j) | `79.50% <76.47%> (+2.36%)` | :arrow_up: |
| [libs/framework/src/bundle.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2J1bmRsZS5j) | `68.31% <76.92%> (+0.08%)` | :arrow_up: |
| [libs/framework/src/bundle\_context.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2J1bmRsZV9jb250ZXh0LmM=) | `69.93% <78.48%> (-0.19%)` | :arrow_down: |
| [libs/framework/src/framework.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2ZyYW1ld29yay5j) | `74.66% <83.54%> (-0.66%)` | :arrow_down: |
| [libs/framework/src/service\_registry.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfcmVnaXN0cnkuYw==) | `73.35% <86.90%> (+1.66%)` | :arrow_up: |
| [...dles/pubsub/pubsub\_spi/src/pubsub\_endpoint\_match.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3NwaS9zcmMvcHVic3ViX2VuZHBvaW50X21hdGNoLmM=) | `56.37% <87.50%> (-2.23%)` | :arrow_down: |
| [libs/utils/src/utils.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy91dGlscy9zcmMvdXRpbHMuYw==) | `87.39% <90.90%> (+4.66%)` | :arrow_up: |
| ... and [28 more](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=footer). Last update [7a26aea...c9600a4](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes commented on a change in pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
pnoltes commented on a change in pull request #286:
URL: https://github.com/apache/celix/pull/286#discussion_r507163266
##########
File path: libs/framework/include/celix_framework.h
##########
@@ -156,6 +156,12 @@ void celix_framework_waitForEmptyEventQueue(celix_framework_t *fw);
void celix_framework_setLogCallback(celix_framework_t* fw, void* logHandle, void (*logFunction)(void* handle, celix_log_level_e level, const char* file, const char *function, int line, const char *format, va_list formatArgs));
+/**
+ * wait till all events for the bundle identified by the bndId are processed.
+ */
+void celix_framework_waitForEvents(celix_framework_t* fw, long bndId);
Review comment:
updated
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] codecov-io edited a comment on pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #286:
URL: https://github.com/apache/celix/pull/286#issuecomment-711176467
# [Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=h1) Report
> Merging [#286](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=desc) into [master](https://codecov.io/gh/apache/celix/commit/20f794bd1dece8c986119553bc97205bee09cca8?el=desc) will **increase** coverage by `0.29%`.
> The diff coverage is `77.80%`.
[](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #286 +/- ##
==========================================
+ Coverage 68.30% 68.59% +0.29%
==========================================
Files 136 136
Lines 27371 27777 +406
==========================================
+ Hits 18697 19055 +358
- Misses 8674 8722 +48
```
| [Impacted Files](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [libs/framework/src/celix\_framework\_factory.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2NlbGl4X2ZyYW1ld29ya19mYWN0b3J5LmM=) | `87.50% <ø> (-0.74%)` | :arrow_down: |
| [bundles/http\_admin/http\_admin/src/http\_admin.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9odHRwX2FkbWluL2h0dHBfYWRtaW4vc3JjL2h0dHBfYWRtaW4uYw==) | `68.04% <66.66%> (+2.63%)` | :arrow_up: |
| [libs/framework/src/bundle\_context.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2J1bmRsZV9jb250ZXh0LmM=) | `66.51% <72.51%> (-3.61%)` | :arrow_down: |
| [libs/framework/src/service\_tracker.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfdHJhY2tlci5j) | `79.50% <76.47%> (+2.18%)` | :arrow_up: |
| [libs/framework/src/bundle.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2J1bmRsZS5j) | `68.31% <76.92%> (+0.08%)` | :arrow_up: |
| [libs/framework/src/framework.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2ZyYW1ld29yay5j) | `73.38% <78.53%> (-1.93%)` | :arrow_down: |
| [libs/framework/src/service\_registry.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfcmVnaXN0cnkuYw==) | `73.32% <86.82%> (+1.70%)` | :arrow_up: |
| [bundles/logging/log\_admin/src/celix\_log\_admin.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9sb2dnaW5nL2xvZ19hZG1pbi9zcmMvY2VsaXhfbG9nX2FkbWluLmM=) | `99.73% <100.00%> (-0.01%)` | :arrow_down: |
| [bundles/pubsub/pubsub\_spi/src/pubsub\_endpoint.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3NwaS9zcmMvcHVic3ViX2VuZHBvaW50LmM=) | `72.11% <100.00%> (-3.13%)` | :arrow_down: |
| [...gy\_manager/tms\_tst/disc\_mock/disc\_mock\_activator.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9yZW1vdGVfc2VydmljZXMvdG9wb2xvZ3lfbWFuYWdlci90bXNfdHN0L2Rpc2NfbW9jay9kaXNjX21vY2tfYWN0aXZhdG9yLmM=) | `91.30% <100.00%> (+0.12%)` | :arrow_up: |
| ... and [20 more](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=footer). Last update [20f794b...8846681](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes commented on a change in pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
pnoltes commented on a change in pull request #286:
URL: https://github.com/apache/celix/pull/286#discussion_r507164062
##########
File path: libs/framework/src/framework.c
##########
@@ -1922,88 +1861,68 @@ bundle_context_t* framework_getContext(const_framework_pt framework) {
return result;
}
-celix_status_t fw_fireBundleEvent(framework_pt framework, bundle_event_type_e eventType, celix_framework_bundle_entry_t* entry) {
- celix_status_t status = CELIX_SUCCESS;
-
+void fw_fireBundleEvent(framework_pt framework, bundle_event_type_e eventType, celix_framework_bundle_entry_t* entry) {
if (eventType == OSGI_FRAMEWORK_BUNDLE_EVENT_STOPPING || eventType == OSGI_FRAMEWORK_BUNDLE_EVENT_UNINSTALLED || eventType == OSGI_FRAMEWORK_BUNDLE_EVENT_STOPPED) {
if (entry->bndId == framework->bundleId) {
//NOTE for framework bundle not triggering events while framework is stopped (and as result in use)
- return CELIX_SUCCESS;
+ return;
}
}
- request_t* request = (request_t*) calloc(1, sizeof(*request));
- if (!request) {
- status = CELIX_ENOMEM;
- } else {
- fw_bundleEntry_increaseUseCount(entry);
+ fw_bundleEntry_increaseUseCount(entry);
- request->eventType = eventType;
- request->filter = NULL;
- request->type = BUNDLE_EVENT_TYPE;
- request->error = NULL;
- request->bndEntry = entry;
+ celix_framework_event_t event;
+ memset(&event, 0, sizeof(event));
+ event.type = CELIX_BUNDLE_EVENT_TYPE;
+ event.bndEntry = entry;
+ event.bundleEvent = eventType;
+ celix_framework_addToEventQueue(framework, &event);
+}
- celixThreadMutex_lock(&framework->dispatcher.mutex);
- if (framework->dispatcher.active) {
- //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Adding dispatcher bundle event request for bnd id %li with event type %i", entry->bndId, eventType);
- celix_arrayList_add(framework->dispatcher.requests, request);
- celixThreadCondition_broadcast(&framework->dispatcher.cond);
- } else {
- /*
- * NOTE because stopping the framework is done through stopping the framework bundle,
- * most bundle stopping / stopped events cannot be fired.
- *
- * TBD if this needs to addressed.
- */
- fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Cannot fire event dispatcher not active. Event is %x for bundle %s", eventType, celix_bundle_getSymbolicName(entry->bnd));
- fw_bundleEntry_decreaseUseCount(entry);
- free(request);
- }
- celixThreadMutex_unlock(&framework->dispatcher.mutex);
+void fw_fireFrameworkEvent(framework_pt framework, framework_event_type_e eventType, celix_status_t errorCode) {
+ celix_framework_event_t event;
+ memset(&event, 0, sizeof(event));
+ event.type = CELIX_FRAMEWORK_EVENT_TYPE;
+ event.fwEvent = eventType;
+ event.errorCode = errorCode;
+ event.error = "";
+ if (errorCode != CELIX_SUCCESS) {
+ event.error = celix_strerror(errorCode);
}
- framework_logIfError(framework->logger, status, NULL, "Failed to fire bundle event");
-
- return status;
+ celix_framework_addToEventQueue(framework, &event);
}
-celix_status_t fw_fireFrameworkEvent(framework_pt framework, framework_event_type_e eventType, celix_status_t errorCode) {
- celix_status_t status = CELIX_SUCCESS;
-
- request_t* request = calloc(1, sizeof(*request));
- if (!request) {
- status = CELIX_ENOMEM;
+static void celix_framework_addToEventQueue(celix_framework_t *fw, const celix_framework_event_t* event) {
+ celixThreadMutex_lock(&fw->dispatcher.mutex);
+ //try to add to static queue
+ if (celix_arrayList_size(fw->dispatcher.dynamicEventQueue) > 0) { //always to dynamic queue if not empty (to ensure order)
+ celix_framework_event_t *e = malloc(sizeof(*e));
+ *e = *event; //shallow copy
+ celix_arrayList_add(fw->dispatcher.dynamicEventQueue, e);
+ if (celix_arrayList_size(fw->dispatcher.dynamicEventQueue) % 100 == 0) {
+ fw_log(fw->logger, CELIX_LOG_LEVEL_WARNING, "dynamic event queue size is %i. Is there a bundle blocking on the event loop thread?", celix_arrayList_size(fw->dispatcher.dynamicEventQueue));
+ }
+ } else if (fw->dispatcher.eventQueueSize < CELIX_FRAMEWORK_STATIC_EVENT_QUEUE_SIZE) {
+ size_t index = (fw->dispatcher.eventQueueFirstEntry + fw->dispatcher.eventQueueSize) %
+ CELIX_FRAMEWORK_STATIC_EVENT_QUEUE_SIZE;
+ fw->dispatcher.eventQueue[index] = *event; //shallow copy
+ fw->dispatcher.eventQueueSize += 1;
} else {
- request->eventType = eventType;
- request->filter = NULL;
- request->type = FRAMEWORK_EVENT_TYPE;
- request->errorCode = errorCode;
- request->error = "";
-
- if (errorCode != CELIX_SUCCESS) {
- request->error = celix_strerror(errorCode);
- }
-
- celixThreadMutex_lock(&framework->dispatcher.mutex);
- if (framework->dispatcher.active) {
- //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Adding dispatcher framework event request for event type %i", eventType);
- celix_arrayList_add(framework->dispatcher.requests, request);
- celixThreadCondition_broadcast(&framework->dispatcher.cond);
- } else {
- free(request);
- }
- celixThreadMutex_unlock(&framework->dispatcher.mutex);
- }
-
- framework_logIfError(framework->logger, status, NULL, "Failed to fire framework event");
-
- return status;
+ //static queue is full, dynamics queue is empty. Add first entry to dynamic queue
+ fw_log(fw->logger, CELIX_LOG_LEVEL_WARNING,
+ "Static event queue for celix framework is full, falling back to dynamic allocated events. Increase static event queue size, current size is %i", CELIX_FRAMEWORK_STATIC_EVENT_QUEUE_SIZE);
+ celix_framework_event_t *e = malloc(sizeof(*e));
+ *e = *event; //shallow copy
+ celix_arrayList_add(fw->dispatcher.dynamicEventQueue, e);
+ }
+ celixThreadCondition_broadcast(&fw->dispatcher.cond);
+ celixThreadMutex_unlock(&fw->dispatcher.mutex);
}
-
-static void fw_handleEventRequest(celix_framework_t *framework, request_t* request) {
- if (request->type == BUNDLE_EVENT_TYPE) {
+static void fw_handleEventRequest(celix_framework_t *framework, celix_framework_event_t* event) {
+ if (event->type == CELIX_BUNDLE_EVENT_TYPE) {
+ //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Handling fw bundle event for bundle %s", event->bndEntry->bnd->symbolicName);
Review comment:
removed
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes commented on a change in pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
pnoltes commented on a change in pull request #286:
URL: https://github.com/apache/celix/pull/286#discussion_r545894180
##########
File path: libs/framework/src/bundle_context.c
##########
@@ -461,52 +505,117 @@ long celix_bundleContext_registerServiceWithOptions(bundle_context_t *ctx, const
}
const char *lang = opts->serviceLanguage != NULL && strncmp("", opts->serviceLanguage, 1) != 0 ? opts->serviceLanguage : CELIX_FRAMEWORK_SERVICE_C_LANGUAGE;
celix_properties_set(props, CELIX_FRAMEWORK_SERVICE_LANGUAGE, lang);
- if (opts->serviceName != NULL && strncmp("", opts->serviceName, 1) != 0) {
- if (opts->factory != NULL) {
- reg = celix_framework_registerServiceFactory(ctx->framework, ctx->bundle, opts->serviceName, opts->factory, props);
- } else {
- bundleContext_registerService(ctx, opts->serviceName, opts->svc, props, ®);
- }
- svcId = serviceRegistration_getServiceId(reg); //save to call with NULL
+
+ long svcId = -1;
+ if (!async && celix_framework_isCurrentThreadTheEventLoop(ctx->framework)) {
+ /*
+ * Note already on event loop, cannot register the service async, because we cannot wait a future event (the
+ * service registration) the event loop.
+ *
+ * So in this case we handle the service registration the "traditional way" and call the sync fw service
+ * registrations versions on the event loop thread
+ */
+
+ svcId = celix_framework_registerService(ctx->framework, ctx->bundle, opts->serviceName, opts->svc, opts->factory, props);
} else {
- framework_logIfError(ctx->framework->logger, CELIX_ILLEGAL_ARGUMENT, NULL, "Required serviceName argument is NULL");
+ svcId = celix_framework_registerServiceAsync(ctx->framework, ctx->bundle, opts->serviceName, opts->svc, opts->factory, props, opts->asyncData, opts->asyncCallback, NULL, NULL);
+ if (!async && svcId >= 0) {
+ //note on event loop thread, but in a sync call, so waiting till service registration is concluded
+ celix_bundleContext_waitForAsyncRegistration(ctx, svcId);
+ }
}
+
+
if (svcId < 0) {
properties_destroy(props);
} else {
celixThreadMutex_lock(&ctx->mutex);
- arrayList_add(ctx->svcRegistrations, reg);
+ celix_arrayList_addLong(ctx->svcRegistrations, svcId);
celixThreadMutex_unlock(&ctx->mutex);
+ if (!async) {
Review comment:
Good point.
I updated this. So that the async callbacks are only called when using the async api.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] Oipo edited a comment on pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
Oipo edited a comment on pull request #286:
URL: https://github.com/apache/celix/pull/286#issuecomment-753642764
FYI, I ran `floodEventLoopTest` through callgrind, and this is the result. The .txt file can be opened with kcachegrind.
[callgrind.out.90753.txt](https://github.com/apache/celix/files/5761879/callgrind.out.90753.txt)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes commented on a change in pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
pnoltes commented on a change in pull request #286:
URL: https://github.com/apache/celix/pull/286#discussion_r520077829
##########
File path: libs/framework/src/framework.c
##########
@@ -2015,67 +1934,112 @@ static void fw_handleEventRequest(celix_framework_t *framework, request_t* reque
for (int i = 0; i < celix_arrayList_size(localListeners); ++i) {
fw_bundle_listener_pt listener = arrayList_get(localListeners, i);
- bundle_event_t event;
- memset(&event, 0, sizeof(event));
- event.bnd = request->bndEntry->bnd;
- event.type = request->eventType;
- fw_invokeBundleListener(framework, listener->listener, &event, listener->bundle);
+ bundle_event_t bEvent;
+ memset(&bEvent, 0, sizeof(bEvent));
+ bEvent.bnd = event->bndEntry->bnd;
+ bEvent.type = event->bundleEvent;
+ fw_invokeBundleListener(framework, listener->listener, &bEvent, listener->bundle);
fw_bundleListener_decreaseUseCount(listener);
}
celix_arrayList_destroy(localListeners);
- } else if (request->type == FRAMEWORK_EVENT_TYPE) {
+ } else if (event->type == CELIX_FRAMEWORK_EVENT_TYPE) {
+ //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Handling fw event");
celixThreadMutex_lock(&framework->frameworkListenersLock);
- //TODO refactor use of framework listeners to use a useCount + conditition.
for (int i = 0; i < celix_arrayList_size(framework->frameworkListeners); ++i) {
fw_framework_listener_pt listener = celix_arrayList_get(framework->frameworkListeners, i);
- framework_event_t event;
- memset(&event, 0, sizeof(event));
- event.type = request->eventType;
- event.error = request->error;
- event.errorCode = request->errorCode;
+ framework_event_t fEvent;
+ memset(&fEvent, 0, sizeof(fEvent));
+ fEvent.type = event->fwEvent;
+ fEvent.error = event->error;
+ fEvent.errorCode = event->errorCode;
- fw_invokeFrameworkListener(framework, listener->listener, &event, listener->bundle);
+ fw_invokeFrameworkListener(framework, listener->listener, &fEvent, listener->bundle);
}
celixThreadMutex_unlock(&framework->frameworkListenersLock);
+ } else if (event->type == CELIX_REGISTER_SERVICE_EVENT) {
+ //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Handling fw service registration event for service '%s' with svc id %li", event->serviceName, event->registerServiceId);
+ service_registration_t* reg = NULL;
+ celix_status_t status;
+ if (event->factory != NULL) {
+ status = celix_serviceRegistry_registerServiceFactory(framework->registry, event->bndEntry->bnd, event->serviceName, event->factory, event->properties, event->registerServiceId, ®);
+ } else {
+ status = celix_serviceRegistry_registerService(framework->registry, event->bndEntry->bnd, event->serviceName, event->svc, event->properties, event->registerServiceId, ®);
+ }
+ if (status != CELIX_SUCCESS) {
+ fw_log(framework->logger, CELIX_LOG_LEVEL_ERROR, "Could not register service async. svc name is %s, error is %s", event->serviceName, celix_strerror(status));
+ } else if (event->registerCallback != NULL) {
+ event->registerCallback(event->registerData, serviceRegistration_getServiceId(reg));
+ }
+ } else if (event->type == CELIX_UNREGISTER_SERVICE_EVENT) {
+ //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Handling fw service unregister event for service id %li", event->unregisterServiceId);
+ celix_serviceRegistry_unregisterService(framework->registry, event->bndEntry->bnd, event->unregisterServiceId);
+ } else if (event->type == CELIX_GENERIC_EVENT) {
+ //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Handling event %s", event->eventName);
+ if (event->genericProcess != NULL) {
+ event->genericProcess(event->genericProcessData);
+ }
+ }
+
+ if (event->doneCallback != NULL) {
+ event->doneCallback(event->doneData);
}
}
-static inline void fw_handleEvents(celix_framework_t* framework, celix_array_list_t* localRequests) {
+static inline celix_framework_event_t* fw_topEventFromQueue(celix_framework_t* fw) {
+ celix_framework_event_t* e = NULL;
+ celixThreadMutex_lock(&fw->dispatcher.mutex);
+ if (fw->dispatcher.eventQueueSize > 0) {
+ e = &fw->dispatcher.eventQueue[fw->dispatcher.eventQueueFirstEntry];
+ } else if (celix_arrayList_size(fw->dispatcher.dynamicEventQueue) > 0) {
+ e = celix_arrayList_get(fw->dispatcher.dynamicEventQueue, 0);
+ }
+ celixThreadMutex_unlock(&fw->dispatcher.mutex);
+ return e;
+}
+
+static inline bool fw_removeTopEventFromQueue(celix_framework_t* fw) {
+ bool dynamicallyAllocated = false;
+ celixThreadMutex_lock(&fw->dispatcher.mutex);
+ if (fw->dispatcher.eventQueueSize > 0) {
+ fw->dispatcher.eventQueueFirstEntry = (fw->dispatcher.eventQueueFirstEntry+1) % CELIX_FRAMEWORK_STATIC_EVENT_QUEUE_SIZE;
+ fw->dispatcher.eventQueueSize -= 1;
Review comment:
the event queue is tested indirectly through service registration, tracking, etc.
Flooding the event queue so that the increasing of the event queue is tested is done at
bundle_context_services_test.cpp / floodEventLoopTest
Also added an explicit test for firing a generic event
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] codecov-io edited a comment on pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #286:
URL: https://github.com/apache/celix/pull/286#issuecomment-711176467
# [Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=h1) Report
> Merging [#286](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=desc) (6ebe5b9) into [master](https://codecov.io/gh/apache/celix/commit/dd4dca1d52c36a794dfac71867f0abc3fd81241c?el=desc) (dd4dca1) will **increase** coverage by `0.47%`.
> The diff coverage is `82.42%`.
[](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #286 +/- ##
==========================================
+ Coverage 68.17% 68.64% +0.47%
==========================================
Files 148 148
Lines 30088 30556 +468
==========================================
+ Hits 20511 20974 +463
- Misses 9577 9582 +5
```
| [Impacted Files](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [libs/framework/src/celix\_framework\_factory.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2NlbGl4X2ZyYW1ld29ya19mYWN0b3J5LmM=) | `87.50% <ø> (-0.74%)` | :arrow_down: |
| [bundles/pubsub/pubsub\_utils/src/pubsub\_matching.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3V0aWxzL3NyYy9wdWJzdWJfbWF0Y2hpbmcuYw==) | `78.62% <50.00%> (+3.29%)` | :arrow_up: |
| [bundles/http\_admin/http\_admin/src/http\_admin.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9odHRwX2FkbWluL2h0dHBfYWRtaW4vc3JjL2h0dHBfYWRtaW4uYw==) | `68.04% <66.66%> (+2.63%)` | :arrow_up: |
| [libs/framework/src/bundle.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2J1bmRsZS5j) | `68.31% <76.92%> (+0.08%)` | :arrow_up: |
| [libs/framework/src/service\_tracker.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfdHJhY2tlci5j) | `81.98% <78.82%> (+1.12%)` | :arrow_up: |
| [libs/framework/src/bundle\_context.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2J1bmRsZV9jb250ZXh0LmM=) | `70.35% <79.31%> (+0.23%)` | :arrow_up: |
| [libs/framework/src/framework.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2ZyYW1ld29yay5j) | `74.66% <83.54%> (-0.88%)` | :arrow_down: |
| [...dles/pubsub/pubsub\_spi/src/pubsub\_endpoint\_match.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3NwaS9zcmMvcHVic3ViX2VuZHBvaW50X21hdGNoLmM=) | `56.37% <87.50%> (-2.23%)` | :arrow_down: |
| [libs/framework/src/service\_registry.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfcmVnaXN0cnkuYw==) | `73.87% <89.28%> (+2.17%)` | :arrow_up: |
| [libs/utils/src/utils.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy91dGlscy9zcmMvdXRpbHMuYw==) | `87.39% <90.90%> (+4.66%)` | :arrow_up: |
| ... and [26 more](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=footer). Last update [dd4dca1...6ebe5b9](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] codecov-io commented on pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #286:
URL: https://github.com/apache/celix/pull/286#issuecomment-711176467
# [Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=h1) Report
> Merging [#286](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=desc) into [master](https://codecov.io/gh/apache/celix/commit/20f794bd1dece8c986119553bc97205bee09cca8?el=desc) will **increase** coverage by `0.26%`.
> The diff coverage is `77.80%`.
[](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #286 +/- ##
==========================================
+ Coverage 68.30% 68.57% +0.26%
==========================================
Files 136 136
Lines 27371 27777 +406
==========================================
+ Hits 18697 19047 +350
- Misses 8674 8730 +56
```
| [Impacted Files](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [libs/framework/src/celix\_framework\_factory.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2NlbGl4X2ZyYW1ld29ya19mYWN0b3J5LmM=) | `87.50% <ø> (-0.74%)` | :arrow_down: |
| [bundles/http\_admin/http\_admin/src/http\_admin.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9odHRwX2FkbWluL2h0dHBfYWRtaW4vc3JjL2h0dHBfYWRtaW4uYw==) | `68.04% <66.66%> (+2.63%)` | :arrow_up: |
| [libs/framework/src/bundle\_context.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2J1bmRsZV9jb250ZXh0LmM=) | `66.51% <72.51%> (-3.61%)` | :arrow_down: |
| [libs/framework/src/service\_tracker.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfdHJhY2tlci5j) | `79.50% <76.47%> (+2.18%)` | :arrow_up: |
| [libs/framework/src/bundle.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2J1bmRsZS5j) | `68.31% <76.92%> (+0.08%)` | :arrow_up: |
| [libs/framework/src/framework.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL2ZyYW1ld29yay5j) | `73.38% <78.53%> (-1.93%)` | :arrow_down: |
| [libs/framework/src/service\_registry.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-bGlicy9mcmFtZXdvcmsvc3JjL3NlcnZpY2VfcmVnaXN0cnkuYw==) | `73.32% <86.82%> (+1.70%)` | :arrow_up: |
| [bundles/logging/log\_admin/src/celix\_log\_admin.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9sb2dnaW5nL2xvZ19hZG1pbi9zcmMvY2VsaXhfbG9nX2FkbWluLmM=) | `99.73% <100.00%> (-0.01%)` | :arrow_down: |
| [bundles/pubsub/pubsub\_spi/src/pubsub\_endpoint.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9wdWJzdWIvcHVic3ViX3NwaS9zcmMvcHVic3ViX2VuZHBvaW50LmM=) | `72.11% <100.00%> (-3.13%)` | :arrow_down: |
| [...gy\_manager/tms\_tst/disc\_mock/disc\_mock\_activator.c](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree#diff-YnVuZGxlcy9yZW1vdGVfc2VydmljZXMvdG9wb2xvZ3lfbWFuYWdlci90bXNfdHN0L2Rpc2NfbW9jay9kaXNjX21vY2tfYWN0aXZhdG9yLmM=) | `91.30% <100.00%> (+0.12%)` | :arrow_up: |
| ... and [19 more](https://codecov.io/gh/apache/celix/pull/286/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=footer). Last update [20f794b...8846681](https://codecov.io/gh/apache/celix/pull/286?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes commented on a change in pull request #286: Feature/async svc registration
Posted by GitBox <gi...@apache.org>.
pnoltes commented on a change in pull request #286:
URL: https://github.com/apache/celix/pull/286#discussion_r520067092
##########
File path: libs/framework/src/framework.c
##########
@@ -2015,67 +1934,112 @@ static void fw_handleEventRequest(celix_framework_t *framework, request_t* reque
for (int i = 0; i < celix_arrayList_size(localListeners); ++i) {
fw_bundle_listener_pt listener = arrayList_get(localListeners, i);
- bundle_event_t event;
- memset(&event, 0, sizeof(event));
- event.bnd = request->bndEntry->bnd;
- event.type = request->eventType;
- fw_invokeBundleListener(framework, listener->listener, &event, listener->bundle);
+ bundle_event_t bEvent;
+ memset(&bEvent, 0, sizeof(bEvent));
+ bEvent.bnd = event->bndEntry->bnd;
+ bEvent.type = event->bundleEvent;
+ fw_invokeBundleListener(framework, listener->listener, &bEvent, listener->bundle);
fw_bundleListener_decreaseUseCount(listener);
}
celix_arrayList_destroy(localListeners);
- } else if (request->type == FRAMEWORK_EVENT_TYPE) {
+ } else if (event->type == CELIX_FRAMEWORK_EVENT_TYPE) {
+ //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Handling fw event");
celixThreadMutex_lock(&framework->frameworkListenersLock);
- //TODO refactor use of framework listeners to use a useCount + conditition.
for (int i = 0; i < celix_arrayList_size(framework->frameworkListeners); ++i) {
fw_framework_listener_pt listener = celix_arrayList_get(framework->frameworkListeners, i);
- framework_event_t event;
- memset(&event, 0, sizeof(event));
- event.type = request->eventType;
- event.error = request->error;
- event.errorCode = request->errorCode;
+ framework_event_t fEvent;
+ memset(&fEvent, 0, sizeof(fEvent));
+ fEvent.type = event->fwEvent;
+ fEvent.error = event->error;
+ fEvent.errorCode = event->errorCode;
- fw_invokeFrameworkListener(framework, listener->listener, &event, listener->bundle);
+ fw_invokeFrameworkListener(framework, listener->listener, &fEvent, listener->bundle);
}
celixThreadMutex_unlock(&framework->frameworkListenersLock);
+ } else if (event->type == CELIX_REGISTER_SERVICE_EVENT) {
+ //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Handling fw service registration event for service '%s' with svc id %li", event->serviceName, event->registerServiceId);
+ service_registration_t* reg = NULL;
+ celix_status_t status;
+ if (event->factory != NULL) {
+ status = celix_serviceRegistry_registerServiceFactory(framework->registry, event->bndEntry->bnd, event->serviceName, event->factory, event->properties, event->registerServiceId, ®);
+ } else {
+ status = celix_serviceRegistry_registerService(framework->registry, event->bndEntry->bnd, event->serviceName, event->svc, event->properties, event->registerServiceId, ®);
+ }
+ if (status != CELIX_SUCCESS) {
+ fw_log(framework->logger, CELIX_LOG_LEVEL_ERROR, "Could not register service async. svc name is %s, error is %s", event->serviceName, celix_strerror(status));
+ } else if (event->registerCallback != NULL) {
+ event->registerCallback(event->registerData, serviceRegistration_getServiceId(reg));
+ }
+ } else if (event->type == CELIX_UNREGISTER_SERVICE_EVENT) {
+ //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Handling fw service unregister event for service id %li", event->unregisterServiceId);
+ celix_serviceRegistry_unregisterService(framework->registry, event->bndEntry->bnd, event->unregisterServiceId);
+ } else if (event->type == CELIX_GENERIC_EVENT) {
+ //fw_log(framework->logger, CELIX_LOG_LEVEL_TRACE, "Handling event %s", event->eventName);
+ if (event->genericProcess != NULL) {
+ event->genericProcess(event->genericProcessData);
+ }
+ }
+
+ if (event->doneCallback != NULL) {
+ event->doneCallback(event->doneData);
}
}
-static inline void fw_handleEvents(celix_framework_t* framework, celix_array_list_t* localRequests) {
+static inline celix_framework_event_t* fw_topEventFromQueue(celix_framework_t* fw) {
+ celix_framework_event_t* e = NULL;
+ celixThreadMutex_lock(&fw->dispatcher.mutex);
+ if (fw->dispatcher.eventQueueSize > 0) {
+ e = &fw->dispatcher.eventQueue[fw->dispatcher.eventQueueFirstEntry];
+ } else if (celix_arrayList_size(fw->dispatcher.dynamicEventQueue) > 0) {
+ e = celix_arrayList_get(fw->dispatcher.dynamicEventQueue, 0);
+ }
+ celixThreadMutex_unlock(&fw->dispatcher.mutex);
+ return e;
+}
+
+static inline bool fw_removeTopEventFromQueue(celix_framework_t* fw) {
+ bool dynamicallyAllocated = false;
+ celixThreadMutex_lock(&fw->dispatcher.mutex);
+ if (fw->dispatcher.eventQueueSize > 0) {
+ fw->dispatcher.eventQueueFirstEntry = (fw->dispatcher.eventQueueFirstEntry+1) % CELIX_FRAMEWORK_STATIC_EVENT_QUEUE_SIZE;
+ fw->dispatcher.eventQueueSize -= 1;
Review comment:
I agree using a existing ring buffer would be preferable.
The c-ringbuf has no license, so this is not an option.
I was hoping for something like freebsd queue (see `man queue`), but this has no ring buffer.
There is a buf_ring from freebsd, but this is not "header (marcro) only".
So for this PR I would like to keep the current ring buffer usage.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org