gnodet commented on issue #12301:
URL: https://github.com/apache/maven/issues/12301#issuecomment-4738551206
## Root Cause Analysis
### The Project Pattern That Triggers This
Jackrabbit FileVault uses an **internal parent** anti-pattern in combination
with CI-friendly versions:
- The **root POM** (`pom.xml`) declares `<parent>` pointing to
`parent/pom.xml` (a reactor module in the same repo)
- The parent reference uses a **CI-friendly version**:
`<version>${revision}</version>`
- `.mvn/` lives at the repo root, so `session.getRootDirectory()` = the
directory of `pom.xml`
### The Exact Recursion Cycle
The cycle involves **two interacting feedback loops** exposed by how
`CyclicCacheAccessException` is handled.
**Step-by-step trace** (all on one thread):
1. `loadFilePom` calls `readFileModel(rootPOM)` → `cache(rootPOM, FILE,
doReadFileModel)` — cache miss, starts computing, `CachingSupplier` marks
`computingThread = currentThread`
2. Inside `doReadFileModel(rootPOM)` (line ~1579–1601):
The parent version `${revision}` sets `versionContainsExpression = true`,
so it resolves the parent path and calls:
`derive(parent/pom.xml).readFileModel()` → `cache(parent/pom.xml, FILE,
doReadFileModel)` — new cache entry, starts computing
3. Inside `doReadFileModel(parent/pom.xml)` (line 1653):
`getEnhancedProperties(parentModel, rootDirectory)` is called.
Since `parent/pom.xml` is not the root (`/filevault/parent/` ≠
`/filevault/`), it takes the non-root branch (line 703–706):
```java
Model rootModel =
derive(Sources.buildSource(rootModelPath)).readFileModel();
// ↑ rootModelPath =
/filevault/pom.xml
```
4. `readFileModel(rootPOM)` → `cache(rootPOM, FILE, ...)` — **same key as
step 1!**
`CachingSupplier` detects re-entrancy: `value == null && computingThread
== currentThread` → throws `CyclicCacheAccessException`
5. **`AbstractRequestCache.request()` catches `CyclicCacheAccessException`
and calls `supplier.apply(req)` directly** (line 69), bypassing the cache. This
re-invokes `doReadFileModel(rootPOM)` without any guard.
6. `doReadFileModel(rootPOM)` again (step 2 repeats): parent version has
`${revision}` → tries to read `parent/pom.xml` → `cache(parent/pom.xml, FILE,
...)` — **same key as step 3!**
`CyclicCacheAccessException` again → `doReadFileModel(parent/pom.xml)`
called directly
7. `doReadFileModel(parent/pom.xml)` again (step 3 repeats):
`getEnhancedProperties` → `readFileModel(rootPOM)` → step 4 repeats → step 5
repeats → step 6...
**Steps 5→6→7→5→6→7→...** until `StackOverflowError`.
### Why the `CyclicCacheAccessException` Fallback Makes It Worse
`AbstractRequestCache.request()` catches `CyclicCacheAccessException` and
falls back to calling the supplier directly. The comment says this is to handle
batch/singular re-entrant access. But in this context it is the wrong escape
hatch: calling `supplier.apply(req)` on `doReadFileModel` doesn't terminate —
it just re-enters the same computation that caused the exception, starting the
whole cycle over with one more frame on the stack.
### Two Interacting Root Causes
1. **`getEnhancedProperties` reads the root model via `readFileModel`**
(line 705), which triggers the full `doReadFileModel` pipeline including
another call to `getEnhancedProperties`. For a non-root module whose parent IS
the root POM, this creates a circular read dependency:
`doReadFileModel(parent/) → getEnhancedProperties → readFileModel(root) →
doReadFileModel(root) → readFileModel(parent/) → ...`
2. **`doReadFileModel` reads the parent POM to resolve CI-friendly version
coordinates** (line 1601, when `versionContainsExpression`), which for the root
POM triggers reading the internal parent module. Combined with (1), these two
loops interlock into an unbounded cycle.
### Suggested Fix
The cleanest fix is to make `getEnhancedProperties` perform a **lightweight
property-only read** of the root model rather than calling the full
`readFileModel()` pipeline:
```java
// In getEnhancedProperties(), instead of:
Model rootModel = derive(Sources.buildSource(rootModelPath)).readFileModel();
// Use a lightweight read that extracts only properties (no parent
resolution, no getEnhancedProperties call):
Model rootModel =
derive(Sources.buildSource(rootModelPath)).readRawXmlPropertiesOnly();
```
Alternatively, add a depth guard / in-progress set to
`getEnhancedProperties` so that if it is already being executed for a given
source path on the current call chain it returns early (skipping the recursive
root model read). A thread-local `Set<Path>` of "currently resolving" paths in
`getEnhancedProperties` would suffice.
The `CyclicCacheAccessException` fallback itself might also need to be
re-examined: re-running the full supplier without a guard means any cycle that
goes through that path will stack-overflow rather than produce a meaningful
error.
---
*Analyzed from the Maven source at
`impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java`
and
`impl/maven-impl/src/main/java/org/apache/maven/impl/cache/AbstractRequestCache.java`.
The critical methods are `doReadFileModel` (lines 1491–1705),
`getEnhancedProperties` (lines 676–714), and `AbstractRequestCache.request`
(lines 61–71).*
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
