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

Error loading NumberSystem ServiceProvider with multiple ClassLoaders #383

Closed
etatara opened this issue Sep 22, 2022 · 5 comments
Closed
Assignees
Milestone

Comments

@etatara
Copy link

etatara commented Sep 22, 2022

The Calculus class makes calls to ServiceLoader.load(NumberSystem.class) that will fail when using multiple ClassLoaders and Calculus.class is loaded using a ClassLoader other than the default ClassLoader. I suggest providing the additional ClassLoader argument that will use whatever ClassLoader actually loaded the Calculus class:

ServiceLoader<NumberSystem> loader = ServiceLoader.load(NumberSystem.class, Calculus.class.getClassLoader());
@keilw
Copy link
Member

keilw commented Sep 23, 2022

Thanks, is there a scenario to replicate that, e.g. in an app server or other cases with multiple class loaders?

@blackdrag
Copy link

ServiceLoader.load uses the context class loader. The context class loader usually is a child or identical to the class loader used as default.

In Open Webstart the webstart application is loaded by its own class loader, which is not the default. The context class loader is set, but in the case of the application, that did bring me here, there are a lot of libraries involved. Which means some library somewhere is spawning a thread, which then tries to load (in the end) DefaultNumberSystem and fails to do so.

In our case if you would not try only the context class loader, but also the loader of NumberSystem (NumberSystem.class.getClassLoader()) the issue would be resolved.

@evanjs
Copy link

evanjs commented Nov 6, 2022

Hey, so we have been stuck witth this problem for a while now, but have not had the time to make a PR.
This completely breaks things like Swagger, so we had to patch locally and repeat with several sister libraries.

It's been a while since I've dealt with the issue, but I believe it only applies to the Gradle plugin (swagger-gradle), not the Maven plugin.

As mentioned in the original post, the solution appears to be fairly simple, potential side effects notwithstanding.

With this patch applied, Swagger/OpenAPI 3 is able to generate documentation and properly process annotations with Unit-based types.

ClassLoader Patch
diff --git a/src/main/java/tech/units/indriya/function/Calculus.java b/src/main/java/tech/units/indriya/function/Calculus.java
index 89bd23c3..5688edf4 100644
--- a/src/main/java/tech/units/indriya/function/Calculus.java
+++ b/src/main/java/tech/units/indriya/function/Calculus.java
@@ -75,10 +75,8 @@ public final class Calculus {
      */
     public static List<NumberSystem> getAvailableNumberSystems() {
         List<NumberSystem> systems = new ArrayList<>();
-        ServiceLoader<NumberSystem> loader = ServiceLoader.load(NumberSystem.class);
-        loader.forEach(NumberSystem -> {
-            systems.add(NumberSystem);
-        });
+        ServiceLoader<NumberSystem> loader = ServiceLoader.load(NumberSystem.class, NumberSystem.class.getClassLoader());
+        loader.forEach(systems::add);
         return systems;
     }

@@ -107,7 +105,7 @@ public final class Calculus {
      * Returns the given {@link NumberSystem} used for Number arithmetic by (class) name.
      */
     public static NumberSystem getNumberSystem(String name) {
-        final ServiceLoader<NumberSystem> loader = ServiceLoader.load(NumberSystem.class);
+        final ServiceLoader<NumberSystem> loader = ServiceLoader.load(NumberSystem.class, NumberSystem.class.getClassLoader());
         final Iterator<NumberSystem> it = loader.iterator();
         while (it.hasNext()) {
             NumberSystem provider = it.next();

Thanks, is there a scenario to replicate that, e.g. in an app server or other cases with multiple class loaders?

@keilw We are using this library in a private project, but if I understand correctly, you should be able to reproduce this issue if you try to generate documentation for a basic Gradle-based Swagger/OpenAPI 3 project.

Something like specifying a Unit-based type as an @ApiOperation's response like SampleData.class is here should be enough to prevent Swagger's resolve task from running, which then renders Swagger unusable.


Related issue for swagger-gradle-plugin: swagger-api/swagger-core#3726


cc @addict3d

@keilw
Copy link
Member

keilw commented Jun 7, 2023

This is applied, thanks @evanjs for the patch. Could everyone (also @addict3d, maybe @andi-huber) check it out in the 2.2-SNAPSHOT build of Indriya?
Changing the status to "in Review", if the solution works feel free to close it.

@evanjs
Copy link

evanjs commented Jun 19, 2023

@keilw indeed, the latest SNAPSHOT appears to work without issue, and I am able to generate OpenAPI documentation (with our project that depends on Indriya) utilizing the Swagger Gradle plugin

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

No branches or pull requests

4 participants