Swappable modules classloader (#6787): the swap is inert behind loader.path, and the watcher blocks the shared scheduler

[delchev]

Review findings on PR #6787 (part 2 of #6776, closing #6778). Line references are against that
PR's head, not master.

Two of these are blockers: the headline capability does not work in the shipped Docker deployment
after the first restart (1), and one failure class leaves partial registry state behind while the
endpoint answers 500 (5). The rest are correctness, lifecycle and observability defects that are
cheaper to fix on the PR than after it lands.

---

Blockers

1. Restartless upgrade and removal are inert in the shipped Docker image after the first restart

build/application/Dockerfile:42 launches with -Dloader.path=/modules,/root/.dirigible/resolved-modules,
and ResolvedModulesLinker.directory() defaults to ~/.dirigible/resolved-modules — the same
directory. Dockerfile:37 tells operators to mount /root/.dirigible on a volume so the resolved
jars survive restarts, which is the documented and expected deployment.

So from the second boot onward the resolved jars are on the application classloader.
ModulesClassLoader is deliberately parent-first, and the consequences follow:

• Upgrade is inert. A 1.0.0 -> 1.1.0 swap builds a new generation whose parent already defines
  the class; parent-first serves the old one. The endpoint keeps answering with the old body.
• AOT registration is inert. CompiledModuleClassProvider.load() resolves through the parent
  and records type.getClassLoader() — the app classloader, not the new generation.
• Removal is inert. scanWithResolver scans through the parent chain, still finds
  META-INF/dirigible/<project>/.compiled in the jar on loader.path, and re-registers the
  controller. The endpoint answers 200 where DynamicDependenciesIT expects 404.

In every one of these cases the pipeline logs Dependency layer swapped to generation [N] and the
state JSON reports the new artifact as active. The only signal is the WARN from
warnOnPlatformShadowing (DependencySynchronizer.java:164), and the swap proceeds regardless.

DynamicDependenciesIT cannot catch this: its resolved-modules directory is a fresh @TempDir and
loader.path is unset under failsafe, so the shadowing condition never arises in the test.

This needs a decision, not a patch: either the resolved-modules directory stops being a
loader.path entry (the modules classloader becomes the only path by which declared dependencies
enter the JVM), or a shadowed artifact is a hard abort rather than a WARN. Silently reporting a
successful swap that changed nothing is the worst of the three.

2. The dependency watcher starves the security access-constraint refresh

DependenciesWatcher.watch() (DependenciesWatcher.java:50) is @Scheduled on Spring's shared
TaskScheduler, whose pool size is 1 — there is no spring.task.scheduling.pool.size override
anywhere in the repository. At DependenciesWatcher.java:66 it calls
dependenciesService.resolveAndActivate() synchronously on that thread, which performs a Maven
resolution (network, subject to connect and read timeouts) and then
javaSynchronizer.rebuildOnDependenciesChanged() — a full javac rebuild of the entire client
codebase.

The only other @Scheduled beans in the platform share that single thread:

• AccessVerifier.scheduledRefreshCache — @Scheduled(fixedRate = 8_000), AccessVerifier.java:65
• AccessVerifier.scheduledRefreshModified — @Scheduled(fixedRate = 5_000), AccessVerifier.java:148
• TscWatcherService.monitorTscProcess

For the whole duration of a resolve-and-rebuild — tens of seconds normally, minutes on a repository
timeout — access definitions stop refreshing, so newly published or removed *.access constraints
are not enforced, and the transpiler watchdog stops restarting a dead tsc. The pipeline needs its
own executor.

---

Should be fixed before merge

3. The native-library veto is over-inclusive, misses common forms, and is process-wide

DependencySynchronizer.java:153:

• The test is name.endsWith(".so" | ".dylib" | ".dll"), which matches any jar that merely ships
  a native library as a resource — org.xerial:sqlite-jdbc, net.java.dev.jna,
  netty-transport-native-*, snappy-java, lz4-java. Declaring one of those blocks every other
  project's dependencies from ever activating.
• On the first swap added contains the launch-classpath jars, because target is seeded from
  launchClasspathJars() (DependencySynchronizer.java:121, :294) and the current generation is
  empty. A single /modules drop-in jar carrying a native library therefore aborts every swap for
  the lifetime of the process — and nobody declared it.
• Versioned and Apple forms are missed: libfoo.so.1, .jnilib.
• The abort is the whole swap, not the offending coordinate, and the error advises
  scope: "platform", which is not implemented yet.

At minimum: exclude launch-classpath jars from validation, and reject only the offending
declaration.

4. ClasspathExpander.remove(String) deletes the whole registry collection, so every upgrade destroys non-jar content

ClasspathExpander.remove(String) (ClasspathExpander.java:142) calls
repository.removeCollection("/registry/public/" + project) unconditionally. Because an upgrade is
implemented as remove-then-re-expand (DependencySynchronizer.java:215 then :165), every version
bump of a module destroys anything else living under /registry/public/<project> — for example a
customization or extension published into the module's project folder. Scope the removal to the
entry names the leaving jar actually carried.

Compounded by an inconsistency between the two components: ModuleJarInspector.inspect
(ModuleJarInspector.java:59) does not honor the META-INF/dirigible/.skip marker, while
ClasspathExpander.copyRegistryContent does. A jar with .skip plus
META-INF/dirigible/myproj/... reports projects=[myproj] but lays nothing down; when that
dependency is later removed, remove("myproj") deletes /registry/public/myproj — a collection the
jar never created, and possibly a developer's own published project of that name. The inspector
should honor .skip and report no projects.

5. The registry mutation precedes the swap and is unguarded, so a write failure leaves partial state and answers 500

reconcileRegistryPayload runs at DependencySynchronizer.java:165, before
loaderHolder.swap(target) at :167, and neither classpathExpander.remove(...) nor
classpathExpander.expand(...) is guarded. ClasspathExpander.expand throws UncheckedIOException
(ClasspathExpander.java:127) and repository.createResource throws RepositoryWriteException —
both unchecked, and DependenciesEndpoint.resolve() has no handler for either.

The outcome contradicts the DependencySynchronizer javadoc's "no partial swap" guarantee: the
leaving module's registry payload is already deleted, no new generation is installed, lastState is
never updated, and POST /services/core/dependencies/resolve answers 500 instead of the documented
failures map. Either move the payload reconciliation after the swap, or wrap it so a failure
becomes a reported abort like the other two.

6. A transient resolution failure permanently disarms the watcher

DependenciesService.java:107 records lastDeclaredFingerprint before
resolver.resolve(...) on line 108. So: a project declaring a new dependency is published, the
remote repository 503s or the network blips, failures is non-empty and no swap happens — but the
fingerprint is stored anyway. After the network recovers the declarations are unchanged,
current.equals(last) short-circuits in the watcher, and the dependency never activates. No retry
until someone edits project.json, POSTs /resolve, or restarts.

This is exactly the outage ResolvedModulesLinker.sync goes out of its way to tolerate (it skips
stale removal after a partial resolution). Record the fingerprint only on a clean pass.

7. Every swap runs the full generation dispatch twice, with a window on the old jars

JavaDependenciesChangedListener.java:72 calls
compiledModuleClassProvider.rediscover(...), which reaches JavaLoader.installCompiledModules
(JavaLoader.java:261). That method takes loaderHolder.current() (JavaLoader.java:270) — the
existing ClientClassLoader, whose parent is the now-retired modules generation — and runs a
complete applyGeneration: consumer onClassUnloaded / onClassLoaded over the whole generation
plus componentContainer.rebuild(...). Line 73 then calls rebuildOnDependenciesChanged(), which
does all of it again with a correctly-parented loader.

So each swap re-instantiates every client bean twice (@PostConstruct twice) and unregisters and
re-registers every Quartz job, JMS listener, controller mapping and websocket twice — the JMS
reconnect in particular is not free. In the window between the two passes, client code and JS
Java.type lookups (via the new hostClassLoader wiring in DirigibleJavascriptCodeRunner)
resolve against the old dependency jars. Have rediscover record the compiled set and let the
single rebuildAll() apply it.

---

Smaller

8. The IDE's Java editor keeps the pre-swap classpath

JdtLsManager.defaultClasspathXml() (JdtLsManager.java:350) memoises the rendered .classpath
XML in a field, and its javadoc at :346 states this is safe because
"ClassPathIndex#classPathEntries() is itself cached for the application lifetime". The new
ClassPathIndex.invalidate() breaks that premise: the index now changes mid-run, but the memoised
XML never does.

Effect: after a dependency is added restartlessly, a registry .java importing it shows unresolved
imports in the Problems view and Monaco while the runtime compiles and serves it correctly. Note
also that classpathFingerprint() (JdtLsManager.java:427) does re-read the index, so the
fingerprint changes while the written .classpath stays stale — an internal inconsistency, not just
staleness. JdtLsManager should observe DependenciesChangedEvent and drop the memoised XML.

9. The 5-second poll is not side-effect-free and re-parses the whole registry

DependenciesWatcher.java:59 calls collector.collect() every tick purely to compute a
fingerprint. ProjectDependenciesCollector logs as a side effect of collecting: an ERROR with
stack trace for an invalid maven coordinate, an ERROR for a missing id, a WARN for an unparseable
project.json, a WARN per unknown dependency type. Since the watcher is armed by default
(DIRIGIBLE_DEPENDENCIES_DYNAMIC defaults to true) and a failing declaration is deliberately not
retried, one project with a typo'd coordinate emits an ERROR every 5 seconds indefinitely, drowning
the Logs view and any aggregation.

It also re-reads and JSON-parses every registry project's project.json on every tick — N
repository reads per 5 seconds, which is meaningful on a DB-backed repository with hundreds of
projects. A quiet collection path for the fingerprint, or a longer interval, or reuse of the
existing synchronizer change detection.

10. InvalidPathException aborts the startup sweep on Windows

ClasspathExpander.java:180 replaces string handling with registryRoot.resolve(relative), and
java.nio.file.Path parsing throws the unchecked InvalidPathException on Windows for any jar
entry name containing :, ?, * or ". The surrounding catch at ClasspathExpander.java:89 is
URISyntaxException | IOException, so such an entry now aborts the entire startup sweep (and, from
expand, the entire swap) on Windows, where the previous code copied it without complaint. Catch
InvalidPathException alongside the existing null-target case and skip the entry.

11. DynamicDependenciesIT leaves state behind in a shared JVM

The test sets three RUNTIME configuration keys (DIRIGIBLE_MAVEN_REPOSITORIES,
DIRIGIBLE_MAVEN_LOCAL_REPO, DIRIGIBLE_DEPENDENCIES_DIR) in @BeforeAll and never restores them,
and leaves a registry project plus a client .java source whose declared dependency resolves to a
jar under a @TempDir that is deleted when the class finishes. Failsafe has no forkCount or
reuseForks override, so the whole shard shares one JVM and one Spring context.

It does not currently break the suite — the last test deliberately keeps textlib declared, and
JavaLoader's last-good bytecode carry-over absorbs the rest — but any later client-Java rebuild in
the same fork compiles that leftover source against a deleted classpath entry. An @AfterAll that
removes the project and restores configuration would close it.

---

Checked and not issues

Recorded so they are not re-litigated:

• CompiledModuleClassProvider's installedBefore flag correctly handles removing the last module
  (an empty discovery is still installed once a non-empty one was).
• rebuild(List.of()) still installs a ClientClassLoader parented on the new modules generation,
  so JS Java.type sees new jars even with zero client .java sources.
• The leading-slash normalization in copyRegistryContent is behaviour-preserving:
  RepositoryPath tokenizes away //.
• The new modules/engines/engine-graalium -> components/core/core-java dependency does not
  introduce a new layering violation; components/core/core-base was already a dependency of that
  module.
• The Zip Slip guard, the immutable-versioned-path rule, and the unit-test coverage named in Swappable modules classloader — restartless add/upgrade/remove of dependency JARs #6778's
  checklist are all present and correct.