-
Notifications
You must be signed in to change notification settings - Fork 782
Circular reference in XmlChannelTypeProvider #6054
Comments
Has the OH distro used the same way as the distros before? |
Yes, nothing has changed about the distro and the problem is that this is definitely not reproducible - I only saw it once during a startup now, a subsequent one did not show it. So I cannot say that it has been introduced recently, I might only have observed it by chance today. |
Thanks for the hint @cweitkamp, here is what I found. We have the following dependency chain in OSGi services:
All services have |
Anybody with a good idea how this could be resolved? |
Doesn't it help if we remove all the non really necessary immediate activations? |
Why should it? It would not guarantee any kind of instantiation order and not remove the cycle - so I would guess the risk would be still there. |
I didn't have a look at the current implementation but immediate or not has (if my reading is correct) influences how SCR works (see e.g. https://issues.apache.org/jira/plugins/servlet/mobile#issue/FELIX-4417). |
I don't think that a 0..n dependency can be treated as a "break" in the cycle. Imho, such cyclic dependencies must be avoided in general, independent of the type of dependency. |
Sure, I didn't say (hopefully nothing different).
Hm, why not? Wouldn't that order work:
for each XMLChannelTypeProvider / XMLChannelGroupTypeProvider
My assumption has been it could perhaps work if the provider don't declare to be activated immediately, so it could be postponed... But let's try to break the cycle ourselves (I still -- you already know -- would like that we sometime remove the immediate activation if it is not necessary): @Override
protected ChannelType localize(Bundle bundle, ChannelType channelType, Locale locale) {
if (thingTypeI18nLocalizationService == null) {
return null;
}
return thingTypeI18nLocalizationService.createLocalizedChannelType(bundle, channelType, locale);
} If there is a "thing type i18n location service" and the provider already delegate the localization to that service without any other addition, why providing such a method on the provider at all? |
I also think this is the only way to go. Here is may plan (which is nearly ready in terms of code btw): The cyclic dependency occurs because we need the ChannelTypeRegistry when localising ThingType and ChannelGroupType. When localising ChannelType the ChannelTypeRegistry is not used. Currently the ChannelTypeRegistry manages both ChannelType & ChannelGroupType. |
Thanks for the link, @maggu2810. So yes, by this a 0..n dependency should indeed break the cycle (although it is a pity that the spec does not go on and say whether or not the 0..n dependency (if it is a dynamic one) should be resolved after all the rest has been activated). Anyhow, the safest way is to not have cycles at all, so I am all for @htreu's fix 👍 . |
@htreu : is this issue to be fixed now that your PR was merged ? |
Yes, thanks! |
New exception that I have just found in the latest openHAB distro:
The text was updated successfully, but these errors were encountered: