Skip to content

StopWalkException escapes RevWalk.iterator() under RevSort.TOPO (regression in 7.8.0) #294

Description

@JoostK

Version

7.8.0.202609011348-r

Operating System

Windows/Linux (irrelevant)

Bug description

Since 7.8.0, a RevFilter that halts a walk by throwing StopWalkException — including jgit's own
MaxCountRevFilter — propagates that exception out of RevWalk.iterator() when the walk is sorted
with RevSort.TOPO. It worked in 7.7.1 and still works without RevSort.TOPO.

Introduced by ab51fe2 "RevWalk: Use an incremental topological sorting algorithm". Still
present on master.

Reproduction

JoostK/jgit-topo-sort-regression

mvn test                                        # 7.8.0.202609011348-r -> 1 failure
mvn test -Djgit.version=7.7.1.202607240634-r    # 7.7.1 -> all pass

The essential part:

// 5-commit linear history, HEAD == E
try (RevWalk revWalk = new RevWalk(repository)) {
  revWalk.markStart(revWalk.parseCommit(headId));
  revWalk.sort(RevSort.TOPO);
  revWalk.setRevFilter(MaxCountRevFilter.create(2));

  for (RevCommit commit : revWalk) {   // throws StopWalkException on 7.8.0
    ...
  }
}

Expected: two commits. Actual on 7.8.0: org.eclipse.jgit.errors.StopWalkException.

Cause

StopWalkException is the documented way for a filter to stop a walk (RevFilter#include: "the
walk is halted and no more results are provided"
), and PendingGenerator.next() is the only place
in jgit that catches it:

} catch (StopWalkException swe) {
    pending.clear();
    return null;
}

Before 7.8.0, RevSort.TOPO wrapped a PendingGenerator (TopoSortGenerator), so the exception
was absorbed there. Since ab51fe2, StartGenerator.next() builds a TopoSortPendingGenerator
instead, which applies the RevFilter itself:

TopoSortPendingGenerator.<init>
  -> TopoInDegreePhase.initialize -> calculateInDegrees -> rewriteIfNeeded
  -> TopoExplorePhase.explore -> TopoExplorePhase.passThroughFilter
  -> filter.include(walker, c)

TopoExplorePhase.passThroughFilter calls filter.include(walker, c) with no handling, and the
identifier StopWalkException appears in none of TopoExplorePhase, TopoInDegreePhase or
TopoSortPendingGenerator — on 7.8.0 or on master. The exception therefore travels up through
StartGenerator.next() and out of RevWalk.next() / nextForIterator() / iterator().

The filter is consulted from two places in the new code (TopoExplorePhase.explore marks
topoPassedFilterFlag, TopoSortPendingGenerator.next reads it via hasPassedFilter), so a fix
presumably needs to stop the exploration and drain whatever is already queued, rather than just
catching at the outermost frame.

Affected versions

  • 7.8.0.202609011348-r — affected
  • 7.7.1.202607240634-r — not affected

Actual behavior

mvn test
[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running org.example.FilterOrderTest
SLF4J(W): No SLF4J providers were found.
SLF4J(W): Defaulting to no-operation (NOP) logger implementation
SLF4J(W): See https://www.slf4j.org/codes.html#noProviders for further details.
=== TOPO
  filter asked about : [H, F, E, D, C, B, A]
  walk emitted       : [H, F, E, D, C, B, A]
  orders agree       : true
=== COMMIT_TIME_DESC (default)
  filter asked about : [H, F, E, D, C, B, A]
  walk emitted       : [H, F, E, D, C, B, A]
  orders agree       : true
=== REVERSE
  filter asked about : [H, F, E, D, C, B, A]
  walk emitted       : [A, B, C, D, E, F, H]
  orders agree       : false
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.755 s -- in org.example.FilterOrderTest
[INFO] Running org.example.TopoSortStopWalkTest
[ERROR] Tests run: 4, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 2.545 s <<< FAILURE! -- in org.example.TopoSortStopWalkTest
[ERROR] org.example.TopoSortStopWalkTest.maxCountUnderTopoSort -- Time elapsed: 0.476 s <<< ERROR!
org.eclipse.jgit.errors.StopWalkException
	at org.eclipse.jgit.errors.StopWalkException.<clinit>(StopWalkException.java:23)
	at org.eclipse.jgit.treewalk.filter.PathFilterGroup$Group.include(PathFilterGroup.java:226)
	at org.eclipse.jgit.treewalk.filter.TreeFilter.matchFilter(TreeFilter.java:198)
	at org.eclipse.jgit.treewalk.TreeWalk.next(TreeWalk.java:919)
	at org.eclipse.jgit.dircache.DirCache.updateSmudgedEntries(DirCache.java:1024)
	at org.eclipse.jgit.dircache.DirCache.writeTo(DirCache.java:683)
	at org.eclipse.jgit.dircache.DirCache.write(DirCache.java:632)
	at org.eclipse.jgit.dircache.BaseDirCacheEditor.commit(BaseDirCacheEditor.java:247)
	at org.eclipse.jgit.dircache.DirCacheBuilder.commit(DirCacheBuilder.java:1)
	at org.eclipse.jgit.merge.ResolveMerger$WorkTreeUpdater.writeWorkTreeChanges(ResolveMerger.java:373)
	at org.eclipse.jgit.merge.ResolveMerger.mergeTrees(ResolveMerger.java:1974)
	at org.eclipse.jgit.merge.ResolveMerger.mergeImpl(ResolveMerger.java:951)
	at org.eclipse.jgit.merge.Merger.merge(Merger.java:233)
	at org.eclipse.jgit.merge.Merger.merge(Merger.java:186)
	at org.eclipse.jgit.merge.ThreeWayMerger.merge(ThreeWayMerger.java:95)
	at org.eclipse.jgit.api.MergeCommand.call(MergeCommand.java:347)
	at org.example.TestHistory.mergeFrom(TestHistory.java:70)
	at org.example.FilterOrderTest.reportFilterOrderVersusEmissionOrder(FilterOrderTest.java:36)
	at java.base/java.lang.reflect.Method.invoke(Method.java:565)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1604)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1604)

[INFO]
[INFO] Results:
[INFO]
[ERROR] Errors:
[ERROR]   TopoSortStopWalkTest.maxCountUnderTopoSort » StopWalk
[INFO]
[ERROR] Tests run: 5, Failures: 0, Errors: 1, Skipped: 0

Expected behavior

mvn test -Djgit.version=7.7.1.202607240634-r
[INFO] --- surefire:3.5.2:test (default-test) @ jgit-topo-stopwalk-repro ---
[INFO] Using auto detected provider org.apache.maven.surefire.junitplatform.JUnitPlatformProvider
[INFO]
[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running org.example.FilterOrderTest
SLF4J(W): No SLF4J providers were found.
SLF4J(W): Defaulting to no-operation (NOP) logger implementation
SLF4J(W): See https://www.slf4j.org/codes.html#noProviders for further details.
=== TOPO
  filter asked about : [H, F, E, D, C, B, A]
  walk emitted       : [H, F, E, D, C, B, A]
  orders agree       : true
=== COMMIT_TIME_DESC (default)
  filter asked about : [H, F, E, D, C, B, A]
  walk emitted       : [H, F, E, D, C, B, A]
  orders agree       : true
=== REVERSE
  filter asked about : [H, F, E, D, C, B, A]
  walk emitted       : [A, B, C, D, E, F, H]
  orders agree       : false
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.458 s -- in org.example.FilterOrderTest
[INFO] Running org.example.TopoSortStopWalkTest
[INFO] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.365 s -- in org.example.TopoSortStopWalkTest
[INFO]
[INFO] Results:
[INFO]
[INFO] Tests run: 5, Failures: 0, Errors: 0, Skipped: 0

Other information

A note on the stack trace

Anyone triaging this from a user-supplied stack trace should know that
StopWalkException preallocates a singleton and does not override fillInStackTrace():

public static final StopWalkException INSTANCE = new StopWalkException();

The trace is captured once, at class initialisation, and every later throw reuses it. Since jgit
throws StopWalkException as ordinary control flow elsewhere (e.g.
PathFilterGroup$Single.include during AddCommand), the reported frames often point somewhere
unrelated to the failing walk. Might be worth overriding fillInStackTrace() to return this and
documenting that the trace is meaningless, so it stops being misleading.


This issue/repro was authored by Claude Opus 5, reproducing a regression that surfaced in the update to 7.8.0 in a private app.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions