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

Configured class loader not used when loading services #690

Closed
marinier opened this issue Mar 7, 2024 · 2 comments · Fixed by #692
Closed

Configured class loader not used when loading services #690

marinier opened this issue Mar 7, 2024 · 2 comments · Fixed by #692
Labels
bug Something isn't working
Milestone

Comments

@marinier
Copy link
Contributor

marinier commented Mar 7, 2024

Describe the bug
When creating a SolverConfig you can set a class loader on it like this:

solverConfig.setClassLoader(this.getClass().getClassLoader());

This class loader eventually gets passed to ScoreDirectorFactoryFactory::decideMultipleScoreDirectorFactories, but it is not passed to the ServiceLoader.load call on line 69 of ScoreDirectorFactoryFactory.java. That is, instead of using the provided class loader, it ends up using the default class loader. In my application, this does not work -- the provided class loader needs to be used.

Expected behavior
Uses the class loaded provided by the SolverConfig when loading services.

Actual behavior
Uses the default class loader when loading services. (Specifically, ServiceLoader defaults to Thread.currentThread().getContextClassLoader().)

To Reproduce
Reproducing my problem where the service loader fails requires access to the specific environment I'm working in, which is not possible. However, you can easily check which class loader is being used by setting a breakpoint and stepping in the ServiceLoader.load call on line 69 of ScoreDirectorFactoryFactory.java.

Environment

Timefold Solver Version or Git ref:

1.7.0

Output of java -version:

openjdk version "17.0.10" 2024-01-16
OpenJDK Runtime Environment Temurin-17.0.10+7 (build 17.0.10+7)
OpenJDK 64-Bit Server VM Temurin-17.0.10+7 (build 17.0.10+7, mixed mode, sharing)

Output of uname -a or ver:

OS Name Microsoft Windows 11 Pro
Version 10.0.22631 Build 22631

Additional information

The fix should be very simple, as the class loader is already passed as an argument to ScoreDirectorFactoryFactory::decideMultipleScoreDirectorFactories. Simply change:

ServiceLoader.load(ScoreDirectorFactoryService.class)

to

ServiceLoader.load(ScoreDirectorFactoryService.class, classLoader)

I'm happy to provide a pull request if desired.

@marinier marinier added bug Something isn't working process/needs triage Requires initial assessment of validity, priority etc. labels Mar 7, 2024
@triceo
Copy link
Contributor

triceo commented Mar 7, 2024

Hello @marinier - PRs are welcome. Even if we eventually remove the service loader from constraint streams (as discussed here), it will still remain for enterprise edition, so this will still make sense.

@triceo triceo removed the process/needs triage Requires initial assessment of validity, priority etc. label Mar 7, 2024
@marinier
Copy link
Contributor Author

marinier commented Mar 7, 2024

Added a PR here: #692

@triceo triceo linked a pull request Mar 8, 2024 that will close this issue
@triceo triceo added this to the v1.8.0 milestone Mar 8, 2024
triceo pushed a commit that referenced this issue Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants