jermus67 commented on a change in pull request #317:
URL: https://github.com/apache/celix/pull/317#discussion_r576105540
##########
File path: libs/framework/src/service_tracker.c
##########
@@ -163,26 +163,56 @@ celix_status_t serviceTracker_destroy(service_tracker_pt
tracker) {
celix_status_t serviceTracker_open(service_tracker_pt tracker) {
celixThreadMutex_lock(&tracker->mutex);
- bool alreadyOpen = tracker->open;
- tracker->open = true;
+ bool addListener = false;
+ switch (tracker->state) {
+ case CELIX_SERVICE_TRACKER_CLOSED:
+ tracker->state = CELIX_SERVICE_TRACKER_OPENING;
+ addListener = true;
+ break;
+ case CELIX_SERVICE_TRACKER_CLOSING:
+ celix_bundleContext_log(tracker->context, CELIX_LOG_LEVEL_WARNING,
"Cannot open closing tracker");
+ break;
+ default:
+ //nop
+ break;
+ }
celixThreadMutex_unlock(&tracker->mutex);
- if (!alreadyOpen) {
+ if (!addListener) {
bundleContext_addServiceListener(tracker->context, &tracker->listener,
tracker->filter);
}
- return CELIX_SUCCESS;
+
+ celixThreadMutex_lock(&tracker->mutex);
+ tracker->state = CELIX_SERVICE_TRACKER_OPEN;
+ celixThreadMutex_unlock(&tracker->mutex);
+
+ //ensure that the set callback is called once the tracker is open.
+ celix_serviceTracker_useHighestRankingService(tracker,
tracker->serviceName, tracker, NULL, NULL,
serviceTracker_checkAndInvokeSetService);
+
+ return CELIX_SUCCESS;
}
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
celixThreadMutex_lock(&tracker->mutex);
- bool open = tracker->open;
- tracker->open = false;
+ bool needClosing = false;
+ switch (tracker->state) {
+ case CELIX_SERVICE_TRACKER_OPEN:
+ tracker->state = CELIX_SERVICE_TRACKER_CLOSING;
+ needClosing = true;
+ break;
+ case CELIX_SERVICE_TRACKER_OPENING:
+ celix_bundleContext_log(tracker->context, CELIX_LOG_LEVEL_WARNING,
"Cannot close opening tracker");
+ break;
+ default:
+ //nop
+ break;
+ }
celixThreadMutex_unlock(&tracker->mutex);
- if (!open) {
+ if (!needClosing) {
Review comment:
Funny, serviceTracker_close only does something when the serviceTracker
is not actually closed ... And if it does that, which apparently it was
intended to do, it returns failure?
```suggestion
if (needClosing) {
```
This way the function returns success when it has changed the state of the
serviceTracker from "open" to "closing"
I don't know right know what the receiving side does with success / failure,
but the suggestion would make more sense to me (that is why I suggested it, I
think).
----------------------------------------------------------------
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]
