PengZheng commented on code in PR #414:
URL: https://github.com/apache/celix/pull/414#discussion_r856888330
##########
bundles/remote_services/topology_manager/src/topology_manager.c:
##########
@@ -446,122 +418,138 @@ celix_status_t topologyManager_importScopeChanged(void
*handle, char *service_na
topology_manager_pt manager = (topology_manager_pt) handle;
bool found = false;
- // add already exported services to new rsa
- if (celixThreadMutex_lock(&manager->importedServicesLock) ==
CELIX_SUCCESS) {
- hash_map_iterator_pt importedServicesIterator =
hashMapIterator_create(manager->importedServices);
- while (!found &&
hashMapIterator_hasNext(importedServicesIterator)) {
- hash_map_entry_pt entry =
hashMapIterator_nextEntry(importedServicesIterator);
- endpoint = hashMapEntry_getKey(entry);
-
- entry = hashMap_getEntry(endpoint->properties, (void *)
OSGI_FRAMEWORK_OBJECTCLASS);
- char* name = (char *) hashMapEntry_getValue(entry);
- // Test if a service with the same name is imported
- if (strcmp(name, service_name) == 0) {
- found = true;
- }
+ // add already imported services to new rsa
+ celixThreadMutex_lock(&manager->lock);
+
+ hash_map_iterator_pt importedServicesIterator =
hashMapIterator_create(manager->importedServices);
+ while (!found && hashMapIterator_hasNext(importedServicesIterator)) {
+ hash_map_entry_pt entry =
hashMapIterator_nextEntry(importedServicesIterator);
+ endpoint = hashMapEntry_getKey(entry);
+
+ entry = hashMap_getEntry(endpoint->properties, (void *)
OSGI_FRAMEWORK_OBJECTCLASS);
+ char* name = (char *) hashMapEntry_getValue(entry);
+ // Test if a service with the same name is imported
+ if (strcmp(name, service_name) == 0) {
+ found = true;
}
- hashMapIterator_destroy(importedServicesIterator);
- celixThreadMutex_unlock(&manager->importedServicesLock);
}
+ hashMapIterator_destroy(importedServicesIterator);
if (found) {
- status = topologyManager_removeImportedService(manager,
endpoint, NULL);
+ status = topologyManager_removeImportedService_nolock(manager,
endpoint, NULL);
if (status != CELIX_SUCCESS) {
celix_logHelper_log(manager->loghelper,
CELIX_LOG_LEVEL_ERROR, "TOPOLOGY_MANAGER: Removal of imported service (%s; %s)
failed.", endpoint->service, endpoint->id);
} else {
- status = topologyManager_addImportedService(manager,
endpoint, NULL);
+ status =
topologyManager_addImportedService_nolock(manager, endpoint, NULL);
}
}
+
+ //should unlock until here ?, avoid endpoint is released during
topologyManager_removeImportedService
+ celixThreadMutex_unlock(&manager->lock);
+
return status;
}
-celix_status_t topologyManager_addImportedService(void *handle,
endpoint_description_t *endpoint, char *matchedFilter) {
+static celix_status_t topologyManager_addImportedService_nolock(void *handle,
endpoint_description_t *endpoint, char *matchedFilter) {
celix_status_t status = CELIX_SUCCESS;
topology_manager_pt manager = handle;
celix_logHelper_log(manager->loghelper, CELIX_LOG_LEVEL_INFO,
"TOPOLOGY_MANAGER: Add imported service (%s; %s).", endpoint->service,
endpoint->id);
Review Comment:
As for suitable logging levels, shall we adopt the interpretation of
https://stackoverflow.com/questions/2031163/when-to-use-the-different-log-levels
?
If we change level here,
`topologyManager_removeImportedService_nolock`/`topologyManager_addExportedService_nolock`/`topologyManager_removeExportedService_nolock`
should also be changed to keep everything consistent.
--
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
