tjwatson commented on PR #334: URL: https://github.com/apache/felix-dev/pull/334#issuecomment-2996682449
The prototype service references were implemented before my involvement in the SCR implementation. I admit that I am unclear on exactly how the prototype references are supposed to be handled, but I think there is a larger issue with how the prototype references are managed and tracked beyond this IllegalArgumentException. Consider a component that has two field references that are prototype required. In this case lets have them be a reference to A and a reference to B: ``` A reference1; B reference2; ``` Should `reference1` and `reference2` here be the SAME object or should SCR cause the prototype service to create two instances of `BImpl` Right now it appears Felix SCR creates only one `BImpl` instance and injects that same object to both `reference1` and `reference2`. But from the specification I am unclear if that is the right behavior. My expectation is that we would have to instances of `BImpl` tracked and injected into the two fields. The other problem, which is more directly related to this issue, is that this call ends up ungetting the service object: `DependencyManager.invokeUnbindMethod(ComponentContextImpl<S>, RefPair<S, T>, int, EdgeInfo)` In fact it ends up ungetting all prototype instances from the prototye factory and closing down the tracking. This seems wrong to me and I am unclear what would happen if we had `reference1` and `reference2` above but made `reference2` a dynamic/optional reference and changed the service properties such that `reference2` become unsatisfied but `reference1` remained valid. my guess is that SCR will unget the service object for `reference2`, but that would be wrong since `reference1` still would reference the same object. With all that said, this appears to be a much more complicated issue that I cannot spend time one for now and I am not convinced the fix in the current PR is correct for non-prototype service references. For now I suggest we catch the `IllegalArgumentException` in `org.apache.felix.scr.impl.manager.AbstractPrototypeRefPair.doUngetService(ScrComponentContext, T)` That is until we can get a better view of the correct specification behavior with respect to prototype service references. -- 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...@felix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
