SpotBugs concurrency: holder-idiom singleton + volatile flag

[Vest]

Four SpotBugs concurrency findings in two files — all real thread-safety concerns.

Fixes

AT_STALE_THREAD_WRITE_OF_PRIMITIVE — AbstractReferenceManufacturer.java:69

isResolved was a plain boolean, set by the data-load thread in resolveReferences() and then read on the variable-binding hot paths (lines 693, 1253) from other threads. Two real failure modes:

1. The reader could keep observing a cached false indefinitely on memory architectures with weak ordering.
2. Worse: a reader that does observe true could still see a half-populated active map — the isResolved = true write isn't ordered with respect to the prior map writes.

Marked the field volatile. The write-once / read-many access pattern doesn't need full synchronized; volatile gives both visibility and the happens-before edge that orders the prior writes to active before any reader who sees true.

LI_LAZY_INIT_STATIC + SING_SINGLETON_GETTER_NOT_SYNCHRONIZED — PluginFunctionLibrary.getInstance()

Classic broken double-init pattern:

private static PluginFunctionLibrary instance = null;
public static PluginFunctionLibrary getInstance() {
    if (instance == null) {                              // ← unsafe read
        instance = new PluginFunctionLibrary();          // ← two threads can both pass
    }
    return instance;
}

Replaced with the Initialization-on-Demand Holder Idiom:

private static final class Holder {
    static final PluginFunctionLibrary INSTANCE = new PluginFunctionLibrary();
}
public static PluginFunctionLibrary getInstance() {
    return Holder.INSTANCE;
}

JVM class-init guarantees the Holder is initialised exactly once, lazily on first getInstance() call, and publishes a happens-before edge for INSTANCE. No synchronized, no volatile, no allocation overhead. The instance static field is gone; the list field can now be final.

THROWS_METHOD_THROWS_CLAUSE_BASIC_EXCEPTION — PluginFunctionLibrary.loadPlugin

Narrowed throws Exception to throws ReflectiveOperationException, which is what clazz.getDeclaredConstructor().newInstance() actually throws (covers NoSuchMethodException, InstantiationException, IllegalAccessException, InvocationTargetException). Also replaced the deprecated clazz.newInstance() with getDeclaredConstructor().newInstance(). The impl signature can legally narrow the interface's throws Exception.

Why not Spring / LazyConstant for the singleton?

Discussed during planning:

• Spring is the wiring mechanism for the facet graph (~232 beans) but PluginFunctionLibrary is a PluginLoader, not a facet — sits next to PrerequisiteTestFactory.getInstance(), PJEP.getJepPluginLoader(), etc., none of which are Spring beans. Plugin loaders register during Main.bootstrap before most Spring consumers come up. Adopting Spring here would mean either flipping bootstrap order for one outlier, or doing a project-wide refactor of all PluginLoader.getInstance() singletons — out of scope for a SpotBugs fix.
• LazyConstant (JEP 526, preview in JDK 26) is the JDK's eventual answer but still preview, requires --enable-preview on every JVM in the toolchain, and the two advantages it brings over the holder idiom (JIT constant-folding, retry-on-init-failure) don't apply here — getInstance() is called twice in the codebase against a no-op constructor. The holder idiom is the idiomatic Java answer that's been in Effective Java since 1998 and is JVM-spec thread-safe without any preview flag.

Tests

New PluginFunctionLibraryTest (7 tests) pinning:

• Singleton identity (getInstance() returns same ref).
• getPluginClasses() returns { FormulaFunction.class }.
• loadPlugin registers a FormulaFunction.
• loadPlugin silently ignores non-FormulaFunction classes.
• loadPlugin rejects duplicate names without throwing.
• loadPlugin propagates ReflectiveOperationException on a class with no accessible no-arg constructor.
• getFunctions() returns an unmodifiable view.

The volatile fix is a memory-model property and is not testable from a single-threaded JUnit run — writing a stress test that's reliable enough for CI would be more work than the fix and would essentially be testing the JVM spec. Behavioural regression is covered transitively by 2965 itest cases that load real data through AbstractReferenceManufacturer.resolveReferences(). This is called out in the test class Javadoc.

Verification

• ./gradlew compileJava → BUILD SUCCESSFUL
• Scoped :test --tests "pcgen.cdom.*" --tests "plugin.lsttokens.*" → 12075 tests, 0 failures
• ./gradlew :itest → 2965 tests, 0 failures
• ./gradlew spotbugsMain → 4 targeted findings cleared, 0 new findings introduced. Total dropped from 76 → 72.

Scope note

Standalone — independent of the still-open #7627 (mark-cdom-classes-final) and the merged #7624–#7628. Branched off latest origin/master.

[Vest]

I want to use LazyConstant, when we switch to another Java or LTS.
I don't want to have much code :)