Community
Participate
Working Groups
Currently if a ServiceFactory causes a recursive call into itself during getService the framework allows it. This usually leads to endless recursion and a stack overflow. At least two other framework implementations that I know of prevent this (Apache Felix and Prosyst). In both cases they detect the recursive call, fire an error event and return null. I don't think the spec mandates this behavior but it seems reasonable.
Created attachment 182940 [details] Patch to detect recursion, log an error and return null. I think we should detect recursive calls to the ServiceFactory. There is little point in no handling this since the thread/VM will fail with stack overflow. This patch will detect the recursion, log an error and return null to the caller. A new ServiceException type is defined by OSGi as well.
Created attachment 182941 [details] fix + apifilter for new osgi API Same patch but with an additional filter for API tooling to ignore the additional OSGi API.
+1 BJ release when ready.
Released the updated patch with some comment corrections.
BJ, in the OSGi bug and in comment 1 you mention that the framework should log an error. My interpretation is that means a FrameworkEvent of type ERROR should be published. In the fix you use a WARNING. Should it be an ERROR event?
(In reply to comment #5) > BJ, in the OSGi bug and in comment 1 you mention that the framework should log > an error. My interpretation is that means a FrameworkEvent of type ERROR > should be published. In the fix you use a WARNING. Should it be an ERROR > event? Hmmm. I guess it should be an error. I copied the code from the service==null test which only logs a warning. Why is that not an error also?
(In reply to comment #6) > Hmmm. I guess it should be an error. I copied the code from the service==null > test which only logs a warning. Why is that not an error also? A strict reading of BundleContext.getService(ServiceReference<S>) indicates the null case should result in an ERROR also: <javadoc> If the service object returned by the {@code ServiceFactory} object is {@code null}, not an {@code instanceof} all the classes named when the service was registered or the {@code ServiceFactory} object throws an exception, {@code null} is returned and a Framework event of type {@link FrameworkEvent#ERROR} containing a {@link ServiceException} describing the error is fired. </javadoc> Perhaps we thought a null case was not really an ERROR on the ServiceFactory's part. The ServiceFactory may have decided to blacklist some bundle and not grant it access to its own service. Should we really log an error in that case?
(In reply to comment #7) > A strict reading of BundleContext.getService(ServiceReference<S>) indicates the > null case should result in an ERROR also: > I guess all three tests should publish an ERROR per the javadoc. Note, I just redid that text for OSGi 4.3 so it does differ from 4.2. <4.2 javadoc> If the service object returned by the ServiceFactory object is not an instanceof all the classes named when the service was registered or the ServiceFactory object throws an exception, null is returned and a Framework event of type FrameworkEvent.ERROR containing a ServiceException describing the error is fired. </4.2 javadoc> The 4.2 text only referred to the instanceof test. I updated the javadoc for 4.3 due to OSGi-bug 1781.
I agree based on the new javadoc interpretation that ERROR should be used in all three cases. But I still wonder if a factory returning null really is an Error condition or if a Warning is more appropriate in that case.
(In reply to comment #9) > I agree based on the new javadoc interpretation that ERROR should be used in > all three cases. But I still wonder if a factory returning null really is an > Error condition or if a Warning is more appropriate in that case. The contract for ServiceFactory.getService is to return a service object. Null is not a service object but it is a choice by the factory to decline to return a service object. We could update the javadoc to declare returning null to be a warning, while an exception or recursion or an object not implementing all the named types is an error.
