celix-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [celix] pnoltes commented on a change in pull request #313: Feature/refactor c dep man service trackers
Date Tue, 02 Feb 2021 13:44:54 GMT

pnoltes commented on a change in pull request #313:
URL: https://github.com/apache/celix/pull/313#discussion_r568611096



##########
File path: libs/framework/src/dm_component_impl.c
##########
@@ -416,466 +432,250 @@ celix_status_t celix_dmComponent_removeInterface(celix_dm_component_t
*component
     celix_status_t status = CELIX_ILLEGAL_ARGUMENT;
 
     celixThreadMutex_lock(&component->mutex);
-    int nof_interfaces = arrayList_size(component->dm_interfaces);
-    for (unsigned int i = 0; i < nof_interfaces; ++i) {
-        dm_interface_t *interface = (dm_interface_t *) arrayList_get(component->dm_interfaces,
i);
+    int nof_interfaces = celix_arrayList_size(component->providedInterfaces);
+    dm_interface_t* removedInterface = NULL;
+    for (int i = 0; i < nof_interfaces; ++i) {
+        dm_interface_t *interface = celix_arrayList_get(component->providedInterfaces,
i);
         if (interface->service == service) {
-            arrayList_remove(component->dm_interfaces, i);
-            if (component->state == DM_CMP_STATE_TRACKING_OPTIONAL) {
-                celixThreadMutex_unlock(&component->mutex);
-                component_unregisterServices(component);
-                component_registerServices(component);
-                celixThreadMutex_lock(&component->mutex);
-            }
-            status = CELIX_SUCCESS;
+            celix_arrayList_removeAt(component->providedInterfaces, i);
+            removedInterface = interface;
             break;
         }
     }
     celixThreadMutex_unlock(&component->mutex);
 
+    if (removedInterface != NULL) {
+        celix_bundleContext_unregisterService(component->context, removedInterface->svcId);
+        free(removedInterface->serviceName);
+        free(removedInterface);
+    }
+
     return status;
 }
 
-celix_status_t component_getInterfaces(celix_dm_component_t *component, array_list_pt *out)
{
+celix_status_t component_getInterfaces(celix_dm_component_t *component, celix_array_list_t
**out) {
     return celix_dmComponent_getInterfaces(component, out);
 }
 
-celix_status_t celix_dmComponent_getInterfaces(celix_dm_component_t *component, array_list_pt
*out) {
-    celix_status_t status = CELIX_SUCCESS;
-    array_list_pt names = NULL;
-    arrayList_create(&names);
+celix_status_t celix_dmComponent_getInterfaces(celix_dm_component_t *component, celix_array_list_t
**out) {
+    celix_array_list_t* names = celix_arrayList_create();
+
     celixThreadMutex_lock(&component->mutex);
-    int size = arrayList_size(component->dm_interfaces);
-    int i;
-    for (i = 0; i < size; i += 1) {
-        dm_interface_info_pt info = calloc(1, sizeof(*info));
-        dm_interface_t *interface = arrayList_get(component->dm_interfaces, i);
-        info->name = strdup(interface->serviceName);
-        properties_copy(interface->properties, &info->properties);
-        arrayList_add(names, info);
+    int size = celix_arrayList_size(component->providedInterfaces);
+    for (int i = 0; i < size; i += 1) {
+        dm_interface_info_t* info = calloc(1, sizeof(*info));
+        dm_interface_t *interface = celix_arrayList_get(component->providedInterfaces,
i);
+        info->name = celix_utils_strdup(interface->serviceName);
+        info->properties = celix_properties_copy(interface->properties);
+        celix_arrayList_add(names, info);
     }
     celixThreadMutex_unlock(&component->mutex);
+    *out = names;
 
-    if (status == CELIX_SUCCESS) {
-        *out = names;
-    }
-
-    return status;
-}
-
-celix_status_t celix_private_dmComponent_handleEvent(celix_dm_component_t *component, celix_dm_service_dependency_t
*dependency, dm_event_pt event) {
-	celix_status_t status = CELIX_SUCCESS;
-
-	dm_handle_event_type_pt data = calloc(1, sizeof(*data));
-	data->dependency = dependency;
-	data->event = event;
-	data->newEvent = NULL;
-
-	status = executor_executeTask(component->executor, component, component_handleEventTask,
data);
-//	component_handleEventTask(component, data);
-
-	return status;
+    return CELIX_SUCCESS;
 }
 
-static celix_status_t component_handleEventTask(celix_dm_component_t *component, dm_handle_event_type_pt
data) {
-	celix_status_t status = CELIX_SUCCESS;
-
-	switch (data->event->event_type) {
-		case DM_EVENT_ADDED:
-			component_handleAdded(component,data->dependency, data->event);
-			break;
-		case DM_EVENT_CHANGED:
-			component_handleChanged(component,data->dependency, data->event);
-			break;
-		case DM_EVENT_REMOVED:
-			component_handleRemoved(component,data->dependency, data->event);
-			break;
-		case DM_EVENT_SWAPPED:
-			component_handleSwapped(component,data->dependency, data->event, data->newEvent);
-			break;
-		default:
-			break;
-	}
-
-	free(data);
-
-	return status;
+celix_status_t celix_private_dmComponent_handleEvent(celix_dm_component_t *component, const
celix_dm_event_t* event) {
+    celix_status_t status = CELIX_SUCCESS;
+    switch (event->eventType) {
+        case CELIX_DM_EVENT_SVC_ADD:
+            celix_dmComponent_handleAdd(component, event);
+            break;
+        case CELIX_DM_EVENT_SVC_REM:
+            celix_dmComponent_handleRemove(component, event);
+            break;
+        case CELIX_DM_EVENT_SVC_SET:
+            celix_dmComponent_handleSet(component, event);
+            break;
+        default:
+            break;
+    }
+    return status;
 }
 
-static celix_status_t component_suspend(celix_dm_component_t *component, celix_dm_service_dependency_t
*dependency) {
+static celix_status_t celix_dmComponent_suspend(celix_dm_component_t *component, celix_dm_service_dependency_t
*dependency) {
 	celix_status_t status = CELIX_SUCCESS;
-
-	dm_service_dependency_strategy_t strategy;
-	serviceDependency_getStrategy(dependency, &strategy);
+	dm_service_dependency_strategy_t strategy = celix_dmServiceDependency_getStrategy(dependency);
 	if (strategy == DM_SERVICE_DEPENDENCY_STRATEGY_SUSPEND &&  component->callbackStop
!= NULL) {
+        celix_bundleContext_log(component->context, CELIX_LOG_LEVEL_TRACE,
+               "Suspending component %s (uuid=%s)",
+               component->name,
+               component->uuid);
+        celix_dmComponent_unregisterServices(component, true);
 		status = component->callbackStop(component->implementation);
 	}
-
 	return status;
 }
 
-static celix_status_t component_resume(celix_dm_component_t *component, celix_dm_service_dependency_t
*dependency) {
+static celix_status_t celix_dmComponent_resume(celix_dm_component_t *component, celix_dm_service_dependency_t
*dependency) {
 	celix_status_t status = CELIX_SUCCESS;
-
-	dm_service_dependency_strategy_t strategy;
-	serviceDependency_getStrategy(dependency, &strategy);
-	if (strategy == DM_SERVICE_DEPENDENCY_STRATEGY_SUSPEND &&  component->callbackStop
!= NULL) {
-		status = component->callbackStart(component->implementation);
+    dm_service_dependency_strategy_t strategy = celix_dmServiceDependency_getStrategy(dependency);
+	if (strategy == DM_SERVICE_DEPENDENCY_STRATEGY_SUSPEND &&  component->callbackStart
!= NULL) {
+	    //ensure that the current state is still TRACKING_OPTION. Else during a add/rem/set
call a required svc dep has been added.
+        celixThreadMutex_lock(&component->mutex);
+        celix_dm_component_state_t currentState = component->state;
+        celixThreadMutex_unlock(&component->mutex);
+        if (currentState == DM_CMP_STATE_TRACKING_OPTIONAL) {
+            celix_bundleContext_log(component->context, CELIX_LOG_LEVEL_TRACE,
+                                    "Resuming component %s (uuid=%s)",
+                                    component->name,
+                                    component->uuid);
+            celix_dmComponent_registerServices(component, true);
+            status = component->callbackStart(component->implementation);
+            component->nrOfTimesResumed += 1;
+        } else {
+            celix_bundleContext_log(component->context, CELIX_LOG_LEVEL_TRACE,
+                                    "Skipping resume because cmp state is now %s",
+                                    celix_dmComponent_stateToString(currentState));
+        }
 	}
-
 	return status;
 }
 
-static celix_status_t component_handleAdded(celix_dm_component_t *component, celix_dm_service_dependency_t
*dependency, dm_event_pt event) {
-    celix_status_t status = CELIX_SUCCESS;
-
-    pthread_mutex_lock(&component->mutex);
-    array_list_pt events = hashMap_get(component->dependencyEvents, dependency);
-    arrayList_add(events, event);
-    pthread_mutex_unlock(&component->mutex);
-
-    serviceDependency_setAvailable(dependency, true);
-
+static celix_status_t celix_dmComponent_handleEvent(celix_dm_component_t *component, const
celix_dm_event_t* event, celix_status_t (*setAddOrRemFp)(celix_dm_service_dependency_t *dependency,
void* svc, const celix_properties_t* props), const char *invokeName) {
+    bool needSuspend = false;
+    celixThreadMutex_lock(&component->mutex);
     switch (component->state) {
-        case DM_CMP_STATE_WAITING_FOR_REQUIRED: {
-            serviceDependency_invokeSet(dependency, event);
-            bool required = false;
-            serviceDependency_isRequired(dependency, &required);
-            if (required) {
-                component_handleChange(component);
-            }
-            break;
-        }
-        case DM_CMP_STATE_INSTANTIATED_AND_WAITING_FOR_REQUIRED: {
-            bool instanceBound = false;
-            serviceDependency_isInstanceBound(dependency, &instanceBound);
-            bool required = false;
-            serviceDependency_isRequired(dependency, &required);
-            if (!instanceBound) {
-                if (required) {
-                    serviceDependency_invokeSet(dependency, event);
-                    serviceDependency_invokeAdd(dependency, event);
-                }
-                dm_event_pt event = NULL;
-                component_getDependencyEvent(component, dependency, &event);
-                component_updateInstance(component, dependency, event, false, true);
-            }
-
-            if (required) {
-                component_handleChange(component);
-            }
-            break;
-        }
         case DM_CMP_STATE_TRACKING_OPTIONAL:
-		    component_suspend(component,dependency);
-            serviceDependency_invokeSet(dependency, event);
-            serviceDependency_invokeAdd(dependency, event);
-		    component_resume(component,dependency);
-            dm_event_pt event = NULL;
-            component_getDependencyEvent(component, dependency, &event);
-            component_updateInstance(component, dependency, event, false, true);
+            needSuspend = true;
             break;
-        default:
+        default: //DM_CMP_STATE_INACTIVE
             break;
     }
-
-    return status;
-}
-
-static celix_status_t component_handleChanged(celix_dm_component_t *component, celix_dm_service_dependency_t
*dependency, dm_event_pt event) {
-    celix_status_t status = CELIX_SUCCESS;
-
-    pthread_mutex_lock(&component->mutex);
-    array_list_pt events = hashMap_get(component->dependencyEvents, dependency);
-    int index = arrayList_indexOf(events, event);
-    if (index < 0) {
-	pthread_mutex_unlock(&component->mutex);
-        status = CELIX_BUNDLE_EXCEPTION;
-    } else {
-        dm_event_pt old = arrayList_remove(events, (unsigned int) index);
-        arrayList_add(events, event);
-        pthread_mutex_unlock(&component->mutex);
-
-        serviceDependency_invokeSet(dependency, event);
-        switch (component->state) {
-            case DM_CMP_STATE_TRACKING_OPTIONAL:
-			    component_suspend(component,dependency);
-                serviceDependency_invokeChange(dependency, event);
-			    component_resume(component,dependency);
-                dm_event_pt hevent = NULL;
-                component_getDependencyEvent(component, dependency, &hevent);
-                component_updateInstance(component, dependency, hevent, true, false);
-                break;
-            case DM_CMP_STATE_INSTANTIATED_AND_WAITING_FOR_REQUIRED: {
-                bool instanceBound = false;
-                serviceDependency_isInstanceBound(dependency, &instanceBound);
-                if (!instanceBound) {
-                    serviceDependency_invokeChange(dependency, event);
-                    dm_event_pt hevent = NULL;
-                    component_getDependencyEvent(component, dependency, &hevent);
-                    component_updateInstance(component, dependency, hevent, true, false);
-                }
-                break;
-            }
-            default:
-                break;
-        }
-
-        event_destroy(&old);
-    }
-
-    return status;
+    celixThreadMutex_unlock(&component->mutex);
+    if (needSuspend) {
+        celix_dmComponent_suspend(component, event->dep);
+    }
+    celix_bundleContext_log(component->context, CELIX_LOG_LEVEL_TRACE,
+        "Calling %s service for component %s (uuid=%s) on service dependency with type %s",
+        invokeName,
+        component->name,
+        component->uuid,
+        event->dep->serviceName);
+    setAddOrRemFp(event->dep, event->svc, event->props);
+    //note that after after a set/add/rem a new svc dependency can be added, removed, etc
+    if (needSuspend) {
+        celix_dmComponent_resume(component, event->dep);
+    }
+    celix_dmComponent_handleChange(component);
+    return CELIX_SUCCESS;
 }
 
-static celix_status_t component_handleRemoved(celix_dm_component_t *component, celix_dm_service_dependency_t
*dependency, dm_event_pt event) {
-    celix_status_t status = CELIX_SUCCESS;
-
-    pthread_mutex_lock(&component->mutex);
-    array_list_pt events = hashMap_get(component->dependencyEvents, dependency);
-    int size = arrayList_size(events);
-    if (arrayList_contains(events, event)) {
-        size--;
-    }
-    pthread_mutex_unlock(&component->mutex);
-    serviceDependency_setAvailable(dependency, size > 0);
-    component_handleChange(component);
-
-    pthread_mutex_lock(&component->mutex);
-    int index = arrayList_indexOf(events, event);
-    if (index < 0) {
-	pthread_mutex_unlock(&component->mutex);
-        status = CELIX_BUNDLE_EXCEPTION;
-    } else {
-        dm_event_pt old = arrayList_remove(events, (unsigned int) index);
-        pthread_mutex_unlock(&component->mutex);
-
-
-        switch (component->state) {
-            case DM_CMP_STATE_INSTANTIATED_AND_WAITING_FOR_REQUIRED: {
-                serviceDependency_invokeSet(dependency, event);
-                bool instanceBound = false;
-                serviceDependency_isInstanceBound(dependency, &instanceBound);
-                if (!instanceBound) {
-                    bool required = false;
-                    serviceDependency_isRequired(dependency, &required);
-                    if (required) {
-                        serviceDependency_invokeRemove(dependency, event);
-                    }
-                    dm_event_pt hevent = NULL;
-                    component_getDependencyEvent(component, dependency, &hevent);
-                    component_updateInstance(component, dependency, hevent, false, false);
-                }
-                break;
-            }
-            case DM_CMP_STATE_TRACKING_OPTIONAL:
-			    component_suspend(component,dependency);
-                serviceDependency_invokeSet(dependency, event);
-                serviceDependency_invokeRemove(dependency, event);
-			    component_resume(component,dependency);
-                dm_event_pt hevent = NULL;
-                component_getDependencyEvent(component, dependency, &hevent);
-                component_updateInstance(component, dependency, hevent, false, false);
-                break;
-            default:
-                break;
-        }
-
-        event_destroy(&event);
-        if (old) {
-            event_destroy(&old);
-        }
-    }
-
-    return status;
+static celix_status_t celix_dmComponent_handleAdd(celix_dm_component_t *component, const
celix_dm_event_t* event) {
+    return celix_dmComponent_handleEvent(component, event, celix_dmServiceDependency_invokeAdd,
"add");
 }
 
-static celix_status_t component_handleSwapped(celix_dm_component_t *component, celix_dm_service_dependency_t
*dependency, dm_event_pt event, dm_event_pt newEvent) {
-    celix_status_t status = CELIX_SUCCESS;
-
-    pthread_mutex_lock(&component->mutex);
-    array_list_pt events = hashMap_get(component->dependencyEvents, dependency);
-    int index = arrayList_indexOf(events, event);
-    if (index < 0) {
-	pthread_mutex_unlock(&component->mutex);
-        status = CELIX_BUNDLE_EXCEPTION;
-    } else {
-        dm_event_pt old = arrayList_remove(events, (unsigned int) index);
-        arrayList_add(events, newEvent);
-        pthread_mutex_unlock(&component->mutex);
-
-        serviceDependency_invokeSet(dependency, event);
-
-        switch (component->state) {
-            case DM_CMP_STATE_WAITING_FOR_REQUIRED:
-                break;
-            case DM_CMP_STATE_INSTANTIATED_AND_WAITING_FOR_REQUIRED: {
-                bool instanceBound = false;
-                serviceDependency_isInstanceBound(dependency, &instanceBound);
-                if (!instanceBound) {
-                    bool required = false;
-                    serviceDependency_isRequired(dependency, &required);
-                    if (required) {
-                        serviceDependency_invokeSwap(dependency, event, newEvent);
-                    }
-                }
-                break;
-            }
-            case DM_CMP_STATE_TRACKING_OPTIONAL:
-			    component_suspend(component,dependency);
-                serviceDependency_invokeSwap(dependency, event, newEvent);
-			    component_resume(component,dependency);
-                break;
-            default:
-                break;
-        }
-
-        event_destroy(&event);
-        if (old) {
-            event_destroy(&old);
-        }
-    }
-
-    return status;
+static celix_status_t celix_dmComponent_handleRemove(celix_dm_component_t *component, const
celix_dm_event_t* event) {
+    return celix_dmComponent_handleEvent(component, event, celix_dmServiceDependency_invokeRemove,
"remove");
 }
 
-static celix_status_t component_updateInstance(celix_dm_component_t *component, celix_dm_service_dependency_t
*dependency, dm_event_pt event, bool update, bool add) {
-    celix_status_t status = CELIX_SUCCESS;
-
-    bool autoConfig = false;
-
-    serviceDependency_isAutoConfig(dependency, &autoConfig);
-
-    if (autoConfig) {
-        const void *service = NULL;
-        const void **field = NULL;
-
-        if (event != NULL) {
-            event_getService(event, &service);
-        }
-        serviceDependency_getAutoConfig(dependency, &field);
-        serviceDependency_lock(dependency);
-        *field = service;
-        serviceDependency_unlock(dependency);
-    }
-
-    return status;
+static celix_status_t celix_dmComponent_handleSet(celix_dm_component_t *component, const
celix_dm_event_t* event) {
+    return celix_dmComponent_handleEvent(component, event, celix_dmServiceDependency_invokeSet,
"set");
 }
 
-static celix_status_t component_startDependencies(celix_dm_component_t *component __attribute__((unused)),
array_list_pt dependencies) {
-    celix_status_t status = CELIX_SUCCESS;
-    array_list_pt required_dependencies = NULL;
-    arrayList_create(&required_dependencies);
-
-    for (unsigned int i = 0; i < arrayList_size(dependencies); i++) {
-        celix_dm_service_dependency_t *dependency = arrayList_get(dependencies, i);
-        bool required = false;
-        serviceDependency_isRequired(dependency, &required);
-        if (required) {
-            arrayList_add(required_dependencies, dependency);
-            continue;
-        }
-
-        serviceDependency_start(dependency);
-    }
-
-    for (unsigned int i = 0; i < arrayList_size(required_dependencies); i++) {
-        celix_dm_service_dependency_t *dependency = arrayList_get(required_dependencies,
i);
-        serviceDependency_start(dependency);
+/**
+ * perform state transition. This function should be called with the component->mutex
locked.
+ */
+static celix_status_t celix_dmComponent_enableDependencies(celix_dm_component_t *component)
{
+    for (int i = 0; i < celix_arrayList_size(component->dependencies); i++) {
+        celix_dm_service_dependency_t *dependency = arrayList_get(component->dependencies,
i);
+        celix_dmServiceDependency_enable(dependency);
     }
-
-    arrayList_destroy(required_dependencies);
-
-    return status;
+    return CELIX_SUCCESS;
 }
 
-static celix_status_t component_stopDependencies(celix_dm_component_t *component) {
-    celix_status_t status = CELIX_SUCCESS;
-
-    pthread_mutex_lock(&component->mutex);
-    for (unsigned int i = 0; i < arrayList_size(component->dependencies); i++) {
+/**
+ * perform state transition. This function should be called with the component->mutex
locked.
+ */
+static celix_status_t celix_dmComponent_disableDependencies(celix_dm_component_t *component)
{
+    for (int i = 0; i < celix_arrayList_size(component->dependencies); i++) {
         celix_dm_service_dependency_t *dependency = arrayList_get(component->dependencies,
i);
-        pthread_mutex_unlock(&component->mutex);
-        serviceDependency_stop(dependency);
-        pthread_mutex_lock(&component->mutex);
+        celix_dmServiceDependency_disable(dependency);
     }
-    pthread_mutex_unlock(&component->mutex);
-
-    return status;
+    return CELIX_SUCCESS;
 }
 
-static celix_status_t component_handleChange(celix_dm_component_t *component) {
-    celix_status_t status = CELIX_SUCCESS;
 
+/**
+ * Calculate and handle state change. This function should be called with the component->mutex
locked.

Review comment:
       comment is incorrect, removed.




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



Mime
View raw message