kotlin icon indicating copy to clipboard operation
kotlin copied to clipboard

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

Open danysantiago opened this issue 10 months ago • 4 comments

Fixes KT-65761

danysantiago avatar May 02 '24 14:05 danysantiago

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 avatar May 02 '24 15:05 danysantiago

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.

danysantiago avatar May 02 '24 17:05 danysantiago

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.

dkrasnoff avatar May 06 '24 09:05 dkrasnoff

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

danysantiago avatar May 06 '24 16:05 danysantiago

@danysantiago Thanks for the contribution. It was merged with a change to use either the platform class loader or the bootstrap class loader depending on running JDK version. https://github.com/JetBrains/kotlin/compare/59cf97e245dfc6830e99d8cd050f9604a79463a9...2513befb5bfaf635d51705bb3e90abbee45868d8

ALikhachev avatar May 30 '24 19:05 ALikhachev

Great! Thanks for taking care of this @ALikhachev! 🙏🏼

danysantiago avatar May 31 '24 00:05 danysantiago