Skip to content

Fix: preserve original trace data when stamping re-entrancy marker#1980

Merged
gnodet merged 2 commits into
apache:masterfrom
gnodet:fix/trace-marker-preserve-data
Jul 20, 2026
Merged

Fix: preserve original trace data when stamping re-entrancy marker#1980
gnodet merged 2 commits into
apache:masterfrom
gnodet:fix/trace-marker-preserve-data

Conversation

@gnodet

@gnodet gnodet commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Summary

PR #1957 (commit e29d0cd) added re-entrancy detection to DefaultRepositorySystem by stamping a REPOSITORY_SYSTEM_CALL marker into the RequestTrace chain. However, it placed the marker as the trace tip data, replacing whatever was there before (typically an Artifact set by Maven core).

Plugins that walk the RequestTrace chain and cast getData() to their expected type without an instanceof check (e.g. pgpverify-maven-plugin, maven-gpg-plugin) get a ClassCastException:

ClassCastException: class org.eclipse.aether.internal.impl.DefaultRepositorySystem$1 
cannot be cast to class org.apache.maven.artifact.Artifact

This is a regression — both maven-artifact-plugin and maven-gpg-plugin were passing with rc-5 and are now failing with resolver 2.0.21-SNAPSHOT.

Fix

Instead of placing the marker as the trace tip, insert it one level deeper and re-attach the original tip data on top:

private static RequestTrace stampReentrancyMarker(RequestTrace currentTrace) {
    RequestTrace markerTrace = RequestTrace.newChild(currentTrace, REPOSITORY_SYSTEM_CALL);
    return currentTrace != null
            ? RequestTrace.newChild(markerTrace, currentTrace.getData())
            : markerTrace;
}

The isReentrant() method is unaffected — it walks the full chain and will still find the marker.

Test plan

  • Added outerCallPreservesOriginalTraceData test verifying that original trace data remains at the tip after stamping
  • Added outerCallWithNullTraceStillStampsMarker test verifying the null-trace case still works correctly for re-entrancy detection
  • All existing re-entrancy tests continue to pass
  • Full maven-resolver-impl test suite passes (454 tests, 0 failures)

🤖 Generated with Claude Code

The re-entrancy detection added in e29d0cd stamps a marker object
into the RequestTrace chain. However, it placed the marker as the
trace tip data, replacing whatever was there before (typically an
Artifact set by Maven core). Plugins that walk the trace chain and
cast getData() to Artifact (e.g. pgpverify-maven-plugin) get a
ClassCastException when they hit the marker object.

This fix inserts the marker one level deeper in the trace chain and
re-attaches the original tip data on top, so existing code that
reads getData() continues to find the expected object. The
isReentrant() method is unaffected because it walks the full chain
and will still find the marker.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a regression in DefaultRepositorySystem re-entrancy detection by stamping the REPOSITORY_SYSTEM_CALL marker into the RequestTrace chain without overwriting the original trace tip data (which is commonly an Artifact provided by Maven core). This avoids downstream ClassCastExceptions in plugins that assume the trace tip’s getData() is of a specific type.

Changes:

  • Introduce stampReentrancyMarker(RequestTrace) to insert the marker below the trace tip while preserving the original tip data.
  • Update all relevant public DefaultRepositorySystem entrypoints to use stampReentrancyMarker(...) instead of stamping the marker as the trace tip.
  • Add tests covering preservation of original trace tip data and the null-trace stamping/re-entrancy behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultRepositorySystem.java Replace direct marker stamping with stampReentrancyMarker(...) to preserve original RequestTrace tip data.
maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultRepositorySystemReentrancyTest.java Add regression tests for trace tip preservation and null-trace marker stamping behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +279 to +282
// The marker should be present deeper in the chain (parent of the tip)
RequestTrace parent = resultTrace.getParent();
assertNotNull(parent, "Trace should have a parent containing the re-entrancy marker");
}
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@gnodet
gnodet marked this pull request as ready for review July 19, 2026 16:32
@gnodet
gnodet merged commit 1e5d22a into apache:master Jul 20, 2026
20 checks passed
@github-actions github-actions Bot added this to the 2.0.21 milestone Jul 20, 2026
@github-actions

Copy link
Copy Markdown

@gnodet Please assign appropriate label to PR according to the type of change.

gnodet added a commit to gnodet/maven-resolver that referenced this pull request Jul 20, 2026
gnodet added a commit to gnodet/maven that referenced this pull request Jul 20, 2026
Includes fix for trace marker regression (apache/maven-resolver#1980)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants