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
Java 11 ServiceLoader and ForkJoinPool.commonPool() problem #94
Comments
Thank you for reporting this! I'll have to think about a solution for this. |
I've done some more investigation and it looks to me as the ForkJorkPool.makeCommonPool() is executed at different times in Java 8 and java 9+. I will dig some more. Not sure if it's just a startup difference that I can work around or if it is classloader changes that creates this behavior. |
I've done some more digging around and managed to have the debugger stop in ForkJoin. java 8:
java 11:
The difference is the "ClassLoaders$AppClassLoader" in the "java11 -jar " example, that is the result when the call originate from the common ForkJoinPool. Trying to figure out why it does not happen in the java 8 example. The ForkJoinPool class has been refactored in java 9+. while Java 11 has
The difference is that the ForkJoinWorkerThread gest the ClassLoader.getSystemClassLoader()) in Java 11! |
I've also created an issuse for Spring Boot because I think that root cause might be the way Spring boot "shaded" jars work isn't supported in java11 |
Thank you for the investigation so far. 👍 I will look into spring-boot and JDK11 combination as well, as I have not used that combination yet. Do you have a testproject (zip or github) I can use to reproduce this issue? |
I've found a workaround but it is somewhat ugly. |
I've created a small test project showing the problem. can provide a working example later (probably early next week) |
I think a viable fix for this would be to still use the context classloader to lookup the The most simple solution I can think of is to keep a hard reference to the Serialization of the snapshot may then become an issue, but that is probably a more niche use-case than being compatible with the standard fork-join threadpool! |
It looks like the code in #95 works, I've built branch fix-94 locally and ran some test against it. |
Ops, looks Like I spoke too soon. I've just encountered a case were a createSnapshot is done within a ForkJoinPool thread. I have a feeling that there is no easy fix and we have to live with the ugly workaround in Spring Boot where you inject your own ThreadPoolFactory.
|
Noticed that the zip file I uploaded was more or less empty! Here is a new one that is working. |
Just out of curiosity, what is your use-case for taking a snapshot from inside the fork-join pool? My guess is, the more common scenario would be the other way round; propagating a context into a background thread. |
I think I'll merge #95 either way, but reopen this issue when it auto-closes so we can discuss further. |
I'm not entirely sure why it happens in this case. Our applications are async and nonblocking and it Might be that we have some code that is doing something strange, but it happens in more than one of our applications. |
I just noticed that |
The test project you provided no longer logs problems about empty snapshots or missing I will not put effort into the corner-case of propagating context changes from one fork-join thread into another, I'd rather have current (predictable) behaviour where an initial caller's Please let me know if you run into that situation though, maybe we can find a more elegant workaround by then... |
Yes, #96 looks better. I still have the corner case but I think I know why. It's when we use CompletableFuture.completedFuture() that we get the corner case. My guess is that we somewhere manage to do a thenApplyAsync() with a ForkJoinPool that is not wrapped because we created a CompletableFuture instead of ContextAwareCompletableFuture at the start. I might provide a pull request later today that adds the following static methods to ContextAwareCompletableFuture (those exists in CompletableFuture for Java 11) static ContextAwareCompletableFuturecompletedFuture(U value) static ContextAwareCompletableFuturefailedFuture(Throwable ex) |
Good to hear that it looks better. I will merge this and push out a new release (at the latest end of this week) If you have more to share about your corner cases, please feel free to add a new issue. Thank you very much @hanson76 for helping making this library better! |
Version Thanks again! |
Expected Behavior
snapshot.reactivate() works from withing ForkJoinPool.commonPool() threads
Actual Behavior
snapshot.reactivate() throws exception:
Context manager "com.wirelesscar.componentbase.RequestContextManager" not found in service loader! Cannot reactivate: com.....
Java 9 has introduced more class loaders. It now exists three class loaders.
Bootstrap, Platform and System. https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/ClassLoader.html
ForkJoinPool.commonPool is created in a static block in ForkJoinPool with the Platform classloader,
that Thread pool will have Platform as context class loader.
Spring boot has it's LaunchedURLClassLoader that inherits from System class loader.
The cache in PriorityServiceLoader is using Thread.currentThread().getContextClassLoader() to load classes. This is not working with currentThread == commonPool.
I've verified that it works if I change line 108 in PriorityServiceLoader to read
final ClassLoader contextClassLoader = serviceType.getClassLoader();
I'm thinking of adding a check that does something similair to
contextClassLoader = Thread.currentThread().getContextClassLoader();
if (contextClassLoader.getName().contains("Platform") {
contextClassLoader = serviceType.getClassLoader();
}
My guess is that Jakarta EE etc still need to use the context class loader...
Steps to Reproduce the Problem
Specifications
Version: 1.0.3
Platform: Linux
Java version:
openjdk version "11" 2018-09-25
OpenJDK Runtime Environment 18.9 (build 11+28)
OpenJDK 64-Bit Server VM 18.9 (build 11+28, mixed mode)
--
(If open source) Project link:
The text was updated successfully, but these errors were encountered: