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:
[email protected]
