Use loom-compatibility, avoid hacking into JDK

[sideeffffect]

It was with great excitement that I saw Loom getting support in cask via

• feat: Add loom Support. #159

But there was one aspect of the implementation that I've been concerned about. Namely that the implementation needed to use reflection and hack into JDK, and to facilitate that nudges people to --add-opens java.base/java.lang=ALL-UNNAMED, thus losing the benefits of Java Modules encapsulation.

I researched a simpler way to achieve the goal. To that end, I've created a new pico library:

• https://github.com/sideeffffect/loom-compatibility

loom-compatibility tries to load and expose Loom functionality behind interface(s) or fails with an exception when running on a non-Loom JDK. When using it, the code in cask gets radically simplified.

I'd appreciate your feedback and review @He-Pin @lihaoyi

[He-Pin]

@sideeffffect How about if it's been opened, we use the named one, otherwise, we use the unnamed one, as apache/pekko#1734 did.

I have a PR pending here which will name the Carrier Thread too: apache/pekko#1724

[He-Pin]

But honestly, it's always better to have the thread named, which helps with thread dumps, I plan to add more helper methods to the cask itself, you can see, in the pekko pr, I named the carrier thread too, I want to port that to cask too.

[He-Pin]

I have quick-checked your implementation, thanks for sharing, but I think it's just using a lazy classloading trick with a decorator pattern, the current cask, the user can override the handlerExecutor and provide a Executors.newTask... too, and your code will show up in the stack trace.

[He-Pin]

I think ZIO/pekko/vert.x/kyo is using reflect or method handle too, so do you plan to pr there with your library？

[sideeffffect]

do you plan to pr there with your library?

That is indeed my (not so) secret plan 😈 😄

lazy classloading trick with a decorator pattern

Yes. Much shorter and less brittle than raw reflection, especially in combination with suppressing Java's module system.

[sideeffffect]

@He-Pin the threads are named, as you can see here.

[sideeffffect]

Regarding --add-opens java.base/java.lang=ALL-UNNAMED specifically, I don't think we can reasonably expect users to run with this. I suspect vast majority, if not literally all, will be not OK with this in production. So I wouldn't worry about tweaking things within JDK itself if it doesn't expose it via a normal API.

[He-Pin]

So how do you do this in your library, #164 ? I would like to see.

[sideeffffect]

how do you do this in your library, #164 ?

I don't. That is the point.

The package name itself jdk.internal.misc.CarrierThread tells you you shouldn't use it. It's jdk.internal. You're hacking into JDK, intentionally circumventing the encapsulation of the Java platform.

The whole point of https://github.com/sideeffffect/loom-compatibility and this PR is to find ways how to have Loom features without having to resort to hacking into JDK.

[He-Pin]

Then why the community is using Unsafe:), btw, it's safe to use, as my friend at oracle told me.

[sideeffffect]

You yourself have commented just a little while ago on an issue documenting how problematic it is to hack into JDK or use internal APIs

• Runtime code implementing lazy val should not use sun.misc.Unsafe on Java 9+ (JEP-471) scala/scala3#9013 (comment)

[He-Pin]

that's for unsafe , which will be an error in JDK 26, the code will be removed, a whole different story.

Will the Carrier thread be removed? no.

[He-Pin]

BTW, I always think lines of code is easier to maintain than a third part jar.

[lihaoyi]

Yeah I think for now let's evolve the code in-repo within com-lihaoyi/cask rather than pulling in a third-party jar file. Feel free to refactor or update the code in com-lihaoyi/cask, but I think introducing a repo/org/ownership boundary is not the right thing to do at this point in time

[sideeffffect]

introducing a repo/org/ownership boundary is not the right thing to do at this point in time

Fair enough.

The "repo" part is actually very practical. loom-compatibility is a Java library and although it's minuscule in size and doesn't have any external dependencies, it is quite peculiar: It needs to be compiled against Java 21, but generate bytecode for Java 8. It doesn't really fit into the CI matrix of cask.

But we can do something about the "org/ownership" part. I'd be happy to donate loom-compatibility to https://github.com/com-lihaoyi . Would you accept @lihaoyi ?