PengZheng commented on code in PR #414:
URL: https://github.com/apache/celix/pull/414#discussion_r856889636


##########
bundles/remote_services/topology_manager/src/topology_manager.c:
##########
@@ -380,61 +349,64 @@ celix_status_t topologyManager_exportScopeChanged(void 
*handle, char *filterStr)
        }
 
        // add already exported services to new rsa
-       if (celixThreadMutex_lock(&manager->exportedServicesLock) == 
CELIX_SUCCESS) {
-               hash_map_iterator_pt exportedServicesIterator = 
hashMapIterator_create(manager->exportedServices);
-               int size = hashMap_size(manager->exportedServices);
-               service_reference_pt *srvRefs = (service_reference_pt *) 
calloc(size, sizeof(service_reference_pt));
-               char **srvIds = (char **) calloc(size, sizeof(char*));
-               int nrFound = 0;
-
-               found = false;
-
-               while (hashMapIterator_hasNext(exportedServicesIterator)) {
-                       hash_map_entry_pt entry = 
hashMapIterator_nextEntry(exportedServicesIterator);
-                       service_reference_pt reference = 
hashMapEntry_getKey(entry);
-                       reg = NULL;
-                       serviceReference_getServiceRegistration(reference, 
&reg);
-                       if (reg != NULL) {
-                               props = NULL;
-                               serviceRegistration_getProperties(reg, &props);
-                               status = filter_match(filter, props, &found);
-                               if (found) {
-                                       srvRefs[nrFound] = reference;
-                                       serviceReference_getProperty(reference, 
OSGI_FRAMEWORK_SERVICE_ID, &serviceId);
-                                       srvIds[nrFound++] = (char*)serviceId;
-                               }
+       celixThreadMutex_lock(&manager->lock);
+
+       hash_map_iterator_pt exportedServicesIterator = 
hashMapIterator_create(manager->exportedServices);
+       int size = hashMap_size(manager->exportedServices);
+       service_reference_pt *srvRefs = (service_reference_pt *) calloc(size, 
sizeof(service_reference_pt));
+       char **srvIds = (char **) calloc(size, sizeof(char*));
+       int nrFound = 0;
+
+       found = false;
+
+       while (hashMapIterator_hasNext(exportedServicesIterator)) {
+               hash_map_entry_pt entry = 
hashMapIterator_nextEntry(exportedServicesIterator);
+               service_reference_pt reference = hashMapEntry_getKey(entry);
+               reg = NULL;
+               serviceReference_getServiceRegistration(reference, &reg);
+               if (reg != NULL) {
+                       props = NULL;
+                       serviceRegistration_getProperties(reg, &props);
+                       status = filter_match(filter, props, &found);
+                       if (found) {
+                               srvRefs[nrFound] = reference;
+                               serviceReference_getProperty(reference, 
OSGI_FRAMEWORK_SERVICE_ID, &serviceId);
+                               srvIds[nrFound++] = (char*)serviceId;
                        }
                }
+       }
 
-               hashMapIterator_destroy(exportedServicesIterator);
-               celixThreadMutex_unlock(&manager->exportedServicesLock);
+       hashMapIterator_destroy(exportedServicesIterator);
 
-               if (nrFound > 0) {
-                       for (int i = 0; i < nrFound; i++) {
-                               // Question: can srvRefs become invalid 
meanwhile??
-                               const char* export = NULL;
-                               serviceReference_getProperty(srvRefs[i], (char 
*) OSGI_RSA_SERVICE_EXPORTED_INTERFACES, &export);
+       if (nrFound > 0) {
+               for (int i = 0; i < nrFound; i++) {
+                       // Question: can srvRefs become invalid meanwhile??

Review Comment:
   If we extend managed tracker (created by 
`celix_bundleContext_trackServices`) API to support CELIX_SERVICE_USE_DIRECT, 
as already discussed here: 
https://github.com/apache/celix/pull/399#discussion_r816607673, then a lot of 
external synchronizations, such as  `discovery->discoveredServicesMutex`, can 
be eliminated.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@celix.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to