[GitHub] [celix] jermus67 commented on a change in pull request #317: prevents unneeded suspend triggers in the dm component

[GitBox <gi...@apache.org>]

jermus67 commented on a change in pull request #317:
URL: https://github.com/apache/celix/pull/317#discussion_r576097011



##########
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) {

Review comment:
       Ok, so if "addListener" is false (i.e. do not add listener), we add a listener ....
   ```suggestion
       if (noServiceListenerRegistered) {
   ```
   And can we only add one service listener? I expect the "addListener" to be set to true once bundleConext_addServiceListener() has completed, so this is only done once (at least, that is what I expect).




----------------------------------------------------------------
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