Circular Reference

[sjsf]

!MESSAGE Circular reference detected trying to get service {org.eclipse.smarthome.core.items.ItemProvider}={service.id=451, service.bundleid=61, service.scope=bundle, component.name=org.eclipse.smarthome.core.thing.internal.ChannelItemProvider, component.id=50}
 stack of references: ServiceReference: {org.eclipse.smarthome.core.items.ItemProvider}={service.id=451, service.bundleid=61, service.scope=bundle, component.name=org.eclipse.smarthome.core.thing.internal.ChannelItemProvider, component.id=50}
ServiceReference: {org.eclipse.smarthome.core.thing.link.ItemChannelLinkRegistry}={service.id=450, service.bundleid=61, service.scope=bundle, component.name=org.eclipse.smarthome.core.thing.link.ItemChannelLinkRegistry, component.id=60}
ServiceReference: {org.eclipse.smarthome.core.items.ItemRegistry}={service.id=449, service.bundleid=137, service.scope=bundle, component.name=org.eclipse.smarthome.core.itemregistry, component.id=140}

ItemRegistryImpl -> ChannelItemProvider -> ItemChannelLinkRegistry -> ItemRegistry

[sjsf]

Looks like we missed this cycle when introducing the dependency in #2122

[maggu2810]

This cycle exists since Sep 6, 2016? Why didn't we notice earlier? I have never seen this warning in my runtime (using Felix).

[sjsf]

Why didn't we notice earlier?

The dependency is somewhat hidden to the SCR because the AbstractRegistry registers a ServiceTracker instead of properly declaring a dependency. Probably to circumvent such cycles - which worked in Equinox DS most of the time as a "workaround". So maybe it doesn't anymore since the Equinox upgrade (which is now using Felix SCR). But I'm just guessing wildly, in fact I was surprised by it too.

In any case, it's true and it hurts.

[triller-telekom]

I just had a look at this problem again and the underlying problem is the following:

The ChannelItemProvider provides "intermediate" Items for newly created links where no Item exists so far. However it should withdraw its "intermediate" Item once another ItemProvider wants to provide the "real Item (same name).

If another ItemProvider wants to provide an Item which exists in the ItemRegistry and which is owned by the ChannelItemProvider, it will get an exception because the Item already exists. In order to prevent this exception, there is a RegistryHook mechanism that is used in the adding method. The ChannelItemProvider has such a registered hook and withdraws its intermediate Item and afterwards the other ItemProvider can add its ´Item` without an exception.

That is why the ChannelItemProvider has a reference to the ItemRegistry since the beginning.

One approach I see to solve this issue is that we introduce two different kind of ItemProvider. The regular ones and those who provide intermediate Items. This way the ChannelItemProvider does not need to have a dependency to the ItemRegistry, because the ItemRegistry can ask all IntermediateItemProviders while an Item should be added, if there is such an intermediate Item. If that is the case, it can notify the IntermediateItemProvider owning the Item to withdraw it, before it adds the one from the regular ItemProvider.

This is just a thought which came to my mind to solve this circular dependency. I haven't thought it fully through because the asking the IntermediateItemProvider part and notifying it to remove its intermediate Item has to be synchronous within the Itemregistry.add(Item) method, so the intermediate Item has been removed before the regular one will be added.

[maggu2810]

Just some thoughts:

The Registry already knows a special ManagedProvider, so it should be okay to know a special OnlyIfNoOtherOneProvidesItAlreadyProvider (or some better name).

Interaction / provided methods (perhaps):

• containsUID(Identifiable<T> uid)
• getByUID(Identifiable<T> uid)
• markUsage(Identifiable<T> id, boolean used)

For Items the generic type parameter T would be String.

If an element / Item is added to the registry by another provider the registry could modify its logic:

• The added method currently checks if there is already an existing element. If there is one, it logs a warning and return. It could check if the provider is an instance of OnlyIfNoOtherOneProvidesItAlreadyProvider and if:
  • remove / replace / update that element
  • call markUsage(..., false) to inform the provider that it is now provided by another one / not used by that provider anymore.

Also the removed method could be changed if the provider is not an instance of OnlyIfNoOtherOneProvidesItAlreadyProvider the method getByUID of that provider is called and if the returned reference is not null the current element is replaced by the one of the OnlyIfNoOtherOneProvidesItAlreadyProvider (and the provider could be informed by calling the markUsage method).

I don't know if markUsage or similar is necessary at all. Does the special provider needs to know if someone needs a "virtual" element and clean it up if there is no usage anymore?