[jira] [Comment Edited] (SLING-9210) Make DynamicClassloader behave deterministically in case several bundles export the same package
[ https://issues.apache.org/jira/browse/SLING-9210?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17061047#comment-17061047 ] Carsten Ziegeler edited comment on SLING-9210 at 3/17/20, 4:38 PM: --- The point is, the OSGi framework resolves bundles taking the whole class space into account; it uses export/import package statements together with uses constraints to try to get a consistent space. But this is only possible - if at all - if you know all the imports of the script at once and then its possible to resolve against the providing bundles. So its not about a missing version information on an import; its a) how we do the class loading (one class after the other) and b) we're not taking the existing class space into account (and uses constraints from exporting bundles). And on top of that, we assume that all scripts can happyily resolve in the same class space - which is not true in general either. Bottom line, this was never taken into account when designing the mechanism - it doesn't work today if multiple bundles provide the same packages and it will not by making it deterministic. I'm not against making it deterministic, but I also want to highlight that this does not solve the underlying problem - and this problem exists today without using SLING-9172 was (Author: cziegeler): The point is, the OSGi framework resolves bundles taking the whole class space into account; it uses export/import package statements together with uses constraints to try to get a consistent space. But this is only possible - if at all - if you know all the imports of the script at once and then its possible to resolve against the providing bundles. So its not about a missing version information on an import; its a) how we do the class loading (one class after the one) and b) we're not taking the existing class space into account (and uses constraints from exporting bundles). And on top of that, we assume that all scripts can happyily resolve in the same class space - which is not true in general either. Bottom line, this was never taken into account when designing the mechanism - it doesn't work today if multiple bundles provide the same packages and it will not by making it deterministic. I'm not against making it deterministic, but I also want to highlight that this does not solve the underlying problem - and this problem exists today without using SLING-9172 > Make DynamicClassloader behave deterministically in case several bundles > export the same package > > > Key: SLING-9210 > URL: https://issues.apache.org/jira/browse/SLING-9210 > Project: Sling > Issue Type: Improvement >Affects Versions: Commons ClassLoader 1.4.4 >Reporter: Konrad Windszus >Priority: Major > Fix For: Commons ClassLoader 1.4.6 > > > Currently in case several bundles export the same package (in different > versions) the first one is being picked in > https://github.com/apache/sling-org-apache-sling-commons-classloader/blob/dedb69a9fb0f8cb2ebf3050560311739b9f00ce4/src/main/java/org/apache/sling/commons/classloader/impl/PackageAdminClassLoader.java#L156, > but it is not described which one is the first one in > https://osgi.org/javadoc/r6/core/org/osgi/service/packageadmin/PackageAdmin.html#getExportedPackages(org.osgi.framework.Bundle). > Given that now it is possible to install the same bundle in multiple version > (SLING-9172 and FELIX-6232) a better, more predictable logic should be chosen. > I propose to always pick the highest exported version. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Comment Edited] (SLING-9210) Make DynamicClassloader behave deterministically in case several bundles export the same package
[ https://issues.apache.org/jira/browse/SLING-9210?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17061047#comment-17061047 ] Carsten Ziegeler edited comment on SLING-9210 at 3/17/20, 4:37 PM: --- The point is, the OSGi framework resolves bundles taking the whole class space into account; it uses export/import package statements together with uses constraints to try to get a consistent space. But this is only possible - if at all - if you know all the imports of the script at once and then its possible to resolve against the providing bundles. So its not about a missing version information on an import; its a) how we do the class loading (one class after the one) and b) we're not taking the existing class space into account (and uses constraints from exporting bundles). And on top of that, we assume that all scripts can happyily resolve in the same class space - which is not true in general either. Bottom line, this was never taken into account when designing the mechanism - it doesn't work today if multiple bundles provide the same packages and it will not by making it deterministic. I'm not against making it deterministic, but I also want to highlight that this does not solve the underlying problem - and this problem exists today without using SLING-9172 was (Author: cziegeler): The point is, the OSGi framework resolves bundles taking the whole class space into account; it uses export/import package statements together with uses constraints to try to get a consistent space. But this is only possible - if at all - if you now all the imports of the script at once and then its possible to resolve against the providing bundles. So its not about a missing version information on an import; its a) how we do the class loading (one class after the one) and b) we're not taking the existing class space into account (and uses constraints from exporting bundles). And on top of that, we assume that all scripts can happyily resolve in the same class space - which is not true in general either. Bottom line, this was never taken into account when designing the mechanism - it doesn't work today if multiple bundles provide the same packages and it will not by making it deterministic. I'm not against making it deterministic, but I also want to highlight that this does not solve the underlying problem - and this problem exists today without using SLING-9172 > Make DynamicClassloader behave deterministically in case several bundles > export the same package > > > Key: SLING-9210 > URL: https://issues.apache.org/jira/browse/SLING-9210 > Project: Sling > Issue Type: Improvement >Affects Versions: Commons ClassLoader 1.4.4 >Reporter: Konrad Windszus >Priority: Major > Fix For: Commons ClassLoader 1.4.6 > > > Currently in case several bundles export the same package (in different > versions) the first one is being picked in > https://github.com/apache/sling-org-apache-sling-commons-classloader/blob/dedb69a9fb0f8cb2ebf3050560311739b9f00ce4/src/main/java/org/apache/sling/commons/classloader/impl/PackageAdminClassLoader.java#L156, > but it is not described which one is the first one in > https://osgi.org/javadoc/r6/core/org/osgi/service/packageadmin/PackageAdmin.html#getExportedPackages(org.osgi.framework.Bundle). > Given that now it is possible to install the same bundle in multiple version > (SLING-9172 and FELIX-6232) a better, more predictable logic should be chosen. > I propose to always pick the highest exported version. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Comment Edited] (SLING-9210) Make DynamicClassloader behave deterministically in case several bundles export the same package
[ https://issues.apache.org/jira/browse/SLING-9210?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17061036#comment-17061036 ] Konrad Windszus edited comment on SLING-9210 at 3/17/20, 4:28 PM: -- I prefer determinism over non-determinism for several reasons, first and foremost for bug tracking reasons. I agree that picking the highest version may be wrong, but it is then at least consistently wrong for everyone having the same bundles installled! How does OSGi behave in case no specific import range is given for a bundle? Which version is picked preferably? As OSGi (non dynamic) bundle wiring may run into the same situation I guess there is somewhere a standard defined which bundle is used out of several matching candidates. Let us use the same mechanism within the dynamic class loader. Update: Isn't this the order being used by Felix: https://github.com/apache/felix-dev/blob/2e56c35922262965e5b656809e5690938ecb76f1/framework/src/main/java/org/apache/felix/framework/resolver/CandidateComparator.java#L30? Also Felix returns the highest version for https://github.com/apache/felix-dev/blob/2e56c35922262965e5b656809e5690938ecb76f1/framework/src/main/java/org/apache/felix/framework/PackageAdminImpl.java#L129 was (Author: kwin): I prefer determinism over non-determinism for several reasons, first and foremost for bug tracking reasons. I agree that picking the highest version may be wrong, but it is then at least consistently wrong for everyone having the same bundles installled! How does OSGi behave in case no specific import range is given for a bundle? Which version is picked preferably? As OSGi (non dynamic) bundle wiring may run into the same situation I guess there is somewhere a standard defined which bundle is used out of several matching candidates. Let us use the same mechanism within the dynamic class loader. Update: Isn't this the order being used by Felix: https://github.com/apache/felix-dev/blob/2e56c35922262965e5b656809e5690938ecb76f1/framework/src/main/java/org/apache/felix/framework/resolver/CandidateComparator.java#L30? > Make DynamicClassloader behave deterministically in case several bundles > export the same package > > > Key: SLING-9210 > URL: https://issues.apache.org/jira/browse/SLING-9210 > Project: Sling > Issue Type: Improvement >Affects Versions: Commons ClassLoader 1.4.4 >Reporter: Konrad Windszus >Priority: Major > Fix For: Commons ClassLoader 1.4.6 > > > Currently in case several bundles export the same package (in different > versions) the first one is being picked in > https://github.com/apache/sling-org-apache-sling-commons-classloader/blob/dedb69a9fb0f8cb2ebf3050560311739b9f00ce4/src/main/java/org/apache/sling/commons/classloader/impl/PackageAdminClassLoader.java#L156, > but it is not described which one is the first one in > https://osgi.org/javadoc/r6/core/org/osgi/service/packageadmin/PackageAdmin.html#getExportedPackages(org.osgi.framework.Bundle). > Given that now it is possible to install the same bundle in multiple version > (SLING-9172 and FELIX-6232) a better, more predictable logic should be chosen. > I propose to always pick the highest exported version. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Comment Edited] (SLING-9210) Make DynamicClassloader behave deterministically in case several bundles export the same package
[ https://issues.apache.org/jira/browse/SLING-9210?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17061036#comment-17061036 ] Konrad Windszus edited comment on SLING-9210 at 3/17/20, 4:24 PM: -- I prefer determinism over non-determinism for several reasons, first and foremost for bug tracking reasons. I agree that picking the highest version may be wrong, but it is then at least consistently wrong for everyone having the same bundles installled! How does OSGi behave in case no specific import range is given for a bundle? Which version is picked preferably? As OSGi (non dynamic) bundle wiring may run into the same situation I guess there is somewhere a standard defined which bundle is used out of several matching candidates. Let us use the same mechanism within the dynamic class loader. Update: Isn't this the order being used by Felix: https://github.com/apache/felix-dev/blob/2e56c35922262965e5b656809e5690938ecb76f1/framework/src/main/java/org/apache/felix/framework/resolver/CandidateComparator.java#L30? was (Author: kwin): I prefer determinism over non-determinism for several reasons, first and foremost for bug tracking reasons. I agree that picking the highest version may be wrong, but it is then at least consistently wrong for everyone having the same bundles installled! How does OSGi behave in case no specific import range is given for a bundle? Which version is picked preferably? As OSGi (non dynamic) bundle wiring may run into the same situation I guess there is somewhere a standard defined which bundle is used out of several matching candidates. Let us use the same mechanism within the dynamic class loader. > Make DynamicClassloader behave deterministically in case several bundles > export the same package > > > Key: SLING-9210 > URL: https://issues.apache.org/jira/browse/SLING-9210 > Project: Sling > Issue Type: Improvement >Affects Versions: Commons ClassLoader 1.4.4 >Reporter: Konrad Windszus >Priority: Major > Fix For: Commons ClassLoader 1.4.6 > > > Currently in case several bundles export the same package (in different > versions) the first one is being picked in > https://github.com/apache/sling-org-apache-sling-commons-classloader/blob/dedb69a9fb0f8cb2ebf3050560311739b9f00ce4/src/main/java/org/apache/sling/commons/classloader/impl/PackageAdminClassLoader.java#L156, > but it is not described which one is the first one in > https://osgi.org/javadoc/r6/core/org/osgi/service/packageadmin/PackageAdmin.html#getExportedPackages(org.osgi.framework.Bundle). > Given that now it is possible to install the same bundle in multiple version > (SLING-9172 and FELIX-6232) a better, more predictable logic should be chosen. > I propose to always pick the highest exported version. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Comment Edited] (SLING-9210) Make DynamicClassloader behave deterministically in case several bundles export the same package
[ https://issues.apache.org/jira/browse/SLING-9210?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17060936#comment-17060936 ] Konrad Windszus edited comment on SLING-9210 at 3/17/20, 2:38 PM: -- As https://osgi.org/javadoc/r6/core/org/osgi/service/packageadmin/PackageAdmin.html#getExportedPackages(org.osgi.framework.Bundle) is deprecated anyways, one should maybe leverage the new wiring spec (https://osgi.org/specification/osgi.core/7.0.0/framework.wiring.html) instead. Still one would need to sort the results to make it behave more predictable. [~cziegeler] WDYT? Does OSGi specify a particular order in case multiple bundles would satisfy all requirements? I couldn't find anything in https://osgi.org/specification/osgi.core/7.0.0/framework.module.html#framework.module.resolvingprocess. was (Author: kwin): As https://osgi.org/javadoc/r6/core/org/osgi/service/packageadmin/PackageAdmin.html#getExportedPackages(org.osgi.framework.Bundle) is deprecated anyways, one should maybe leverage the new wiring spec (https://osgi.org/specification/osgi.core/7.0.0/framework.wiring.html) instead. Still one would need to sort the results to make it behave more predictable. [~cziegeler] WDYT? > Make DynamicClassloader behave deterministically in case several bundles > export the same package > > > Key: SLING-9210 > URL: https://issues.apache.org/jira/browse/SLING-9210 > Project: Sling > Issue Type: Improvement >Affects Versions: Commons ClassLoader 1.4.4 >Reporter: Konrad Windszus >Priority: Major > Fix For: Commons ClassLoader 1.4.6 > > > Currently in case several bundles export the same package (in different > versions) the first one is being picked in > https://github.com/apache/sling-org-apache-sling-commons-classloader/blob/dedb69a9fb0f8cb2ebf3050560311739b9f00ce4/src/main/java/org/apache/sling/commons/classloader/impl/PackageAdminClassLoader.java#L156, > but it is not described which one is the first one in > https://osgi.org/javadoc/r6/core/org/osgi/service/packageadmin/PackageAdmin.html#getExportedPackages(org.osgi.framework.Bundle). > Given that now it is possible to install the same bundle in multiple version > (SLING-9172 and FELIX-6232) a better, more predictable logic should be chosen. > I propose to always pick the highest exported version. -- This message was sent by Atlassian Jira (v8.3.4#803005)
