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