Added synchronization for MarkPassing collections, fixing bug 342

2026-10-02 02:13:55 +00:00 · 2012-02-29 12:07:49 +01:00
1 parent 2afb74b95a
commit f36a657dc2
@@ -7,6 +7,7 @@ import java.text.ParseException;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
+import java.util.NavigableSet;
 import java.util.Set;
 import java.util.TreeMap;
 import java.util.logging.Logger;
@@ -193,9 +194,13 @@ public class SwissTimingRaceTrackerImpl extends AbstractRaceTrackerImpl implemen
            // we need to use this to *update* the competitor's mark passings list, not *replace* it
            TreeMap<Integer, MarkPassing> markPassingsByMarkIndex = new TreeMap<Integer, MarkPassing>();
            // now fill with the already existing mark passings for the competitor identified by boatID...
-            for (MarkPassing markPassing : trackedRace.getMarkPassings(competitor)) {
-                markPassingsByMarkIndex.put(
-                        trackedRace.getRace().getCourse().getIndexOfWaypoint(markPassing.getWaypoint()), markPassing);
+            NavigableSet<MarkPassing> markPassings = trackedRace.getMarkPassings(competitor);
+            synchronized (markPassings) {
+                for (MarkPassing markPassing : markPassings) {
+                    markPassingsByMarkIndex.put(
+                            trackedRace.getRace().getCourse().getIndexOfWaypoint(markPassing.getWaypoint()),
+                            markPassing);
+                }
            }
            // ...and then overwrite those for which we received "new evidence"
            for (Triple<Integer, Integer, Long> markIndexRankAndTimeSinceStartInMilliseconds : markIndicesRanksAndTimesSinceStartInMilliseconds) {
@@ -2,6 +2,7 @@ package com.sap.sailing.domain.tracking;

 import java.util.List;
 import java.util.NavigableSet;
+import java.util.SortedSet;

 import com.sap.sailing.domain.base.Buoy;
 import com.sap.sailing.domain.base.Competitor;
@@ -148,6 +149,10 @@ public interface TrackedRace {
     * which point in time. This can, e.g., be used to sort those competitors who already finished a leg within the leg
     * that ends with <code>waypoint</code>. The remaining competitors need to be ordered by the advantage line-related
     * distance to the waypoint.
+     * 
+     * @return the iterable sequence of {@link MarkPassing}s as described above. To iterate on the resulting collection
+     *         the caller needs to synchronize on the iterable returned because insertions into the underlying
+     *         collection will also synchronize on that collection.
     */
    Iterable<MarkPassing> getMarkPassingsInOrder(Waypoint waypoint);

@@ -199,7 +204,10 @@ public interface TrackedRace {
     * returned such that their {@link MarkPassing#getWaypoint() waypoints} are ordered in the same way they are ordered
     * in the race's {@link Course}. Note, that this doesn't necessarily guarantee ascending time points, particularly
     * if premature mark passings have been detected accidentally as can be the case with some tracking providers such
-     * as TracTrac.
+     * as TracTrac. If the caller wants to iterate on the resulting collection or construct a {@link SortedSet#headSet(Object)}
+     * or {@link SortedSet#tailSet(Object)} and then iterate over that, the caller needs to synchronize on the
+     * collection returned because insertions into the competitor's mark passing collection will also synchronize
+     * on that collection.
     */
    NavigableSet<MarkPassing> getMarkPassings(Competitor competitor);

@@ -240,15 +240,22 @@ public class DynamicTrackedRaceImpl extends TrackedRaceImpl implements
    public void updateMarkPassings(Competitor competitor, Iterable<MarkPassing> markPassings) {
        Map<Waypoint, MarkPassing> oldMarkPassings = new HashMap<Waypoint, MarkPassing>();
        synchronized (this) {
-            for (MarkPassing oldMarkPassing : getMarkPassings(competitor)) {
-                oldMarkPassings.put(oldMarkPassing.getWaypoint(), oldMarkPassing);
+            NavigableSet<MarkPassing> markPassingsForCompetitor = getMarkPassings(competitor);
+            synchronized (markPassingsForCompetitor) {
+                for (MarkPassing oldMarkPassing : markPassingsForCompetitor) {
+                    oldMarkPassings.put(oldMarkPassing.getWaypoint(), oldMarkPassing);
+                }
            }
            clearMarkPassings(competitor);
-            NavigableSet<MarkPassing> competitorMarkPassings = getMarkPassings(competitor);
            TimePoint timePointOfLatestEvent = new MillisecondsTimePoint(0);
            for (MarkPassing markPassing : markPassings) {
-                competitorMarkPassings.add(markPassing);
-                getMarkPassingsInOrder(markPassing.getWaypoint()).add(markPassing);
+                synchronized (markPassingsForCompetitor) {
+                    markPassingsForCompetitor.add(markPassing);
+                }
+                Collection<MarkPassing> markPassingsInOrderForWaypoint = getMarkPassingsInOrder(markPassing.getWaypoint());
+                synchronized (markPassingsInOrderForWaypoint) {
+                    markPassingsInOrderForWaypoint.add(markPassing);
+                }
                if (markPassing.getTimePoint().compareTo(timePointOfLatestEvent) > 0) {
                    timePointOfLatestEvent = markPassing.getTimePoint();
                }
@@ -267,11 +274,17 @@ public class DynamicTrackedRaceImpl extends TrackedRaceImpl implements
    }

    private void clearMarkPassings(Competitor competitor) {
-        Iterator<MarkPassing> mpIter = getMarkPassings(competitor).iterator();
-        while (mpIter.hasNext()) {
-            MarkPassing mp = mpIter.next();
-            mpIter.remove();
-            getMarkPassingsInOrder(mp.getWaypoint()).remove(mp);
+        NavigableSet<MarkPassing> markPassings = getMarkPassings(competitor);
+        synchronized (markPassings) {
+            Iterator<MarkPassing> mpIter = markPassings.iterator();
+            while (mpIter.hasNext()) {
+                MarkPassing mp = mpIter.next();
+                mpIter.remove();
+                Collection<MarkPassing> markPassingsInOrder = getMarkPassingsInOrder(mp.getWaypoint());
+                synchronized (markPassingsInOrder) {
+                    markPassingsInOrder.remove(mp);
+                }
+            }
        }
    }

@@ -43,28 +43,35 @@ public class RaceRankComparator implements Comparator<Competitor> {
        if (o1 == o2) {
            result = 0;
        } else {
-            // TODO see also bug 340/342; need to synchronize on TrackedRace to avoid concurrent updates to MarkPassings, although this is course-grained
-            NavigableSet<MarkPassing> o1MarkPassings = trackedRace.getMarkPassings(o1).headSet(
-                    markPassingWithTimePoint, /* inclusive */true);
-            NavigableSet<MarkPassing> o2MarkPassings = trackedRace.getMarkPassings(o2).headSet(
-                    markPassingWithTimePoint, /* inclusive */true);
-            result = o2MarkPassings.size() - o1MarkPassings.size(); // inverted: more legs means smaller rank
-            if (result == 0 && o1MarkPassings.size() > 0) {
-                // Competitors are on same leg and both have already started the first leg.
-                // TrackedLegOfCompetitor comparison also correctly uses finish times for a leg
-                // in case we have the final leg, so both competitors finished the race.
-                TrackedLegOfCompetitor o1Leg = trackedRace.getCurrentLeg(o1, timePoint);
-                if (o1Leg == null) {
-                    // both must already finished race; sort by race finish time: earlier time means smaller (better)
-                    // rank
-                    result = o1MarkPassings.last().getTimePoint().compareTo(o2MarkPassings.last().getTimePoint());
-                } else {
-                    TrackedLegOfCompetitor o2Leg = trackedRace.getCurrentLeg(o2, timePoint);
-                    if (o2Leg == null) {
-                        result = 1; // o1Leg != null, so o1 has started leg already, o2 hasn't
-                    } else {
-                        result = new WindwardToGoComparator(trackedRace.getTrackedLeg(o1Leg.getLeg()), timePoint)
-                                .compare(o1Leg, o2Leg);
+            NavigableSet<MarkPassing> o1MarkPassings = trackedRace.getMarkPassings(o1);
+            synchronized (o1MarkPassings) {
+                NavigableSet<MarkPassing> o2MarkPassings = trackedRace.getMarkPassings(o2);
+                synchronized (o2MarkPassings) {
+                    NavigableSet<MarkPassing> o1MarkPassingsBeforeTimePoint = o1MarkPassings.headSet(
+                            markPassingWithTimePoint, /* inclusive */true);
+                    NavigableSet<MarkPassing> o2MarkPassingsBeforeTimePoint = o2MarkPassings.headSet(
+                            markPassingWithTimePoint, /* inclusive */true);
+                    result = o2MarkPassingsBeforeTimePoint.size() - o1MarkPassingsBeforeTimePoint.size(); // inverted: more legs means smaller rank
+                    if (result == 0 && o1MarkPassingsBeforeTimePoint.size() > 0) {
+                        // Competitors are on same leg and both have already started the first leg.
+                        // TrackedLegOfCompetitor comparison also correctly uses finish times for a leg
+                        // in case we have the final leg, so both competitors finished the race.
+                        TrackedLegOfCompetitor o1Leg = trackedRace.getCurrentLeg(o1, timePoint);
+                        if (o1Leg == null) {
+                            // both must already finished race; sort by race finish time: earlier time means smaller
+                            // (better)
+                            // rank
+                            result = o1MarkPassingsBeforeTimePoint.last().getTimePoint()
+                                    .compareTo(o2MarkPassingsBeforeTimePoint.last().getTimePoint());
+                        } else {
+                            TrackedLegOfCompetitor o2Leg = trackedRace.getCurrentLeg(o2, timePoint);
+                            if (o2Leg == null) {
+                                result = 1; // o1Leg != null, so o1 has started leg already, o2 hasn't
+                            } else {
+                                result = new WindwardToGoComparator(trackedRace.getTrackedLeg(o1Leg.getLeg()),
+                                        timePoint).compare(o1Leg, o2Leg);
+                            }
+                        }
                    }
                }
            }
@@ -374,13 +374,17 @@ public class TrackedLegOfCompetitorImpl implements TrackedLegOfCompetitor {
        if (windwardSpeed == null) {
            return null;
        } else {
-            Iterator<MarkPassing> markPassingsForLegEnd = getTrackedRace().getMarkPassingsInOrder(getLeg().getTo())
-                    .iterator();
            // Has our competitor started the leg already? If not, we won't be able to compute a gap
            if (hasStartedLeg(timePoint)) {
-                if (markPassingsForLegEnd.hasNext()) {
+                Iterable<MarkPassing> markPassingsInOrder = getTrackedRace().getMarkPassingsInOrder(getLeg().getTo());
+                MarkPassing firstMarkPassing = null;
+                synchronized (markPassingsInOrder) {
+                    Iterator<MarkPassing> markPassingsForLegEnd = markPassingsInOrder.iterator();
+                    firstMarkPassing = markPassingsForLegEnd.next();
+                }
+                if (firstMarkPassing != null) {
                    // someone has already finished the leg
-                    TimePoint whenLeaderFinishedLeg = markPassingsForLegEnd.next().getTimePoint();
+                    TimePoint whenLeaderFinishedLeg = firstMarkPassing.getTimePoint();
                    // Was it before the requested timePoint?
                    if (whenLeaderFinishedLeg.compareTo(timePoint) <= 0) {
                        // Has our competitor also already finished this leg?
@@ -208,9 +208,15 @@ public abstract class TrackedRaceImpl implements TrackedRace, CourseListener {
    @Override
    public TimePoint getStart() {
        TimePoint result;
-        Iterator<MarkPassing> markPassingsFirstMarkIter = getMarkPassingsInOrder(getRace().getCourse().getFirstWaypoint()).iterator();
-        if (markPassingsFirstMarkIter.hasNext()) {
-            MarkPassing firstMarkPassingFirstMark = markPassingsFirstMarkIter.next();
+        Iterable<MarkPassing> markPassingsInOrder = getMarkPassingsInOrder(getRace().getCourse().getFirstWaypoint());
+        MarkPassing firstMarkPassingFirstMark = null;
+        synchronized (markPassingsInOrder) {
+            Iterator<MarkPassing> markPassingsFirstMarkIter = markPassingsInOrder.iterator();
+            if (markPassingsFirstMarkIter.hasNext()) {
+                firstMarkPassingFirstMark = markPassingsFirstMarkIter.next();
+            }
+        }
+        if (firstMarkPassingFirstMark != null) {
            TimePoint timeOfFirstMarkPassingFirstMark = firstMarkPassingFirstMark.getTimePoint();
            if (startTimeReceived != null) {
                long startTimeReceived2timeOfFirstMarkPassingFirstMark = timeOfFirstMarkPassingFirstMark.asMillis() -
@@ -232,8 +238,11 @@ public abstract class TrackedRaceImpl implements TrackedRace, CourseListener {
    @Override
    public TimePoint getAssumedEnd() {
        TimePoint result = null;
-        for (MarkPassing passingFinishLine : getMarkPassingsInOrder(getRace().getCourse().getLastWaypoint())) {
-            result = passingFinishLine.getTimePoint();
+        Iterable<MarkPassing> markPassingsInOrder = getMarkPassingsInOrder(getRace().getCourse().getLastWaypoint());
+        synchronized (markPassingsInOrder) {
+            for (MarkPassing passingFinishLine : markPassingsInOrder) {
+                result = passingFinishLine.getTimePoint();
+            }
        }
        return result;
    }
@@ -756,19 +765,20 @@ public abstract class TrackedRaceImpl implements TrackedRace, CourseListener {
                    if (!track.hasDirectionChange(timePoint, getManeuverDegreeAngleThreshold())) {
                        // reduce confidence around mark passings:
                        NavigableSet<MarkPassing> markPassings = getMarkPassings(competitor);
-                        NavigableSet<MarkPassing> prevMarkPassing = markPassings.headSet(dummyMarkPassingForNow, /* inclusive */ true);
-                        NavigableSet<MarkPassing> nextMarkPassing = markPassings.tailSet(dummyMarkPassingForNow, /* inclusive */ true);
                        double markPassingProximityConfidenceReduction = 1.0;
-                        if (prevMarkPassing != null && !prevMarkPassing.isEmpty()) {
-                            markPassingProximityConfidenceReduction *= Math.max(0.0,
-                                    1.0-weigherForMarkPassingProximity.getConfidence(prevMarkPassing.last().getTimePoint(), timePoint));
+                        synchronized (markPassings) {
+                            NavigableSet<MarkPassing> prevMarkPassing = markPassings.headSet(dummyMarkPassingForNow, /* inclusive */ true);
+                            NavigableSet<MarkPassing> nextMarkPassing = markPassings.tailSet(dummyMarkPassingForNow, /* inclusive */ true);
+                            if (prevMarkPassing != null && !prevMarkPassing.isEmpty()) {
+                                markPassingProximityConfidenceReduction *= Math.max(0.0,
+                                        1.0-weigherForMarkPassingProximity.getConfidence(prevMarkPassing.last().getTimePoint(), timePoint));
+                            }
+                            if (nextMarkPassing != null && !nextMarkPassing.isEmpty()) {
+                                markPassingProximityConfidenceReduction *= Math.max(0.0,
+                                        1.0-weigherForMarkPassingProximity.getConfidence(nextMarkPassing.first().getTimePoint(), timePoint));
+                            }
                        }
-                        if (nextMarkPassing != null && !nextMarkPassing.isEmpty()) {
-                            markPassingProximityConfidenceReduction *= Math.max(0.0,
-                                    1.0-weigherForMarkPassingProximity.getConfidence(nextMarkPassing.first().getTimePoint(), timePoint));
-                        }
-                        SpeedWithBearingWithConfidence<TimePoint> estimatedSpeedWithConfidence = track.getEstimatedSpeed(timePoint,
-                                weigher);
+                        SpeedWithBearingWithConfidence<TimePoint> estimatedSpeedWithConfidence = track.getEstimatedSpeed(timePoint, weigher);
                        if (estimatedSpeedWithConfidence != null) {
                            BearingWithConfidence<TimePoint> bearing = new BearingWithConfidenceImpl<TimePoint>(
                                    estimatedSpeedWithConfidence.getObject() == null ? null : estimatedSpeedWithConfidence.getObject().getBearing(),
@@ -1117,13 +1127,18 @@ public abstract class TrackedRaceImpl implements TrackedRace, CourseListener {
        Placemark startBest = null;
        Placemark finishBest = null;
        
-        //Get start postition
+        // Get start postition
        Waypoint start = getRace().getCourse().getFirstWaypoint();
        Iterable<MarkPassing> startPassings = getMarkPassingsInOrder(start);
-        if (startPassings.iterator().hasNext()) {
-            MarkPassing startPassing = startPassings.iterator().next();
+        MarkPassing startPassing = null;
+        synchronized (startPassings) {
+            Iterator<MarkPassing> iterator = startPassings.iterator();
+            if (iterator.hasNext()) {
+                startPassing = iterator.next();
+            }
+        }
+        if (startPassing != null) {
            Position startPosition = getApproximatePosition(start, startPassing.getTimePoint());
-
            try {
                // Get distance to nearest placemark and calculate the search radius
                Placemark startNearest = ReverseGeocoder.INSTANCE.getPlacemarkNearest(startPosition);
@@ -1142,13 +1157,14 @@ public abstract class TrackedRaceImpl implements TrackedRace, CourseListener {
            // Get finish position
            Waypoint finish = getRace().getCourse().getLastWaypoint();
            Iterable<MarkPassing> finishPassings = getMarkPassingsInOrder(finish);
-            Iterator<MarkPassing> finishPassingsIterator = finishPassings.iterator();
            MarkPassing finishPassing = null;
-            while (finishPassingsIterator.hasNext()) {
-                finishPassing = (MarkPassing) finishPassingsIterator.next();
+            synchronized (finishPassings) {
+                Iterator<MarkPassing> finishPassingsIterator = finishPassings.iterator();
+                while (finishPassingsIterator.hasNext()) {
+                    finishPassing = (MarkPassing) finishPassingsIterator.next();
+                }
            }
            Position finishPosition = getApproximatePosition(finish, finishPassing.getTimePoint());
-
            if (startPosition.getDistance(finishPosition).getKilometers() > ReverseGeocoder.POSITION_CACHE_DISTANCE_LIMIT_IN_KM) {
                try {
                    // Get distance to nearest placemark and calculate the search radius
@@ -769,32 +769,35 @@ public class SailingServiceImpl extends RemoteServiceServlet implements SailingS
                    // try to calculate the point in time where all boats passed the start line in a small time frame
                    Waypoint from = trackedLeg.getLeg().getFrom();
                    Iterable<MarkPassing> markPassings = trackedRace.getMarkPassingsInOrder(from);
-                    int markPassingsCount = Util.size(markPassings);
-                    // require more than half the boats registered for the race to have passed the start mark
-                    if (markPassings != null && markPassingsCount > Util.size(trackedRace.getRace().getCompetitors())/2) {
-                        long maxTimeFrameInMs = 60 * 1000; // = 1 minute
-                        Iterator<MarkPassing> iterator = markPassings.iterator();
-                        long currentTimeToCheck = 0;
-                        int enoughCompetitors = markPassingsCount / 2;
-                        int currentCompetitorsInTime = 0;
-                        
-                        while (iterator.hasNext()) { 
-                            MarkPassing currentMarkPassing = iterator.next();
-                            long diff = currentMarkPassing.getTimePoint().asMillis() - currentTimeToCheck;
-                            if (diff > maxTimeFrameInMs) {
-                                // reset the check
-                                currentCompetitorsInTime = 0;
-                                currentTimeToCheck = currentMarkPassing.getTimePoint().asMillis();
-                            } else {
-                                currentCompetitorsInTime++;
-                                if(currentCompetitorsInTime == enoughCompetitors) {
-                                    Date passingDate = currentMarkPassing.getTimePoint().asDate();
-                                    LegTimesInfoDTO legTimepointDTO = new LegTimesInfoDTO("S");
-                                    legTimepointDTO.firstPassingDate = passingDate; 
-                                    legTimes.add(legTimepointDTO);
-                                    raceTimesInfo.setStartOfRace(passingDate);
-                                    lastLegPassingTime = passingDate;
-                                    break;
+                    synchronized (markPassings) {
+                        int markPassingsCount = Util.size(markPassings);
+                        // require more than half the boats registered for the race to have passed the start mark
+                        if (markPassings != null
+                                && markPassingsCount > Util.size(trackedRace.getRace().getCompetitors()) / 2) {
+                            long maxTimeFrameInMs = 60 * 1000; // = 1 minute
+                            Iterator<MarkPassing> iterator = markPassings.iterator();
+                            long currentTimeToCheck = 0;
+                            int enoughCompetitors = markPassingsCount / 2;
+                            int currentCompetitorsInTime = 0;
+
+                            while (iterator.hasNext()) {
+                                MarkPassing currentMarkPassing = iterator.next();
+                                long diff = currentMarkPassing.getTimePoint().asMillis() - currentTimeToCheck;
+                                if (diff > maxTimeFrameInMs) {
+                                    // reset the check
+                                    currentCompetitorsInTime = 0;
+                                    currentTimeToCheck = currentMarkPassing.getTimePoint().asMillis();
+                                } else {
+                                    currentCompetitorsInTime++;
+                                    if (currentCompetitorsInTime == enoughCompetitors) {
+                                        Date passingDate = currentMarkPassing.getTimePoint().asDate();
+                                        LegTimesInfoDTO legTimepointDTO = new LegTimesInfoDTO("S");
+                                        legTimepointDTO.firstPassingDate = passingDate;
+                                        legTimes.add(legTimepointDTO);
+                                        raceTimesInfo.setStartOfRace(passingDate);
+                                        lastLegPassingTime = passingDate;
+                                        break;
+                                    }
                                }
                            }
                        }
@@ -807,17 +810,19 @@ public class SailingServiceImpl extends RemoteServiceServlet implements SailingS
                    // ensure the passing date is in the right time order; there may perhaps be left-overs for marks to be reached later that
                    // claim it has been passed in the past which may have been an accidental tracker read-out;
                    // the results of getMarkPassingsInOrder(to) has by definition an ascending time-point ordering
-                    for (MarkPassing currentMarkPassing : markPassings) {
-                        Date currentPassingDate = currentMarkPassing.getTimePoint().asDate();
-                        if (lastLegPassingTime == null) {
-                            lastLegPassingTime = currentPassingDate;
-                        }
-                        if (currentPassingDate.after(lastLegPassingTime)) {
-                            LegTimesInfoDTO legTimepointDTO = new LegTimesInfoDTO("L" + i++);
-                            legTimepointDTO.firstPassingDate = currentPassingDate; 
-                            legTimes.add(legTimepointDTO);
-                            lastLegPassingTime = currentPassingDate;
-                            break;
+                    synchronized (markPassings) {
+                        for (MarkPassing currentMarkPassing : markPassings) {
+                            Date currentPassingDate = currentMarkPassing.getTimePoint().asDate();
+                            if (lastLegPassingTime == null) {
+                                lastLegPassingTime = currentPassingDate;
+                            }
+                            if (currentPassingDate.after(lastLegPassingTime)) {
+                                LegTimesInfoDTO legTimepointDTO = new LegTimesInfoDTO("L" + i++);
+                                legTimepointDTO.firstPassingDate = currentPassingDate;
+                                legTimes.add(legTimepointDTO);
+                                lastLegPassingTime = currentPassingDate;
+                                break;
+                            }
                        }
                    }
                }