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


##########
libs/framework/src/framework.c:
##########
@@ -439,53 +437,66 @@ celix_status_t fw_init(framework_pt framework) {
     }
 
     if (status != CELIX_SUCCESS) {
-       fw_logCode(framework->logger, CELIX_LOG_LEVEL_ERROR, status, "Could not 
init framework");
+        fw_logCode(framework->logger, CELIX_LOG_LEVEL_ERROR, status, "Could 
not init framework");
+        celix_framework_stopAndJoinEventQueue(framework);
     }
 
        return status;
 }
 
-celix_status_t framework_start(framework_pt framework) {
-       celix_status_t status = CELIX_SUCCESS;
-       bundle_state_e state = CELIX_BUNDLE_STATE_UNKNOWN;
+celix_status_t framework_start(celix_framework_t* framework) {
+    celix_status_t status = CELIX_SUCCESS;
+    bundle_state_e state = celix_bundle_getState(framework->bundle);
 
-       status = CELIX_DO_IF(status, bundle_getState(framework->bundle, 
&state));
-       if (status == CELIX_SUCCESS) {
-           if ((state == CELIX_BUNDLE_STATE_INSTALLED) || (state == 
CELIX_BUNDLE_STATE_RESOLVED)) {
-               status = CELIX_DO_IF(status, fw_init(framework));
-        }
-       }
+    //framework_start should be called when state is INSTALLED or RESOLVED
+    bool expectedState = state == CELIX_BUNDLE_STATE_INSTALLED || state == 
CELIX_BUNDLE_STATE_RESOLVED;
 
-       status = CELIX_DO_IF(status, bundle_getState(framework->bundle, 
&state));
-       if (status == CELIX_SUCCESS && state == CELIX_BUNDLE_STATE_STARTING) {
-        bundle_setState(framework->bundle, CELIX_BUNDLE_STATE_ACTIVE);
-       }
+    if (!expectedState) {
+        fw_log(framework->logger, CELIX_LOG_LEVEL_ERROR, "Could not start 
framework, unexpected state %i", state);
+        return CELIX_ILLEGAL_STATE;
+    }
+
+    status = CELIX_DO_IF(status, fw_init(framework));
+    status = CELIX_DO_IF(status, bundle_setState(framework->bundle, 
CELIX_BUNDLE_STATE_ACTIVE));
 
-       celix_framework_bundle_entry_t* entry = 
celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(framework,
-                                                                               
                           framework->bundleId);
-       CELIX_DO_IF(status, fw_fireBundleEvent(framework, 
OSGI_FRAMEWORK_BUNDLE_EVENT_STARTED, entry));
+    if (status != CELIX_SUCCESS) {
+        fw_log(framework->logger, CELIX_LOG_LEVEL_ERROR, "Could not initialize 
framework");
+        return status;
+    }
+
+    celix_framework_bundle_entry_t* entry =
+        
celix_framework_bundleEntry_getBundleEntryAndIncreaseUseCount(framework, 
framework->bundleId);
+    fw_fireBundleEvent(framework, OSGI_FRAMEWORK_BUNDLE_EVENT_STARTED, entry);
     celix_framework_bundleEntry_decreaseUseCount(entry);
 
-       CELIX_DO_IF(status, fw_fireFrameworkEvent(framework, 
OSGI_FRAMEWORK_EVENT_STARTED, framework->bundleId));
+    celix_status_t startStatus = 
framework_autoStartConfiguredBundles(framework);
+    celix_status_t installStatus = 
framework_autoInstallConfiguredBundles(framework);
 
-       if (status != CELIX_SUCCESS) {
-       status = CELIX_BUNDLE_EXCEPTION;
-       fw_logCode(framework->logger, CELIX_LOG_LEVEL_ERROR, status, "Could not 
start framework");
-       fw_fireFrameworkEvent(framework, OSGI_FRAMEWORK_EVENT_ERROR, status);
+    if (startStatus == CELIX_SUCCESS && installStatus == CELIX_SUCCESS) {
+        //fire started event if all bundles are started/installed and the 
event queue is empty
+        celix_framework_waitForEmptyEventQueue(framework);

Review Comment:
   > I do think the wait call is needed before providing a "framework.ready" 
condition, because only then is the framework and configured bundle startup 
really done.
   
   Indeed, we need the wait call before claiming "framwork.ready" condition.
   I just noticed that sync `celix_framework_startBundleInternal`, which is 
called by `framework_autoStartConfiguredBundlesForList`, calls 
`celix_framework_waitForBundleEvents(fw, bndId)` for each bundle started. 
   
   Does the final wait make these syncs unnecessary? Shall we remove them from 
framework startup?
   Of course `celix_framework_waitForBundleEvents` is needed for normal 
`celix_framework_startBundle`.



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