added lazy activation to smarthome.core

[kaikreuzer]

Signed-off-by: Kai Kreuzer kai@openhab.org

[kaikreuzer]

Please do not merge yet - I assume that this small change makes some tests failing (https://travis-ci.org/eclipse/smarthome/builds/147154102). We will first have to figure out, whether this is a problem with the tests or if they actually hint at some lifecycle problem within our code base.

[maggu2810]

What's the reason to use lazy activation at all?

[kaikreuzer]

See the warning that Eclipse shows you on the MANIFEST.MF:
"Bundles with a Service-Component should set the Bundle-ActivationPolicy to lazy."
And this imho makes sense - if there is a component within the bundle, the bundle should be activated first.

[maggu2810]

That message is already discusses in some Eclipse IDE bugs.
The lazy activation is only relevant for Eclipse RCP applications AFAIK.

A bundle is always activated before a service of a bundle is activated. That is defined by the OSGi spec.

[kaikreuzer]

A bundle is always activated before a service of a bundle is activated.

This sounds differently:

"It's not just Equinox that will only consider DS descriptors in active (or lazy-activated) bundles -- that's in the DS spec"

The lazy activation is hence triggered here once other services access exported interfaces of this bundle. And this imho makes sense, because if you e.g. implement a BindingInfoProvider, you automatically assume that the BindingInfoRegistry service is started as well, which will pick up and use your service.

[maggu2810]

I have written that a bundle is activated before a service of that bundle is activated.
So, it is clear that If a bundle is not activated, then there is also no service activated.

The lazy activation does not change the order between activation of the bundle and the activation of the services.
It could delay the activation of the bundle.

But I don't think we need to care about "delay the activation of smarthome.core until a class is loaded" because I don't think it makes sense to install smarthome.core without using it at all.

[kaikreuzer]

But I don't think we need to care about "delay the activation of smarthome.core until a class is loaded" because I don't think it makes sense to install smarthome.core without using it at all.

Which means that you assume that the bundle is always started anyhow. But this PR shows that some tests do NOT start the bundle and that in the case it IS (lazily) started, the tests fail. And this is exactly the reason why I think the "why" needs to be analyzed, because it is well possible that there is indeed some bug for certain startup orders, don't you think?

[maggu2810]

If there is something wrong it needs to be analysed and resolved. Sure.

Perhaps my reading has been wrong.
My reading was that adding BAPL (BundleActivationPolicy lazy) has introduced the test failures and there has been no failure before.
So I asked you why you have added the BAPL.
There has been two reason (as far as I understand):

1. the Eclipse IDE would like to use BAPL
2. you want to ensure that the bundle activator is called before a DS is activated
   As I have written, 2. is defined by the OSGi spec. If the activator of a DS is called, you can rely on the fact, that the activator of the bundle has been called before.
   The point 1. has been introduced by Neon. BAPL is necessary for Eclipse RCP / Eclipse Plugins but not for plani OSGi bundles. That is caused because Eclipse handles a missing BAPL in Plugins different.
   So, the two points are no argument for me (in respect to my current understanding of the topic) to introduce the BAPL for that bundle (it is an argument for the models to get the designer working).

If there is no problem if we don't introduce it, we can analyse the startup order, too. But is there really a need for?
If there are other reasons to introduce BAPL, I am not aware of it.
If we want to understand why tests are failing using BAPL without using it at the end, I am fine, too. I love to learn more about the topic. But if it is only about that, then I think the priority is much lower. 😉

[kaikreuzer]

So I asked you why you have added the BAPL.

I added it for highlighting a problem (failing test due to different startup behavior).

Your argument is always: The BAPL is not required as it does not make any difference - well, this PR shows that it does.

And again, I think it always makes sense to have a bundle started, if any of its exported classes is used, don't you agree? To make sure that this is the case, BAPL is required (even for non Eclipse RCP apps).

But if it is only about that, then I think the priority is much lower.

If it is only the test that has a problem, I agree that it is low prio. But if the test actually shows a problem that can occur on certain bundle startup orders (and ESH does not define any constraints here, right?), the prio can be high, because it means that some core functionality (i18n support) is broken.

[maggu2810]

Your argument is always: The BAPL is not required as it does not make any difference - well, this PR shows that it does.

@kaikreuzer That is not what I have written. I have written, that it does not make any difference for the order of the DS and bundle activation. So, you have asked me this some time and now I do the same: Please do not shorten what I have written.
BAPL could delay a bundle activation activation until a class is loaded from the bundle.
If we rely on the fact that a service is present (created by the bundle activator) without access a class before, this will surely break something.
That is (for example) the reason why I wanted that you remove the BAPL from a bundle that contains only internal stuff and no DS.

If you really think that using BAPL a service of the bundle is present before the bundle of the activator is called then this is really a bug that should be inspected.

But the fact that the bundle is not activated as long as a class of the bundle is accessed and so a service provided by the bundle activator is missing, is AFAIK the topic, expected behaviour.

I will have a look at this bug after my vacation, but I assume the problem is that the bundle is not activated at the time another one expected an activated and (and the problem is not the order of DS and bundle activator).

[maggu2810]

I agree that I would like to remove start levels and if the test relies on the fact that smarthome.core is activated before and that is broken, then there is a wrong assumption that should be fixed. Will have a look at.
I assume it is wrong to assume the activator of the bundle has been finished already.
I will have a look at ... some time ...

[kaikreuzer]

I have written, that it does not make any difference for the order of the DS and bundle activation.

I didn't refer to your statements in this issue, but to our past discussions (where I cannot find a link right now, but the essence that I thought to remember was that "BAPL is only required for Eclipse RCP and isn't relevant for a headless OSGi app" - if I made this up, please forgive me and ignore it.

I think the main question which we have to answer ourselves first is:

Is it ok that the application allows using classes from bundles that are not started?

To me it always feels weird and rather misleading if a bundle is not started, but only resolved (e.g. when listing the bundles in the console), but still it is under heavy use by others (not talking about DS at all here). This is e.g. a likely situation for API-bundles. For me, it feels much more natural and clear if the above question is answered with "no" (being aware that the OSGi spec does not require this to be the case).

[tomhoefer]

I checked why the ConfigStatusServiceOSGiTest fails. The reason is that the core I18nProvider implementation is used instead of the mocked one.

If the immediate attribute from the ConfigStatusService is removed and the mocked I18nProvider is registered with a higher service ranking then the test will not fail.

As a result it seems that we have some dependency issues when starting the core framework.