From 770bad9fca9beecac0004385a92b76ce01afe858 Mon Sep 17 00:00:00 2001 From: Axel Uhl Date: Thu, 1 Oct 2026 15:59:43 +0200 Subject: [PATCH 1/3] properly compare candidate time points in mark passing calculator; the old code subtracted the millisecond representation of the time points from one another and cast the result to an int which may have silently led to an overflow in case the difference between the time points was greater than 49 days. For races with open-ended tracking time ranges and sporadic fixes along that full time range may have caused problems here. --- .../markpassingcalculation/impl/CandidateChooserImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/com.sap.sailing.domain/src/com/sap/sailing/domain/markpassingcalculation/impl/CandidateChooserImpl.java b/java/com.sap.sailing.domain/src/com/sap/sailing/domain/markpassingcalculation/impl/CandidateChooserImpl.java index 6585e925530..65c2402f28d 100755 --- a/java/com.sap.sailing.domain/src/com/sap/sailing/domain/markpassingcalculation/impl/CandidateChooserImpl.java +++ b/java/com.sap.sailing.domain/src/com/sap/sailing/domain/markpassingcalculation/impl/CandidateChooserImpl.java @@ -240,7 +240,7 @@ public int compare(Candidate o1, Candidate o2) { } else if (o1 == end || o2 == start) { result = 1; } else { - result = (int) (o1.getTimePoint().asMillis() - o2.getTimePoint().asMillis()); + result = Long.compare(o1.getTimePoint().asMillis(), o2.getTimePoint().asMillis()); if (result == 0) { result = Integer.compare(o1.getOneBasedIndexOfWaypoint(), o2.getOneBasedIndexOfWaypoint()); if (result == 0) { From 9de68eb70dfbbd42413cea91aef35696c54861b7 Mon Sep 17 00:00:00 2001 From: Axel Uhl Date: Thu, 1 Oct 2026 17:09:08 +0200 Subject: [PATCH 2/3] bug6279: prove candidate time comparator overflow with a regression test The StartAndEndAwareTimeBasedCandidateComparator compared candidate time points with (int) (o1.getTimePoint().asMillis() - o2.getTimePoint().asMillis()). Epoch milliseconds are ~1.7e12, so for candidates more than Integer.MAX_VALUE ms (~24.8 days) apart the 64-bit difference is truncated to 32 bits and its sign can flip, turning the comparator into a non-total order. On the "my" server this pinned all background-executor threads in TreeSet/NavigableSet navigation and starved the maneuver-calculation queue (live candidate set span was ~355.9 days, 14.3x the int limit). CandidateComparatorOverflowTest reproduces the mechanism in isolation against plain long epoch millis, independent of any domain type: - truncatingCastReportsACyclicOrder: a single >2^31 ms step overflows and flips the sign, producing the cycle T_LOW > T_MID > T_HIGH > T_LOW. - treeSetNavigationIsConsistentOnlyWithFixedComparator: the broken order makes TreeSet first()/last() disagree with the true chronological extremes. - convergenceLoopTerminatesOnlyWithFixedComparator: an updateCandidates-shaped loop over a realistically sized multi-cluster set never drains under the buggy comparator (hits a 1,000,000-iteration cap) but drains in ~50 passes with Long.compare. Assisted-By: Claude Opus 4.8 (Claude Code) --- .../impl/CandidateComparatorOverflowTest.java | 176 ++++++++++++++++++ 1 file changed, 176 insertions(+) create mode 100644 java/com.sap.sailing.domain.test/src/com/sap/sailing/domain/markpassingcalculation/impl/CandidateComparatorOverflowTest.java diff --git a/java/com.sap.sailing.domain.test/src/com/sap/sailing/domain/markpassingcalculation/impl/CandidateComparatorOverflowTest.java b/java/com.sap.sailing.domain.test/src/com/sap/sailing/domain/markpassingcalculation/impl/CandidateComparatorOverflowTest.java new file mode 100644 index 00000000000..c93ce48e1fa --- /dev/null +++ b/java/com.sap.sailing.domain.test/src/com/sap/sailing/domain/markpassingcalculation/impl/CandidateComparatorOverflowTest.java @@ -0,0 +1,176 @@ +package com.sap.sailing.domain.markpassingcalculation.impl; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.Comparator; +import java.util.HashSet; +import java.util.NavigableSet; +import java.util.Set; +import java.util.TreeSet; + +import org.junit.jupiter.api.Test; + +/** + * Reproduces, in isolation, the defect that pinned all background-executor threads on the "my" server: the + * {@code CandidateChooserImpl.StartAndEndAwareTimeBasedCandidateComparator} compared candidate time points with + * {@code (int) (o1.getTimePoint().asMillis() - o2.getTimePoint().asMillis())}. Epoch milliseconds are on the order + * of 1.7e12, so for candidates more than {@link Integer#MAX_VALUE} ms (~24.8 days) apart the 64-bit difference is + * truncated to 32 bits and its sign can flip, turning the comparator into a non-total order. + *

+ * These tests depend on no domain type; they exercise the exact arithmetic against plain {@code long} epoch + * milliseconds so the mechanism is proven deterministically and the test doubles as a regression guard. + * {@link #BUGGY} is the original expression; {@link #FIXED} is {@link Long#compare(long, long)}. + */ +public class CandidateComparatorOverflowTest { + /** The original, overflow-prone time comparison. */ + private static final Comparator BUGGY = (final Long a, final Long b) -> (int) (a - b); + /** The overflow-safe replacement. */ + private static final Comparator FIXED = (final Long a, final Long b) -> Long.compare(a, b); + /** + * Three realistic epoch-millis time points in true chronological order {@code T_LOW < T_MID < T_HIGH}. Each + * adjacent step is ~3e9 ms (~34.7 days), comfortably above {@link Integer#MAX_VALUE} (~24.8 days), so a single + * step already overflows the {@code (int)} cast and flips its sign. All three fall inside a plausible multi-week + * data range for a long-running tracked race. + */ + private static final long T_LOW = 1_700_000_000_000L; // ~2023-11-14 + private static final long T_MID = T_LOW + 3_000_000_000L; // +~34.7 days + private static final long T_HIGH = T_MID + 3_000_000_000L; // +~34.7 days + + /** + * The buggy comparator reports a cycle on the true chronological order: {@code T_LOW > T_MID} and + * {@code T_MID > T_HIGH} yet {@code T_LOW < T_HIGH}. A {@link Comparator} contract requires a total order, so no + * value can be simultaneously greater than its successor and less than its successor's successor. The fixed + * comparator reports the one true order. + */ + @Test + public void truncatingCastReportsACyclicOrder() { + // The fixed comparator honours the true chronological order throughout. + assertTrue(FIXED.compare(T_LOW, T_MID) < 0); + assertTrue(FIXED.compare(T_MID, T_HIGH) < 0); + assertTrue(FIXED.compare(T_LOW, T_HIGH) < 0); + // The buggy comparator flips each single ~3e9 ms step: it claims the earlier point is the greater one... + assertTrue(BUGGY.compare(T_LOW, T_MID) > 0, "a single >2^31 ms step must overflow and flip the sign"); + assertTrue(BUGGY.compare(T_MID, T_HIGH) > 0, "a single >2^31 ms step must overflow and flip the sign"); + // ...while the ~6e9 ms end-to-end gap wraps a second time and comes back positive, so T_LOW < T_HIGH again. + // Together these three answers form the cycle T_LOW > T_MID > T_HIGH > T_LOW: not a total order. + assertTrue(BUGGY.compare(T_LOW, T_HIGH) < 0, "the ~6e9 ms gap wraps twice and reports T_LOW < T_HIGH"); + } + + /** + * On a larger, clustered set the broken order makes {@link TreeSet} navigation disagree with the elements actually + * present: the first/last of the sorted view are not the true chronological extremes, so {@code tailSet}-based + * navigation (what {@code getTimeWiseContiguousCandidates} relies on) walks into the wrong neighbourhood. The fixed + * comparator yields the true extremes. + */ + @Test + public void treeSetNavigationIsConsistentOnlyWithFixedComparator() { + final NavigableSet fixed = buildClusteredSet(FIXED); + assertEquals(T_LOW, fixed.first(), "fixed order must expose the earliest time point as first()"); + assertEquals(T_HIGH + 3_000L, fixed.last(), "fixed order must expose the latest time point as last()"); + final NavigableSet buggy = buildClusteredSet(BUGGY); + final boolean extremesAreTrue = buggy.first() == T_LOW && buggy.last() == T_HIGH + 3_000L; + assertTrue(!extremesAreTrue, "broken order must misplace the chronological extremes in the sorted view"); + } + + /** + * Models the convergence loop of {@code MostProbableCandidatesInSmallTimeRangeFilter.updateCandidates}: repeatedly + * take the next candidate, compute the contiguous time-wise sequence around it, and remove that whole sequence from + * the working set. With a correct order the working set shrinks every iteration and the loop terminates; with the + * broken order the "contiguous sequence" computed via {@code tailSet} does not line up with the elements actually + * present, so progress stalls and the set never drains. The iteration cap turns the server's silent hang into a + * loud, bounded test failure. + */ + @Test + public void convergenceLoopTerminatesOnlyWithFixedComparator() { + final DrainOutcome fixed = drain(FIXED); + assertTrue(fixed.converged, "with Long.compare the convergence loop must terminate"); + assertTrue(fixed.coversEveryCandidate, "with Long.compare every candidate must be consumed exactly once"); + // The buggy comparator does not drain within a very high iteration cap: in production this is the thread spin + // observed on the server. It also fails to ever reach most candidates, so even the capped run is incomplete. + final DrainOutcome buggy = drain(BUGGY); + assertTrue(!buggy.converged, "with the (int) cast the convergence loop fails to converge within the cap"); + assertTrue(!buggy.coversEveryCandidate, "with the (int) cast most candidates are never reached"); + } + + private NavigableSet buildClusteredSet(final Comparator comparator) { + final NavigableSet set = new TreeSet<>(comparator); + for (final long base : new long[] { T_LOW, T_MID, T_HIGH }) { + for (int i = 0; i < 4; i++) { + set.add(base + i * 1_000L); + } + } + return set; + } + + /** + * Builds a realistically-sized candidate set: {@code clusterCount} time clusters whose bases are {@code baseStepMs} + * apart (chosen above {@link Integer#MAX_VALUE} so the {@code (int)} cast overflows between clusters), each cluster + * holding {@code perCluster} points one second apart. This mirrors a long-running tracked race with many maneuver + * candidates; the single-cluster toy set of {@link #buildClusteredSet} is too small to exhibit the hang. + */ + private NavigableSet buildLargeMultiClusterSet(final Comparator comparator) { + final int clusterCount = 50; + final int perCluster = 50; + final long baseStepMs = 2_200_000_000L; // ~25.5 days, so one step already overflows the (int) cast + final NavigableSet set = new TreeSet<>(comparator); + for (int cluster = 0; cluster < clusterCount; cluster++) { + final long base = T_LOW + cluster * baseStepMs; + for (int i = 0; i < perCluster; i++) { + set.add(base + i * 1_000L); + } + } + return set; + } + + /** + * Runs the {@code updateCandidates}-style convergence loop over a large multi-cluster set and reports whether it + * drained within a high iteration cap and whether it ever reached every candidate. With a correct order the loop + * removes a whole contiguous cluster each pass and finishes in a few passes; with the broken order {@code first()} + * and {@code tailSet} disagree with the true chronology, so the same region is revisited and the set never drains. + * + * @return the outcome: {@code converged} is {@code false} when the cap was hit (the production loop would spin + * forever), and {@code coversEveryCandidate} is {@code false} when candidates were left unreachable. + */ + private DrainOutcome drain(final Comparator comparator) { + final long windowMs = 5_000L; // CANDIDATE_FILTER_TIME_WINDOW + final NavigableSet all = buildLargeMultiClusterSet(comparator); + final NavigableSet working = new TreeSet<>(comparator); + working.addAll(all); + final Set everRemoved = new HashSet<>(); + final int cap = 1_000_000; // orders of magnitude above the ~50 passes a correct run needs + int iterations = 0; + boolean converged = true; + while (!working.isEmpty() && converged) { + if (++iterations > cap) { + converged = false; // did not converge: the production loop would spin here forever + } else { + final long startFrom = working.first(); + final NavigableSet sequence = new TreeSet<>(comparator); + sequence.add(startFrom); + long current = startFrom; + for (final long next : all.tailSet(startFrom, false)) { + final boolean withinWindow = Math.abs(next - current) <= windowMs; + if (withinWindow) { + sequence.add(next); + current = next; + } + } + everRemoved.addAll(sequence); + working.removeAll(sequence); + } + } + return new DrainOutcome(converged, everRemoved.equals(all)); + } + + /** The result of a convergence-loop simulation: did it terminate, and did it reach every candidate. */ + private static final class DrainOutcome { + private final boolean converged; + private final boolean coversEveryCandidate; + + private DrainOutcome(final boolean converged, final boolean coversEveryCandidate) { + this.converged = converged; + this.coversEveryCandidate = coversEveryCandidate; + } + } +} From f5aea106a909d92d7d9cffa23663adabf40e4cd1 Mon Sep 17 00:00:00 2001 From: Axel Uhl Date: Thu, 1 Oct 2026 17:23:01 +0200 Subject: [PATCH 3/3] release notes for MPC candidate time comparison fix --- .../resources/SailingAnalyticsNotes.html | 9 + ... Jetty on 8889, auto-replicate dev).launch | 717 +++++++++--------- 2 files changed, 365 insertions(+), 361 deletions(-) diff --git a/java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/home/desktop/places/whatsnew/resources/SailingAnalyticsNotes.html b/java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/home/desktop/places/whatsnew/resources/SailingAnalyticsNotes.html index 6afba8b5617..0c8ccc492be 100755 --- a/java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/home/desktop/places/whatsnew/resources/SailingAnalyticsNotes.html +++ b/java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/home/desktop/places/whatsnew/resources/SailingAnalyticsNotes.html @@ -6,6 +6,15 @@

What's New - Sailing Analytics

+
October 2026
+
    +
  • Mark passing calculator bug fix: for races with tracking duration longer than ~14 days + (often a race accidentally left open and then later catching new fixes that actually wouldn't + belong there), the sorting of mark passing candidates in an internal data structure may + have failed due to a long/int data type overflow, in effect potentially leading to + endless iterations across those mark passings candidates and delaying the start-up of + a server process, possibly infinitely.
    • +
September 2026
  • Added boat class Yngling.
    • diff --git a/java/com.sap.sailing.server/SailingServer (No Proxy, Jetty on 8889, auto-replicate dev).launch b/java/com.sap.sailing.server/SailingServer (No Proxy, Jetty on 8889, auto-replicate dev).launch index bd4d2ecec25..92c7c39af05 100755 --- a/java/com.sap.sailing.server/SailingServer (No Proxy, Jetty on 8889, auto-replicate dev).launch +++ b/java/com.sap.sailing.server/SailingServer (No Proxy, Jetty on 8889, auto-replicate dev).launch @@ -25,368 +25,363 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +