Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ContextFinder can’t locate implementation when its classloader doesn’t match current thread’s context classloader #99

Open
eager opened this issue Jun 18, 2019 · 7 comments

Comments

@eager
Copy link

@eager eager commented Jun 18, 2019

This is related to #78—the stack trace is the same, and it’s because Java 11 no longer includes JAXB—but the root seems to be slightly different.

We are using JAXB 2.3.2 in a Solr 7 plugin on JDK 11. Solr plugins use a custom classloader (SolrResourceLoader). This classloader is not the same the thread’s context classloader, which is Jetty’s WebAppClassLoader. I should note, that Solr’s plugin loader does set Jetty’s classloader as its parent.

Because ContextFinder#newInstance looks up the default factory class using the thread’s classloader, it’s unable to locate the implementation JAR, if it’s installed in the Solr plugin directory. This issue can be worked around by installing the JAXB implementation JARs in Jetty’s lib.

When debugging this issue, calling getClass().getClassLoader() within ContextFinder returns SolrResourceLoader, so it would be possible to locate the implementation that way.

@damienburke

This comment has been minimized.

Copy link

@damienburke damienburke commented Jul 8, 2019

I'm also seeing this behaviour when attempting to work with a maven plugin to depends on JAXB

@CardContact

This comment has been minimized.

Copy link

@CardContact CardContact commented Jul 31, 2019

Same issue here with Equinox on Java 11: jaxb-api 2.3.2 does not find com.sun.xml.bind.v2.ContextFactory provided by the jaxb-osgi bundle, as the context class loader is used by default.

Using the JABXContext.newInstance() method with the bundle class loader solves the problem.

Our issue is however, that we use the JABXContext.newInstance(Class...) variant to pass a list of ObjectFactories. Unfortunately there is no signature of the methods that allows passing a class loader.

I can see two solutions:

  1. Add a method signature that allows passing a classloader
  2. Implement bundle resolving for Equinox similar to the mechanism already implemented for glassfish

I guess the first solution would be more versatile, as class loading problems are common in container implementations.

@CardContact

This comment has been minimized.

Copy link

@CardContact CardContact commented Aug 3, 2019

The following patch makes jaxb-ri an optional import, so OSGI can wire jaxb-api to com.sun.xml.bind:jaxb-osgi:2.3.2.

With this patch the context class loader in jaxb-api is able to resolve com.sun.xml.bind.v2. No need to specify the class loader in the call to JABXContext.newInstance(). Tested with latest Equinox.

0001-Make-JAXB-RI-an-optional-import-to-allow-class-loadi.txt

@kriegfrj

This comment has been minimized.

Copy link

@kriegfrj kriegfrj commented Aug 15, 2019

I've just hit this same issue with Equinox. I was going to raise an issue about adding the optional import to the manifest and found that this suggestion had already been made.

More generally, across all of the Jakarta modules, it would be good if there was an audit done to find everywhere the apis attempt to load a default implementation, and add those packages as optional imports to their respective bundles (if they are not already).

@lukasj

This comment has been minimized.

Copy link
Member

@lukasj lukasj commented Aug 15, 2019

IMHO adding optional import is not the way to go as there exist multiple JAXB implementations, so we'd end up adding an import for each of them.

@kriegfrj

This comment has been minimized.

Copy link

@kriegfrj kriegfrj commented Aug 15, 2019

@lukasj , thank you for the response.

I realise that there are multiple implementations of JAXB and I agree it is not right to explicitly import them all. Other mechanisms will take care of those cases.

However, the default fallback should be considered as a special case and it might be worth using the optional import for that class:

static final String DEFAULT_FACTORY_CLASS = "com.sun.xml.internal.bind.v2.ContextFactory".toString();

@pieterjanpintens

This comment has been minimized.

Copy link

@pieterjanpintens pieterjanpintens commented Dec 23, 2019

For reference this is how we work around this in our OSGi setup.
We create a fragment bundle on the API and add a class like this.

package org.glassfish.hk2.osgiresourcelocator;

import java.util.Collections;
import java.util.logging.Level;
import java.util.logging.Logger;

public class ServiceLoader
{
	private static Logger kLOG = Logger.getLogger("org.glassfish.hk2.osgiresourcelocator.ServiceLoader");


	public static <T> Iterable<Class> lookupProviderClasses(final Class<T> serviceClass)
	{
		if (serviceClass.equals(javax.xml.bind.JAXBContext.class))
		{
			return Collections.singletonList(com.sun.xml.bind.v2.ContextFactory.class);
		}
		kLOG.log(Level.SEVERE, "Failed to load 'com.sun.xml.bind.v2.ContextFactory', jaxb will not be available");
		return null;
	}

}

You will need to add the jaxb osgi plugin as dependency to this fragment.
This will make sure that jaxb is loaded properly, it does not depend on the context classloader.
Looking forward for a proper fix though

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.