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

[Gradle] Use platform classloader as parent in K2Native isolated class loader #5298

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

danysantiago
Copy link
Contributor

Fixes KT-65761

@danysantiago danysantiago marked this pull request as draft May 2, 2024 14:57
@danysantiago
Copy link
Contributor Author

Moving to draft... I was further testing and it seems ClassLoader.getPlatformClassLoader() is not available.? Is the JDK used to compile the plugin at least 9 or greater? I wouldn't like to use ClassLoader. getSystemClassLoader() even though that would also solve the issue...

@danysantiago danysantiago marked this pull request as ready for review May 2, 2024 17:16
@danysantiago
Copy link
Contributor Author

danysantiago commented May 2, 2024

Yup, the plugin is jvm target 8 so ClassLoader.getPlatformClassLoader() is not available (added in Java 9), I opted for removing the second null param of URLClassLoader so it uses the default parent, which should be the system class loader.

I guess the question is, is that OK? It sort of defeats the purpose of an 'isolated' class loader, not entirely but to some degree.

@dkrasnoff
Copy link
Contributor

dkrasnoff commented May 6, 2024

I guess the question is, is that OK? It sort of defeats the purpose of an 'isolated' class loader, not entirely but to some degree.

I'll run this branch against our integration tests to check that it is OK.

But to merge this PR you need to add an integration test for you fix. Here is readme how to do that.

@danysantiago
Copy link
Contributor Author

Added an integration test, thanks for the hints @dkrasnoff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants