Java 11 and ForkJoinPool.commonPool() class loading issue

[hanson76]

I'm trying to migrate an application from Java 8 to java 11 and have found a problem with
ServiceLoader.load() that are being run from within ForkJoinPool.commonPool, that are used by
default when using CompletableFuture.*Async().

The problem is that the commonPool that is created has ClassLoaders$AppClassLoader as class loader
in Java 11 while the applications classes are loaded into LaunchedURLClassLoader.

The problem with this is that if the applications code uses a ServiceLoader.load() from within
a ForkJoinPool.commonPool thread it wont find any classes that are located in any of the .jar files
packaged inside the jar/war file. ServiceLoader.load() uses Thread.currentThread().getContextClassLoader() to load classes.

I've not figured out a workaround for this problem.
There is a way to configure a different thread factory for the ForkJoinPool.commonPool by setting
a system property, but that is not working with Spring boot. There is no way to add own classes
to the .jar file that spring-boot maven plugin creates. The class has to be on the application class path
and therefor need to be a .class inside the root .jar.

There are lot of opensource projects out there that are using ServiceLoader to inject implementations at runtime that will not work if triggered from a CompletableFuture chain.

This is with Spring boot 2.1.2.RELEASE

[wilkinsona]

This is out of Spring Boot's control. I also don't think it has anything specifically to do with Java 11; Java 11 has just introduced a change that has brought the problem to your attention. If the thread context class loader is being used to load classes, you'll need to ensure that the thread context class loader is set to the correct class loader.

There is no way to add own classes to the .jar file that spring-boot maven plugin creates.

While it's more difficult than we'd like, it is possible. Please see #6626.

[stoicflame]

@wilkinsona can I get some clarification on why this issue was closed as status: invalid?

This is out of Spring Boot's control.

It's not, though.... Am I missing something?

The numerous issues that have been linked here have demonstrated various workarounds. It seems like Spring Boot can provide an implementation of ForkJoinWorkerThreadFactory that sets the correct classloader on each thread, and set the java.util.concurrent.ForkJoinPool.common.threadFactory system property during an ApplicationStartingEvent (or even before), no? Would a PR be helpful to illustrate?

Closing this issue as status:invalid is effectively the same as saying that nobody can reliably use any of the CompletableFuture APIs or the Stream.parallel() methods (that are core to the SDK no less!) within a Spring Boot application. Is that inaccurate? Pretty please tell me that's not what you're saying. 🥺

[philwebb]

It seems like Spring Boot can provide an implementation of ForkJoinWorkerThreadFactory that sets the correct classloader on each thread

I guess you mean something like this class that JUnit provides? We certainly could provide something similar, although it's not much code.

and set the java.util.concurrent.ForkJoinPool.common.threadFactory system property during an ApplicationStartingEvent

This is where things get a little more complicated in my mind. I'm not sure that we should assume all users will want this setup. As you can also see from the linked issues, some of the existing fixes have been about configuring existing beans differently (e.g. in the HazelcastServerConfiguration).

Closing this issue as status:invalid is effectively the same as saying that nobody can reliably use any of the CompletableFuture APIs or the Stream.parallel() methods (that are core to the SDK no less!)

I think things are a little more nuanced than that. Users may or may not be able to use those APIs out-of-the-box depending on how they package and run their applications. They also have the option to set the java.util.concurrent.ForkJoinPool.common.threadFactory system property themselves.

Is that inaccurate? Pretty please tell me that's not what you're saying. 🥺

I'm not sure if this is meant as sarcasm or not, but it's not a particularly helpful comment. There are pros and cons to any change like this and we do need to consider them carefully.

I'll flag one for team discussion so we can consider your suggestion.

[stoicflame]

I'm not sure if this is meant as sarcasm or not, but it's not a particularly helpful comment.

It was cheeky, but sarcasm usually implies sharpness which I truly didn't intend. Anyway it fell short of the professionalism that you and your team deserve so I apologize for the unhelpful comment.

I guess you mean something like this class that JUnit provides?

Yes, something like that.

This is where things get a little more complicated in my mind. I'm not sure that we should assume all users will want this setup.

Even in the case with the HazelcastServerConfiguration a custom Spring Boot ForkJoinWorkerThreadFactory for the common pool wouldn't have hurt anything, right? I admit I might be missing something, but I can't think of any case where an app managed and configured by Spring Boot would prefer (or expect) the common pool threads to use the system classloader. Am I wrong about that?

They also have the option to set the java.util.concurrent.ForkJoinPool.common.threadFactory system property themselves.

I admit that option exists, but given how hard (and kinda sketchy) it is to repackage the Spring Boot jar/war to provide their custom thread factory on the system classpath, this really isn't a good option.

I'll flag one for team discussion so we can consider your suggestion.

Thank you, much appreciated.

If (as I hope) you decide to address this issue please let us know where we can track that work if it's somewhere different from this issue.

[wilkinsona]

For background, I believe that JDK-8172726 is the bug that resulted in the change in the JDK.

a custom Spring Boot ForkJoinWorkerThreadFactory for the common pool wouldn't have hurt anything, right?

I think it would.

If we change the TCCL of the ForkJoin common pool, we'd essentially be reverting the fix for JDK-8172726 and reintroducing the possibility of a memory leak. I don't think we can determine enough about how a Spring Boot application is being run to know when a memory leak would and would not occur.

have demonstrated various workarounds

I don't consider them to be workarounds. Without incurring the risk of introducing a memory leak, the ClassLoader that's being used has to be configured with knowledge of the lifecycles involved. It's only safe to configure a component to use a particular ClassLoader when you know that component will have a lifecycle that's the same as or shorter than that of the ClassLoader. We could do it in the fix for #24836 as Hazelcast's lifecycle is a known quantity but I don't think we can do it across the board, certainly not by default.

[raphaelLacerda]

@Bean
    public ForkJoinPool myForkJoinPool() {
        int threads = Runtime.getRuntime().availableProcessors();
        return new ForkJoinPool(threads, makeFactory("MyFactory"), null, false);
    }
    
    private ForkJoinPool.ForkJoinWorkerThreadFactory makeFactory(String prefix) {
        return pool -> {
            final ForkJoinWorkerThread worker = ForkJoinPool.defaultForkJoinWorkerThreadFactory.newThread(pool);
            worker.setName(prefix + worker.getPoolIndex());
            worker.setContextClassLoader(MainApp.class.getClassLoader());
            return worker;
        };
    }

The solution for me was to inject a forkJoinPool bean and run the parallel code with it