ServiceLocator can't find an implementation in common-pool workers

[timsazon]

Hi, I've noticed that we can't read/create a token in a worker from ForkJoinPool.commonPool() in Spring's uber-jar application because ServiceLocator can't find an implementation for that (looks like there is a problem with context class loader related to the thread).

Example (it will throw UnavailableImplementationException for Serializer.class):

ForkJoinPool.commonPool()
        .execute(() -> Jwts.builder()
                .setClaims(Map.of(Claims.SUBJECT, "subject"))
                .compact());

Currently, I have a workaround to that by providing my own serializer (like this https://github.com/jwtk/jjwt#jackson-json-processor).

[lhazlewood]

Interesting. Do you have a simple test case that would allow us to re-create this? Or even a minimal set of code to get us started?

Also, have you seen this?

https://stackoverflow.com/questions/49113207/completablefuture-forkjoinpool-set-class-loader

(specifically the part about not using ForkJoinPool and using an Executor or one of Spring's native constructs?)

I wonder if that or something similar could help.

cc @bdemers

[timsazon]

Thank you for the link!

Unfortunately, I can't get rid of ForkJoinPool in my case because of it in use by Caffeine's cache loader (I'll check the configuration).

Btw, I made an example app: https://github.com/timsazon/jjwt-568

[bdemers]

Hey @timsazon thanks for putting together a minimal example, that made things MUCH easier.

For a short term workaround you could do something like this:

    private static Supplier<String> tokenSupplier() {
        return () -> {
            ClassLoader originalClassloader = Thread.currentThread().getContextClassLoader();
            try {
                Thread.currentThread().setContextClassLoader(Jwts.class.getClassLoader());
                return Jwts.builder()
                        .setClaims(Map.of(Claims.SUBJECT, "subject"))
                        .compact();
            } finally {
                Thread.currentThread().setContextClassLoader(originalClassloader);
            }
        };
    }

Basically just setting the contextClassLoader to something with JJWTs classes.

(or if you can configure the thread pool as mentioned in that stackoverflow post

@lhazlewood
We could set the classloader in Services: https://github.com/jwtk/jjwt/blob/master/impl/src/main/java/io/jsonwebtoken/impl/lang/Services.java#L71
But, we don't actually know which classloader to use (in the case of a more complex classloader setup)

We could also try to fall back, something like this in Services:

    public static <T> T loadFirst(Class<T> spi) {
        Assert.notNull(spi, "Parameter 'spi' must not be null.");
        ServiceLoader<T> serviceLoader = ServiceLoader.load(spi);
        if (serviceLoader.iterator().hasNext()) {
            return serviceLoader.iterator().next();
        } else {
            
            // failed to lookup implementation in current classloader
            // try again with this classes classloader
            try {
                Thread.currentThread().setContextClassLoader(Jwts.class.getClassLoader());
                serviceLoader = ServiceLoader.load(spi);
                if (serviceLoader.iterator().hasNext()) {
                    return serviceLoader.iterator().next();
                } else {
                    throw new UnavailableImplementationException(spi);
                }
            } finally {
                Thread.currentThread().setContextClassLoader(originalClassloader);
            }
        }
    }

(typed into github, so it needs to be cleaned up a little, just wanted to get the idea out)
This should work for most use cases, and anything else, we could recomend setting the Thread's classloader.

[lhazlewood]

@bdemers the old (non ServiceLocator approach) used the proper Classloading logic 🤦‍♂.

jjwt/api/src/main/java/io/jsonwebtoken/lang/Classes.java

Lines 72 to 96 in 111633f

	public static <T> Class<T> forName(String fqcn) throws UnknownClassException {
	Class clazz = THREAD_CL_ACCESSOR.loadClass(fqcn);
	if (clazz == null) {
	clazz = CLASS_CL_ACCESSOR.loadClass(fqcn);
	}
	if (clazz == null) {
	clazz = SYSTEM_CL_ACCESSOR.loadClass(fqcn);
	}
	if (clazz == null) {
	String msg = "Unable to load class named [" + fqcn + "] from the thread context, current, or " +
	"system/application ClassLoaders. All heuristics have been exhausted. Class could not be found.";
	if (fqcn != null && fqcn.startsWith("com.stormpath.sdk.impl")) {
	msg += " Have you remembered to include the stormpath-sdk-impl .jar in your runtime classpath?";
	}
	throw new UnknownClassException(msg);
	}
	return clazz;
	}

I think we need to revert to the old approach - not a wholesale reversion - just the bare minimum to use the proper classloader.

Perhaps we replace ServiceLoader.load(spi) with our own implementation that looks for a META-INF/services/<spiClass> file and then calls Classes.java ?

[lhazlewood]

Hrm - maybe another approach is to use this method:

https://docs.oracle.com/javase/7/docs/api/java/util/ServiceLoader.html#load(java.lang.Class,%20java.lang.ClassLoader)

And implement our own (static singleton) Classloader instance that just delegates to the ClassLoader cascade logic found in Classes.forName.

That static singleton would be created once and we'd change Services.java lines 43 and 71 from:
ServiceLoader.load(spi);
to:
ServiceLoader.load(spi, CASCADING_CLASSLOADER);
(or similar).

Thoughts?

[lhazlewood]

I also want to say thank you to @timsazon for the sample project! That's so helpful!

[bdemers]

I think parsing the META-INF/services/* is out, as you can also load services from a Java 9+ module.

I'm attempting to test, iterating over the three classloaders (similar to the Classes util).
It might turn into a mocking nightmare... However, your idea of the CascadingClassloader might be the solution 🤔 (or something similar).

[bdemers]

@lhazlewood, I didn't use a classloader, but I did mimic the "accessor" model that Classes uses. Let's move the implementation discussion over to #574

(I just tested this out on @timsazon's sample project)