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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]