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, ®); - 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, ®); + 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