Service Loader does not work in Spring Boot

[PRIESt512]

Hi, definition Decoder and Encoder are not defined in the system.

private List<S> loadProviders() {
    List<S> providers = new ArrayList<>();
    ServiceLoader.load(service, Thread.currentThread().getContextClassLoader()).stream()
        .forEach(p -> addProviderLenient(p, providers));
    return Collections.unmodifiableList(providers);
  }

change to "Thread.currentThread().getContextClassLoader()" solves the problem

P.S. - https://docs.spring.io/spring-boot/docs/current/reference/html/executable-jar.html#executable-jar.restrictions:

System classLoader: Launched applications should use Thread.getContextClassLoader() when loading classes (most libraries and frameworks do so by default). Trying to load nested jar classes with ClassLoader.getSystemClassLoader() fails. java.util.Logging always uses the system classloader. For this reason, you should consider a different logging implementation.

locally tested, the problem disappeared

[mizosoft]

Hi @PRIESt512. Thanks for raising an issue & for the link!

I've also come across https://stackoverflow.com/a/36228195, which made me wonder if the context classloader might get weird if threads are being pooled. This can cause classloader leaks across deployments. Also considering that the provider list is kept statically.

I'm thinking if the classloader that loaded BodyAdapterFinder (and DecoderFactoryFinder, which also uses ServiceCache) could be used instead a s a parameter to ServiceCache. This ensures the classloader that loaded the finder class is the one that ends up loading the provider classes. Though, I'm not sure if that'll work. I'll see if I can test this soon as I'm pretty busy these couple of weeks. You can submit a PR if you want!

[PRIESt512]

I was also not sure about the ideality of this idea. For this reason, I didn’t make a pull request right away. I have a few days off from work in the near future. I will try to study this issue and conduct experiments. If anything happens, I will share the results and make a pull request

[laxika]

Any plans to fix this?

I have the same problem while adding Methanol to a Spring Boot app. Included methanol-jackson as a dependency, but had to use auto-service to register the encoder and decoder for Jackson. Otherwise, I got an error stating that no supported Decoder was found.

[mizosoft]

@laxika It seems that the solution is to pass Thread.getContextClassLoader() instead of Class.getSystemClassLoader() here. I was concerned this might cause unintended class-leaks in container environments (as threads (and their associated classloaders) can outlive the application and its libraries, which is not good). I though of using the classloader that loaded the library (service.getClassLoader()), not sure why I haven't tested it though. I will see if that works and submit a fix shortly.

Also, for reference, where you installing the jackson decoder as a module or from the classpath? Do you have any idea why auto-service would work? Considering it only writes the META-INF/services/... files you'd otherwise have to write manually.

[laxika]

@mizosoft Well, to make this solution work, I would need to fork the project and build my custom jar for it right? I saw no way to override ServiceCache or ServiceLoader. To be honest I really want to avoid that.

Tried the deploy my JAR to a test server then noticed that auto-service was not working there. However, it is working in the IDE (IntelliJ clicking on run). I think it works because in the IDE the classpath is different.

Also, for reference, where you installing the jackson decoder as a module or from the classpath?

I have declared it as a dependency in Gradle. So from the classpath.

Ended up creating my own decoder with JacksonAdapterFactory.createDecoder() and using it as a BodyHandler. Luckily the service that is being called always returns a JSON and is controlled by me, so I had the creative freedom to do something like this, but it is far from being ideal that's for sure (high coupling, etc). It will work until this is not getting fixed in one way or another. :)

HttpResponse<StartWorkUnitResponse> startWorkUnitHttpResponse = httpClient.send(httpRequest,
        info -> decoder.toObject(new TypeRef<>() {}, MediaType.APPLICATION_JSON));

[mizosoft]

@laxika No need to override. I've locally tested passing service.getClassLoader() to ServiceLoader here and it seems to be working. I think the main problem is due to the somewhat 'custom' format springboot's runnable jar uses (it nests libraries within the jar). This entails having to use their special classloader to locate these nested libraries (in that case methanol-jackson). Since this is the classloader that loaded methanol in the first place, passing [Any methanol class].getClassLoader() seems to work.

Tricky part is testing in CI. Since the problem seems to only affect the runnable jar (that's why running from IntelliJ works), it has to be tested by invoking it from JUnit through the command line. Not sure if there's a standard way to do this though. Anyhow, I'll try to submit a PR shortly!

[chadselph]

I'm not 100% sure this was fixed; I'm on 1.6.0 and for me it works from intellij but when running from the jar I get

java.lang.UnsupportedOperationException: unsupported encoding: gzip
    at com.github.mizosoft.methanol.MoreBodyHandlers$DecodingHandler.lambda$apply$0(MoreBodyHandlers.java:259)
    at java.util.Optional.orElseThrow
    at com.github.mizosoft.methanol.MoreBodyHandlers$DecodingHandler.apply(MoreBodyHandlers.java:259)
    at jdk.internal.net.http.Stream.readBodyAsync
    at jdk.internal.net.http.Exchange.readBodyAsync
    at jdk.internal.net.http.MultiExchange.lambda$responseAsync0$4

[mizosoft]

@chadselph The fix is pending for the 1.7.0 release, which I'll be able to finally cut this weekend. 1.6.0 is still affected. In the meantime, you can pull in 1.7.0-SNAPSHOT as follows:

repositories {
  maven {
    url 'https://oss.sonatype.org/content/repositories/snapshots'
  }
}

dependencies {
  implementation 'com.github.mizosoft.methanol:methanol:1.7.0-SNAPSHOT'
}

[chadselph]

Sorry, I should have checked that!