eclipse-equinox / equinox

equinox
Eclipse Public License 2.0
33 stars 65 forks source link

LinkageError error with multiple Guava versions and Require-Bundle usage #304

Closed guw closed 1 year ago

guw commented 1 year ago

After installing the latest M2E update (2.4.0) some of my plug-ins started failing. It couldn't start a bundle anymore.

java.lang.ClassNotFoundException: An error occurred while automatically activating bundle com.salesforce.bazel.eclipse.core (1706).
...
Caused by: org.osgi.framework.BundleException: Exception in com.salesforce.bazel.eclipse.core.BazelCorePlugin.start() of bundle com.salesforce.bazel.eclipse.core.
    at org.eclipse.osgi.internal.framework.BundleContextImpl.startActivator(BundleContextImpl.java:839)
...
Caused by: java.lang.LinkageError: loader constraint violation: 
  loader org.eclipse.osgi.internal.loader.EquinoxClassLoader @7b0f5814 wants to load abstract class com.google.common.collect.ImmutableList. 
  A different abstract class with the same name was previously loaded by org.eclipse.osgi.internal.loader.EquinoxClassLoader @7a2fce12. 
  (com.google.common.collect.ImmutableList is in unnamed module of loader org.eclipse.osgi.internal.loader.EquinoxClassLoader @7a2fce12, parent loader 'platform')

Debugging the problem in the host OSGi console shows a wiring issue:

g! ss bazel
id  State       Bundle
1706    RESOLVED    com.salesforce.bazel.eclipse.core_2.0.0.v20230829-1225
1707    STARTING    com.salesforce.bazel.eclipse.ui_2.0.0.v20230829-1225
1708    RESOLVED    com.salesforce.bazel.importedsource_2.0.0.v20230829-1225
1709    RESOLVED    com.salesforce.bazel.logback_1.0.0.v20230829-1225
                Master=1713
1710    ACTIVE      com.salesforce.bazel.sdk_2.0.0.v20230829-1225

g! ss guava
id  State       Bundle
567 RESOLVED    com.google.guava_30.1.0.v20221112-0806
1003    RESOLVED    com.google.guava_31.1.0.jre
1004    RESOLVED    com.google.guava.failureaccess_1.0.1
1595    RESOLVED    com.github.ben-manes.caffeine.guava_3.1.6
1715    RESOLVED    com.google.guava_32.1.2.jre

g! which 1710 com.google.common.collect.ImmutableList
Loaded from: com.google.guava_32.1.2.jre [1715]

g! which 1708 com.google.common.collect.ImmutableList
Loaded from: com.google.guava_31.1.0.jre [1003]

g! which 1706 com.google.common.collect.ImmutableList
Loaded from: com.google.guava_32.1.2.jre [1715]

However, that wiring should be technically impossible because Equinox is supposed to discover that 1710 and 1708 and 1706 require all the same Guava version.

g!  bundle 1710
Module               osgi.identity; type="osgi.bundle"; version:Version="2.0.0.v20230829-1225"; osgi.identity="com.salesforce.bazel.sdk"; singleton:="true" [id=1710]
Location             reference:file:/Users/gwagenknecht/.p2/pool/plugins/com.salesforce.bazel.sdk_2.0.0.v20230829-1225
State                32
Version              2.0.0.v20230829-1225
Bundle                1710|Active     |    4|com.salesforce.bazel.sdk (2.0.0.v20230829-1225)
LastModified         1693313158089
BundleContext        org.eclipse.osgi.internal.framework.BundleContextImpl@523b5ed9
SymbolicName         com.salesforce.bazel.sdk
BundleId             1710
RegisteredServices   null
ServicesInUse        null
Headers               Automatic-Module-Name = com.salesforce.bazel.eclipse.model
 Build-Jdk-Spec = 17
 Bundle-ActivationPolicy = lazy
 Bundle-Activator = com.salesforce.bazel.sdk.BazelJavaSdkPlugin
 Bundle-ManifestVersion = 2
 Bundle-Name = Bazel Java SDK and IntelliJ Helpers
 Bundle-RequiredExecutionEnvironment = JavaSE-17
 Bundle-SymbolicName = com.salesforce.bazel.sdk;singleton:=true
 Bundle-Vendor = Bazel Eclipse Feature
 Bundle-Version = 2.0.0.v20230829-1225
 Created-By = Maven Archiver 3.6.0
 Eclipse-BundleShape = dir
 Eclipse-SourceReferences = scm:git:https://github.com/salesforce/bazel-eclipse.git;path="bundles/com.salesforce.bazel.sdk";commitId=bcdd29395b2c31cd655fd6f702bcae14702758b1
 Export-Package = com.salesforce.bazel.sdk,com.salesforce.bazel.sdk.aspects.intellij,com.salesforce.bazel.sdk.command,com.salesforce.bazel.sdk.command.shell,com.salesforce.bazel.sdk.init,com.salesforce.bazel.sdk.model,com.salesforce.bazel.sdk.projectview,com.salesforce.bazel.sdk.util
 Import-Package = com.google.common.annotations;version="31.1.0",com.google.common.collect;version="31.1.0",com.google.protobuf;version="3.22.0",org.slf4j;version="1.7.30"
 Manifest-Version = 1.0
 Require-Bundle = com.salesforce.bazel.importedsource;bundle-version="2.0.0";visibility:=reexport,org.eclipse.core.runtime;bundle-version="3.27.0"
g!  bundle 1708
Module               osgi.identity; type="osgi.bundle"; version:Version="2.0.0.v20230829-1225"; osgi.identity="com.salesforce.bazel.importedsource" [id=1708]
Location             reference:file:/Users/gwagenknecht/.p2/pool/plugins/com.salesforce.bazel.importedsource_2.0.0.v20230829-1225.jar
State                4
Version              2.0.0.v20230829-1225
Bundle                1708|Resolved   |    4|com.salesforce.bazel.importedsource (2.0.0.v20230829-1225)
LastModified         1693313158086
BundleContext        null
SymbolicName         com.salesforce.bazel.importedsource
BundleId             1708
RegisteredServices   null
ServicesInUse        null
Headers               Automatic-Module-Name = com.salesforce.bazel.eclipse.vendoredsource
 Build-Jdk-Spec = 17
 Bundle-ManifestVersion = 2
 Bundle-Name = Imported Dependencies
 Bundle-RequiredExecutionEnvironment = JavaSE-17
 Bundle-SymbolicName = com.salesforce.bazel.importedsource
 Bundle-Vendor = Bazel Eclipse Feature
 Bundle-Version = 2.0.0.v20230829-1225
 Created-By = Maven Archiver 3.6.0
 Eclipse-SourceReferences = scm:git:https://github.com/salesforce/bazel-eclipse.git;path="bundles/com.salesforce.bazel.importedsource";commitId=bcdd29395b2c31cd655fd6f702bcae14702758b1
 Export-Package = com.google.devtools.build.buildjar.proto,com.google.devtools.build.lib.buildeventstream,com.google.devtools.build.lib.collect.compacthashset,com.google.devtools.build.lib.collect.nestedset,com.google.devtools.build.lib.concurrent,com.google.devtools.build.lib.query2.proto.proto2api,com.google.devtools.build.lib.runtime.proto,com.google.devtools.build.lib.server,com.google.devtools.build.lib.view.proto,com.google.devtools.common.options.proto,com.google.devtools.intellij.aspect,com.google.devtools.intellij.ideinfo,com.google.idea.blaze.base.bazel,com.google.idea.blaze.base.command.buildresult,com.google.idea.blaze.base.command.info,com.google.idea.blaze.base.dependencies,com.google.idea.blaze.base.ideinfo,com.google.idea.blaze.base.model.primitives,com.google.idea.blaze.base.sync.workspace,com.google.idea.blaze.base.util,com.google.idea.blaze.java,com.google.idea.blaze.java.sync.importer,com.google.idea.blaze.java.sync.model,com.intellij.openapi.util.io;x-internal:=true,com.intellij.openapi.util.text;x-internal:=true,net.starlark.java.annot,net.starlark.java.annot.processor,net.starlark.java.cmd,net.starlark.java.eval,net.starlark.java.lib.json,net.starlark.java.spelling,net.starlark.java.syntax
 Import-Package = com.github.benmanes.caffeine;version="3.1.6",com.github.benmanes.caffeine.cache;version="3.1.6",com.github.benmanes.caffeine.guava;version="3.1.6",com.google.auto.value;resolution:=optional,com.google.common.annotations;version="31.1.0",com.google.common.base;version="31.1.0",com.google.common.cache;version="31.1.0",com.google.common.collect;version="31.1.0",com.google.common.io;version="31.1.0",com.google.common.primitives;version="31.1.0",com.google.common.util.concurrent;version="31.1.0",com.google.errorprone.annotations;resolution:=optional,com.google.protobuf;version="3.22.0",javax.annotation;version="3.0.2";resolution:=optional,javax.annotation.concurrent;version="3.0.2";resolution:=optional
 Manifest-Version = 1.0
 Require-Bundle = org.eclipse.core.runtime;bundle-version="3.27.0"

This is especially important because 1710 re-exports 1708. Additionally 1710 has a Require-Bundle on 1708. This should trigger Equniox to resolve to the same imports/version used by each bundle.

laeubi commented 1 year ago

Why do you think it is an equinox issue? You bundles most likely do not use correct use constrains (according to your debug output none at all) or wrong import ranges so nothing to blame to Equinox.

guw commented 1 year ago

Because of the use of Require-Bundle with visibility:=reexport between the two Equinox should never resolve to different import versions. The mandate to have uses directive is not really needed.

laeubi commented 1 year ago

The mandate to have uses directive is not really needed.

Can you please reference the relevant part of the spec about this? reexport only means to rexport the package, so if you don't have any usage constraints they are exported as is (without any constraints) so user of that package are exposed to classpath inconsistencies see

merks commented 1 year ago

This is why I whine endlessly (and to little avail) on cross-projects about duplicates and especially about duplicate guava versions because that in particular so often leads to wiring problems due to the fact that it so often percolates up into the APIs. Of course guava's approach of creating a new major version on a regular basis is a compounding factor. Interestingly, this is not even a wiring problem, but some other nasty flavor of the too many guavas problem long with nasty re-rexports.

Firstly, perhaps we (you) need to explain/understand why guava 31.1.0.jre even present in the installation? Something must require it to the exclusion of 32.1.2.jre. What might that be? Is that thing perhaps in the transitive dependencies of either of these two things? Unfortunately we can't even guess from the information provided here.

Of course as highly technical people, we all realize that we can't concretely investigate this problem without being able to reproduce it...

It might be faster to change com.google.common.collect;version="31.1.0" to 32.1.2 and see if the build even works. That might point the finger at the source of the problem...

laeubi commented 1 year ago

@merks with proper use-constrains the resolver would give a detailed technical answer already :-)

This is not a wiring problem because (as explained) the wire are all correct according to the given constraints (!), the problem now occurs exactly as described in the second article, a service (or even more likely static singleton) is accessed but the provider is using another classloader as the consumer.

So this is likely a result of using different bad style patterns in combination in descending order:

  1. re-exports for "convenience" (because otherwise the build would already have complained here)
  2. Usage of static singeltons (because OSGi would have checked classpath compatibility for services already)
  3. Using imported types in public API without declaring its use in the export-package (because OSGi resolver would have either complained or choose a different solution here)
guw commented 1 year ago

Can you please reference the relevant part of the spec about this.

I don't think this needs a spec. Lots of things in Equinox exist for backward compatibility to the old Eclipse days. This feels like one of those things which should just work.

Of course as highly technical people, we all realize that we can't concretely investigate this problem without being able to reproduce it...

@merks Agreed. That's why I keep my Eclipse instance open and use the host OSGi console to debug. I am not even sure this would reproduce cleanly.

I am trying several things to work around it. Calculating uses in PDE does not work. It's always stuck at this package. Seems to be caught in an endless loop.

why guava 31.1.0.jre even present in the installation?

🤷 It's not in the target platform of the project/plug-in. The plug-in is using the one from Orbit. It's likely some other extension which brought it into the installation. I don't really think you can avoid this at scale out there. Thus, having some logic in Equinox to be smarter here would help.

It might be faster to change com.google.common.collect;version="31.1.0" to 32.1.2 and see if the build even works. That might point the finger at the source of the problem...

Yepp, I'm also investigating this as a workaround as long as uses constraints cannot be computed by PDE.

laeubi commented 1 year ago

I don't think this needs a spec.

Please see the Mission Statement:

From a code point of view, Equinox is an implementation of the OSGi core framework specification, a set of bundles that implement various optional OSGi services and other infrastructure for running OSGi-based systems. The Equinox OSGi core framework implementation is used as the reference implementation and as such it implements all the required features of the latest OSGi core framework specification.

Lots of things in Equinox exist for backward compatibility to the old Eclipse days.

You can always use "good old Eclipse 2.0" for sure, but current Equinox mission is not that... so there is neither a test nor a TCK for this one obviously so whatever you expect is at least undefined behavior.

merks commented 1 year ago

@guw

Would you be able to share the installation profile? I.e., the latest .profile.gz in `/p2/org.eclipse.equinox.p2.engine/profileRegistry/.profile/*.profile.gz`. I might be able to do some more detailed analysis of the IUs and their dependencies from that information because the profile is effectively a type of metadata repository.

guw commented 1 year ago

@merks Thanks Ed! I'd rather avoid you spending time on this analysis. I do believe Uses info would be best here but I'm having trouble calculating it.

merks commented 1 year ago

I'm really rather curious ; goodness knows when the next time a problem like this shows up. I just tested that it's really easy to use the CBI Aggregation Analyzer Editor to analyze profile. I understand if there are reasons you can't share the profile though; that's up to you...

merks commented 1 year ago

So the caffine thing strictly requires 31.x.

image

These bundles can resolve to the 32.x because there is no constraint to the contrary:

image

This bundle shows the possible resolutions from the other end:

image

I expect that OSGi keeps the wiring consistent here.

And then finally this bundle is importing guava packages as well as receiving/seeing them from its bundle requirement via the exports of that required bundle:

image

So it does indeed seem as if it's the case that if "something" was smart enough to wire this SDK bundle to guava 31.x instead of guava 32.x there would be no class loader constraint violation. But nothing knows that the SDK bundle sees the guava version of the sourceimports bundle...

guw commented 1 year ago

Thanks @merks. Meanwhile, I added some Uses constraints and updated to the latest versions. Let's see if this mitigates the problem.

ben-manes commented 1 year ago

sorry for the pains. The versions were generated and appended by the bnd plugin, which inspected the maven dependency version. I believe the fix would be for me to specify it explicitly with a minimum version constraint, e.g. ;version=20.0, and bnd won't override that. I am not super familiar with osgi and use bnd + paxExam tests to try to be a good citizen. I'm not sure if bnd is being too rigid in its defaults or if it's mostly guava predating semver.

The caffeine-guava adapter provides an adapter for users who leaked Guava's cache externally or have a significant amount of code to migrate but don't want to bother yet. In the bazel-eclipse project, it has one usage which is internal and could trivially drop the adapter for shorter code. In this case I think that would be the right fix, and I'll review osgi metadata to be more forgiving.

HannesWell commented 1 year ago

This is why I whine endlessly (and to little avail) on cross-projects about duplicates and especially about duplicate guava versions because that in particular so often leads to wiring problems due to the fact that it so often percolates up into the APIs. Of course guava's approach of creating a new major version on a regular basis is a compounding factor. Interestingly, this is not even a wiring problem, but some other nasty flavor of the too many guavas problem long with nasty re-rexports.

Yes Guava can be really it problem. Xtext's re-exports of Guava prevented us updating our product to recent Eclipse versions since Eclipse 2022-12, with 2023-09 we can finally update because Xtext thankfully updated to the latest guice and guava. What also makes this problem very hard besides the re-exports is that even with 2023-09 we have a mixture of guava 32 from Maven-Central and Guava from 27 and 31 from Orbit (little sarcastic remark: good old Orbit ensures all use the same version). The problem is that Guava from Orbit has another structure and includes more packages than the original.

Regarding the relatively frequent major version in Guava: In the release notes Guava 32 it was announced that Removed @Beta from almost all APIs. For details, see the bottom of the release notes. At this point, it's probably simpler to look at a list of APIs that still are @Beta, such as [this list for guava-jre](https://guava.dev/releases/32.0.0-jre/api/docs/com/google/common/annotations/class-use/Beta.html). Most of the remaining @Beta APIs are in graph and hash. So from an OSGi point of view it might be reasonable to start decoupling the bundle from the package version and control the package version with corresponding annotations plus using the bnd-baseline-maven-plugin to ensure that the packages are versioned in a correct SemVer matter. I have already thought about suggesting this, but didn't have the time for that yet.

merks commented 1 year ago

@guw

Any updates on your attempts at workarounds?

guw commented 1 year ago

Yes. I updated all versions to require the latest one and this forces resolution to be consistent. I also added Uses where possible. That got me past this issue.

merks commented 1 year ago

I think we might as well close this because I think this will be fixed some time with the range [hell-freezes-over,end-of-time). 😱