classloader concurrency

[jurgenvinju]

• fix concurrency and recursive re-entrancy issues in SourceLocationClassLoader
  • parent delegation model was not implemented correctly
  • two phase cache prevents recursive concurrent access to the cache
  • resource lookup stack is now threadlocal
  • added call to ClassLoader.resolve to increase class lookup efficiency without changing semantics

This is meant to fix the persistent exceptions thrown by the tutor when compiling
rascal-shell blocks that define syntax definitions. What happens is that classes
are being loaded by four concurrent Evaluator instances:

1. the one running the tutor code
2. the one running the parser generator of 1.
3. one or more TutorCommandExecutor instances
4. and their parser generator instances.

All of these share the same parent classloader, and some of these share a SourceLocationClassLoader maybe.

Example stacktrace fixed by this PR:

[ERROR] Users/jurgenv/Sites/rascal-website/courses/Recipes/Metrics/MeasuringJava/MeasuringMethods/MeasuringMethods.md:050:0: Code execution failed:
    |mvn://org.rascalmpl--java-air--1.0.0/lang/java/m3/AST.rsc|(21382,545,<384,0>,<390,199>): Java("IllegalStateException","Recursive update")
    	at java.util.concurrent.ConcurrentHashMap.computeIfAbsent(|unknown:///ConcurrentHashMap.java|(0,0,<1760,0>,<1760,0>))
    	at org.rascalmpl.uri.classloaders.SourceLocationClassLoader.findClass(|unknown:///SourceLocationClassLoader.java|(0,0,<102,0>,<102,0>))
    	at java.lang.ClassLoader.loadClass(|unknown:///ClassLoader.java|(0,0,<589,0>,<589,0>))
    	at java.lang.ClassLoader.loadClass(|unknown:///ClassLoader.java|(0,0,<522,0>,<522,0>))
    	at java.lang.ClassLoader.defineClass1(|unknown:///ClassLoader.java|(0,0,<0,0>,<0,0>))
    	at java.lang.ClassLoader.defineClass(|unknown:///ClassLoader.java|(0,0,<1017,0>,<1017,0>))
    	at java.lang.ClassLoader.defineClass(|unknown:///ClassLoader.java|(0,0,<1096,0>,<1096,0>))
    	at org.rascalmpl.uri.classloaders.SourceLocationClassLoader.lambda$findClass$0(|unknown:///SourceLocationClassLoader.java|(0,0,<110,0>,<110,0>))
    	at java.util.concurrent.ConcurrentHashMap.computeIfAbsent(|unknown:///ConcurrentHashMap.java|(0,0,<1705,0>,<1705,0>))
    	at org.rascalmpl.uri.classloaders.SourceLocationClassLoader.findClass(|unknown:///SourceLocationClassLoader.java|(0,0,<102,0>,<102,0>))
    	at java.lang.ClassLoader.loadClass(|unknown:///ClassLoader.java|(0,0,<589,0>,<589,0>))
    	at java.lang.ClassLoader.loadClass(|unknown:///ClassLoader.java|(0,0,<522,0>,<522,0>))
    	at lang.java.m3.internal.EclipseJavaCompiler.constructASTParser(|unknown:///EclipseJavaCompiler.java|(0,0,<347,0>,<347,0>))
    	at lang.java.m3.internal.EclipseJavaCompiler.buildCompilationUnits(|unknown:///EclipseJavaCompiler.java|(0,0,<257,0>,<257,0>))
    	at lang.java.m3.internal.EclipseJavaCompiler.createAstsFromFiles(|unknown:///EclipseJavaCompiler.java|(0,0,<186,0>,<186,0>))
    	at lang.java.m3.internal.EclipseJavaCompiler.createAstsFromFiles(|unknown:///EclipseJavaCompiler.java|(0,0,<175,0>,<175,0>))
    	at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(|unknown:///NativeMethodAccessorImpl.java|(0,0,<0,0>,<0,0>))
    	at createAstsFromFiles(|mvn://org.rascalmpl--java-air--1.0.0/lang/java/m3/AST.rsc|(21240,11,<377,151>,<377,162>))
    	at $(|prompt:///|(10,76,<1,10>,<1,86>))

[codecov]

Codecov Report

❌ Patch coverage is 14.28571% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 46%. Comparing base (e4c83c2) to head (1b75137).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
...pl/uri/classloaders/SourceLocationClassLoader.java	14%	23 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##              main   #2559   +/-   ##
=======================================
  Coverage       46%     46%           
- Complexity    6627    6631    +4     
=======================================
  Files          792     792           
  Lines        65361   65374   +13     
  Branches      9786    9788    +2     
=======================================
+ Hits         30476   30486   +10     
- Misses       32533   32536    +3     
  Partials      2352    2352           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[DavyLandman]

I get the fix for the ThreadLocal, but I'm unsure about the computeIfAbsent fix. I guess the new code would work, but I'm a bit hesitant if it's needed.

[jurgenvinju]

I get the fix for the ThreadLocal, but I'm unsure about the computeIfAbsent fix. I guess the new code would work, but I'm a bit hesitant if it's needed.

I had the same doubts. The reason is that computeIfAbsent can enter itself recursively due to cyclic class dependencies. It throws an exception when that happens. This new code recurses between the get and the set such that this can't happen. It looks ugly compared to the computeIfAbsent design.

[DavyLandman]

I've updated the code and added some comments to explain this. if the test succeed, this will be merged.