Reduce locking in HostClassLoader

[hubertp]

Pull Request Description

Using a neat trick described in #13219 (comment) to avoid large synchronization blocks.

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

• All code follows the
  Scala,
  Java,
  TypeScript,
  and
  Rust
  style guides. In case you are using a language not listed above, follow the Rust style guide.

[Akirathan]

This feels like something that could (should) be unit tested:

• Create an isolated HostClassLoader in the unit test
• Point it to some arbitrary jar archives / directories with classes
  • Scan from classpath system property, pass via sbt, ...
• Spawn multiple threads that will try to load same classes in parallel.

Or alternatively, move the scheduling/synchronization logic outside HostClassLoader and test it in isolation.

Let's try hard to ensure that we do not introduce hard-to-debug race conditions.

[enso-bot]

Hubert Plociniczak reports a new STANDUP for the provided date (2025-08-19):

Progress: Addressing some comments #13781, improving locking in HostClassLoader in #13807. Added draft of caching of all arguments (#13813) which appears to have improved startup but will need more testing. It should be finished by 2025-08-19.

Next Day: Next day I will be working on the #10525 task. Pick up work on reactive caches

[hubertp]

Well, that wasn't a clean merge. Looks like tests got stuck :/

[hubertp]

@JaroslavTulach checking if you are still OK with changes as I had to do a small modification and some time has passed.

[hubertp]

Let's try hard to ensure that we do not introduce hard-to-debug race conditions.

In theory, I agree. In practice this PR has been sitting delayed by that for a while as I didn't have an opportunity to do the refactoring. After a recent merge with develop our test suite has revealed some problems which are now fixed. I would argue we have sufficient enough coverage as-is for HostClassLoader.

[JaroslavTulach]

• I think the spirit of the change is still OK even after 4b8d305

Let's try hard to ensure that we do not introduce hard-to-debug race conditions.

• all the hard synchronization is left on ConcurrentHashMap and Future
  • e.g. I am not afraid of race conditions, but deadlocks could appear...
• but there is one thing we should test:
  • have two classes that depend on each other (referencing a static field of the other for example)
  • block when one thread is about to load the first class and another thread is about to load the other class
  • let the threads continue and see if they finish
• if the test passes then this change is likely correct

[hubertp]

• but there is one thing we should test:

Fair enough. Added a test that should in theory create such a race condition.

[JaroslavTulach]

Added a test that should in theory create such a race condition.

• the important thing to turn a theory into a practice is to block the two threads in the critical section
• that's done by 7e63f82
• once we have both classes "being classloaded at the same time"
• we need to create a dependency between them - done in 3a43d3f
• alas, now it deadlocks that means my original idea with a Map<classname, Future> was a bit too naive, sorry.
• on the other hand, not even regular AppClassLoader can handle such a wild classloading dependency as tested in b711de7
• so maybe we can remove my commits and integrate and hope normal classloading will be more sane than this test
• btw. the last case where I've seen this kind of problem for real was Avoid classloading deadlock by delaying computations of other passes #7943 and Deadlock in NI build wrt. classloading: "Image generator watchdog detected no activity" #7850

[hubertp]

I'm not sure the two examples are comparable.
The first fails because context never gets initialized - the usual TruffleSafepoint limitation we've encountered countless number of times: Ignore that. This appears to only show up when executing an individual test from test class.

   java.lang.Thread.State: WAITING (parking)
        at jdk.internal.misc.Unsafe.park([email protected]/Native Method)
        - parking to wait for  <0x000000028bbbe228> (a com.oracle.truffle.api.impl.ThreadLocalHandshake$Barrier)
        at java.util.concurrent.locks.LockSupport.park([email protected]/LockSupport.java:223)
...

Anyway, I'm leaning towards reverting your changes, as discussed offline.
The regular classloader test deadlock is, it seems, working according to the spec. Here is a good thread from SO.

[JaroslavTulach]

Anyway, I'm leaning towards reverting your changes, as discussed offline.

• OK, revert is reasonable
• Simpler way to revert is:

enso$ git checkout -f wip/hubert/hostclassloader-improvements
enso$ git revert 90b880a3ddfa6736602edd36d5d78235b58a6b51
enso$ git push -f

• but these individual commits will be gone when this PR is squashed, so it doesn't matter