bug 2077: protecting mark passing structures during TrackedRace serialization; don't serialize rank cache in TrackedLegImpl, avoiding CME

java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/DynamicTrackedRaceImpl.java

@@ -581,96 +581,106 @@ DynamicTrackedRace, GPSTrackListener<Competitor, GPSFixMoving> {
 
     @Override
     public void updateMarkPassings(Competitor competitor, Iterable<MarkPassing> markPassings) {
-        Map<Waypoint, MarkPassing> oldMarkPassings = new HashMap<Waypoint, MarkPassing>();
-        MarkPassing oldStartMarkPassing = null;
-        TimePoint oldStartOfRace = getStartOfRace(); // getStartOfRace() may respond with a new result already after updating the mark passings
-        boolean requiresStartTimeUpdate = true;
-        final NavigableSet<MarkPassing> markPassingsForCompetitor = getMarkPassings(competitor);
-        lockForRead(markPassingsForCompetitor);
+        LockUtil.lockForRead(getSerializationLock()); // keep serializer from reading the mark passings collections
         try {
-            for (MarkPassing oldMarkPassing : markPassingsForCompetitor) {
-                if (oldStartMarkPassing == null) {
-                    oldStartMarkPassing = oldMarkPassing;
-                }
-                oldMarkPassings.put(oldMarkPassing.getWaypoint(), oldMarkPassing);
-            }
-        } finally {
-            unlockAfterRead(markPassingsForCompetitor);
-        }
-        final NamedReentrantReadWriteLock markPassingsLock = getMarkPassingsLock(markPassingsForCompetitor);
-        TimePoint timePointOfLatestEvent = new MillisecondsTimePoint(0);
-        // Make sure that clearMarkPassings and the re-adding of the mark passings are non-interruptible by readers.
-        // Note that the write lock for the mark passings in order per waypoint is obtained inside clearMarkPassings(...)
-        // as well as inside the subsequent for-loop. It is important to always first obtain the mark passings lock for the competitor
-        // mark passings before obtaining the lock for the mark passings in order for the waypoint to avoid deadlocks.
-        LockUtil.lockForWrite(markPassingsLock);
-        try {
-            clearMarkPassings(competitor);
-            for (MarkPassing markPassing : markPassings) {
-                // try to find corresponding old start mark passing
-                if (oldStartMarkPassing != null
-                        && markPassing.getWaypoint().equals(oldStartMarkPassing.getWaypoint())) {
-                    if (markPassing.getTimePoint() != null && oldStartMarkPassing.getTimePoint() != null
-                            && markPassing.getTimePoint().equals(oldStartMarkPassing.getTimePoint())) {
-                        requiresStartTimeUpdate = false;
+            Map<Waypoint, MarkPassing> oldMarkPassings = new HashMap<Waypoint, MarkPassing>();
+            MarkPassing oldStartMarkPassing = null;
+            TimePoint oldStartOfRace = getStartOfRace(); // getStartOfRace() may respond with a new result already after
+                                                         // updating the mark passings
+            boolean requiresStartTimeUpdate = true;
+            final NavigableSet<MarkPassing> markPassingsForCompetitor = getMarkPassings(competitor);
+            lockForRead(markPassingsForCompetitor);
+            try {
+                for (MarkPassing oldMarkPassing : markPassingsForCompetitor) {
+                    if (oldStartMarkPassing == null) {
+                        oldStartMarkPassing = oldMarkPassing;
                     }
+                    oldMarkPassings.put(oldMarkPassing.getWaypoint(), oldMarkPassing);
                 }
-                if (!Util.contains(getRace().getCourse().getWaypoints(), markPassing.getWaypoint())) {
-                    StringBuilder courseWaypointsWithID = new StringBuilder();
-                    boolean first = true;
-                    for (Waypoint courseWaypoint : getRace().getCourse().getWaypoints()) {
-                        if (first) {
-                            first = false;
-                        } else {
-                            courseWaypointsWithID.append(" -> ");
+            } finally {
+                unlockAfterRead(markPassingsForCompetitor);
+            }
+            final NamedReentrantReadWriteLock markPassingsLock = getMarkPassingsLock(markPassingsForCompetitor);
+            TimePoint timePointOfLatestEvent = new MillisecondsTimePoint(0);
+            // Make sure that clearMarkPassings and the re-adding of the mark passings are non-interruptible by readers.
+            // Note that the write lock for the mark passings in order per waypoint is obtained inside
+            // clearMarkPassings(...) as well as inside the subsequent for-loop. It is important to always first obtain the mark passings lock
+            // for the competitor mark passings before obtaining the lock for the mark passings in order for the waypoint to avoid
+            // deadlocks.
+            LockUtil.lockForWrite(markPassingsLock);
+            try {
+                clearMarkPassings(competitor);
+                for (MarkPassing markPassing : markPassings) {
+                    // try to find corresponding old start mark passing
+                    if (oldStartMarkPassing != null
+                            && markPassing.getWaypoint().equals(oldStartMarkPassing.getWaypoint())) {
+                        if (markPassing.getTimePoint() != null && oldStartMarkPassing.getTimePoint() != null
+                                && markPassing.getTimePoint().equals(oldStartMarkPassing.getTimePoint())) {
+                            requiresStartTimeUpdate = false;
                         }
-                        courseWaypointsWithID.append(courseWaypoint.toString());
-                        courseWaypointsWithID.append(" (ID=");
-                        courseWaypointsWithID.append(courseWaypoint.getId());
-                        courseWaypointsWithID.append(")");
                     }
-                    logger.severe("Received mark passing " + markPassing + " for race " + getRace()
-                            + " for waypoint ID" + markPassing.getWaypoint().getId()
-                            + " but the waypoint does not exist in course " + courseWaypointsWithID);
-                } else {
-                    markPassingsForCompetitor.add(markPassing);
+                    if (!Util.contains(getRace().getCourse().getWaypoints(), markPassing.getWaypoint())) {
+                        StringBuilder courseWaypointsWithID = new StringBuilder();
+                        boolean first = true;
+                        for (Waypoint courseWaypoint : getRace().getCourse().getWaypoints()) {
+                            if (first) {
+                                first = false;
+                            } else {
+                                courseWaypointsWithID.append(" -> ");
+                            }
+                            courseWaypointsWithID.append(courseWaypoint.toString());
+                            courseWaypointsWithID.append(" (ID=");
+                            courseWaypointsWithID.append(courseWaypoint.getId());
+                            courseWaypointsWithID.append(")");
+                        }
+                        logger.severe("Received mark passing " + markPassing + " for race " + getRace()
+                                + " for waypoint ID" + markPassing.getWaypoint().getId()
+                                + " but the waypoint does not exist in course " + courseWaypointsWithID);
+                    } else {
+                        markPassingsForCompetitor.add(markPassing);
+                    }
+                    Collection<MarkPassing> markPassingsInOrderForWaypoint = getOrCreateMarkPassingsInOrderAsNavigableSet(markPassing
+                            .getWaypoint());
+                    final NamedReentrantReadWriteLock markPassingsLock2 = getMarkPassingsLock(markPassingsInOrderForWaypoint);
+                    LockUtil.lockForWrite(markPassingsLock2);
+                    try {
+                        // The mark passings of competitor have been removed by the call to
+                        // clearMarkPassings(competitor) above
+                        // from both, the collection that holds the mark passings by waypoint and the one that holds the
+                        // mark passings per competitor; so we can simply add here:
+                        markPassingsInOrderForWaypoint.add(markPassing);
+                    } finally {
+                        LockUtil.unlockAfterWrite(markPassingsLock2);
+                    }
+                    if (markPassing.getTimePoint().compareTo(timePointOfLatestEvent) > 0) {
+                        timePointOfLatestEvent = markPassing.getTimePoint();
+                    }
                 }
-                Collection<MarkPassing> markPassingsInOrderForWaypoint = getOrCreateMarkPassingsInOrderAsNavigableSet(markPassing
-                        .getWaypoint());
-                final NamedReentrantReadWriteLock markPassingsLock2 = getMarkPassingsLock(markPassingsInOrderForWaypoint);
-                LockUtil.lockForWrite(markPassingsLock2);
-                try {
-                    // The mark passings of competitor have been removed by the call to clearMarkPassings(competitor) above
-                    // from both, the collection that holds the mark passings by waypoint and the one that holds the
-                    // mark passings per competitor; so we can simply add here:
-                    markPassingsInOrderForWaypoint.add(markPassing);
-                } finally {
-                    LockUtil.unlockAfterWrite(markPassingsLock2);
-                }
-                if (markPassing.getTimePoint().compareTo(timePointOfLatestEvent) > 0) {
-                    timePointOfLatestEvent = markPassing.getTimePoint();
+            } finally {
+                LockUtil.unlockAfterWrite(markPassingsLock);
+            }
+            updated(timePointOfLatestEvent);
+            triggerManeuverCacheRecalculation(competitor);
+            // update the race times like start, end and the leg times
+            if (requiresStartTimeUpdate) {
+                TimePoint interimsStartOfRace = getStartOfRace();
+                invalidateStartTime();
+                TimePoint newStartOfRace = getStartOfRace();
+                if (Util.equalsWithNull(interimsStartOfRace, newStartOfRace)
+                        && !Util.equalsWithNull(oldStartOfRace, newStartOfRace)) {
+                    // invalidateStartTime() will not have thrown a startOfRaceChanged event notification because it
+                    // already saw the new
+                    // start of race time; we have to throw the notification here:
+                    notifyListenersStartOfRaceChanged(oldStartOfRace, newStartOfRace);
                 }
             }
+            invalidateMarkPassingTimes();
+            invalidateEndTime();
+            // notify *after* all mark passings have been re-established; should avoid flicker
+            notifyListeners(competitor, oldMarkPassings, markPassings);
         } finally {
-            LockUtil.unlockAfterWrite(markPassingsLock);
+            LockUtil.unlockAfterRead(getSerializationLock());
         }
-        updated(timePointOfLatestEvent);
-        triggerManeuverCacheRecalculation(competitor);
-        // update the race times like start, end and the leg times
-        if (requiresStartTimeUpdate) {
-            TimePoint interimsStartOfRace = getStartOfRace();
-            invalidateStartTime();
-            TimePoint newStartOfRace = getStartOfRace();
-            if (Util.equalsWithNull(interimsStartOfRace, newStartOfRace) && !Util.equalsWithNull(oldStartOfRace, newStartOfRace)) {
-                // invalidateStartTime() will not have thrown a startOfRaceChanged event notification because it already saw the new
-                // start of race time; we have to throw the notification here:
-                notifyListenersStartOfRaceChanged(oldStartOfRace, newStartOfRace);
-            }
-        }
-        invalidateMarkPassingTimes();
-        invalidateEndTime();
-        // notify *after* all mark passings have been re-established; should avoid flicker
-        notifyListeners(competitor, oldMarkPassings, markPassings);
     }
 
     @Override

java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/TrackedLegImpl.java

@@ -1,5 +1,7 @@
 package com.sap.sailing.domain.tracking.impl;
 
+import java.io.IOException;
+import java.io.ObjectInputStream;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
@@ -31,8 +33,8 @@ import com.sap.sailing.domain.tracking.TrackedLegOfCompetitor;
 import com.sap.sailing.domain.tracking.TrackedRace;
 import com.sap.sailing.domain.tracking.TrackedRaceStatus;
 import com.sap.sailing.domain.tracking.Wind;
-import com.sap.sailing.domain.tracking.WindPositionMode;
 import com.sap.sailing.domain.tracking.WindLegTypeAndLegBearingCache;
+import com.sap.sailing.domain.tracking.WindPositionMode;
 import com.sap.sse.common.Util;
 
 public class TrackedLegImpl implements TrackedLeg, RaceChangeListener {
@@ -45,7 +47,7 @@ public class TrackedLegImpl implements TrackedLeg, RaceChangeListener {
     private final Leg leg;
     private final Map<Competitor, TrackedLegOfCompetitor> trackedLegsOfCompetitors;
     private TrackedRaceImpl trackedRace;
-    private final Map<TimePoint, List<TrackedLegOfCompetitor>> competitorTracksOrderedByRank;
+    private transient Map<TimePoint, List<TrackedLegOfCompetitor>> competitorTracksOrderedByRank;
     
     public TrackedLegImpl(DynamicTrackedRaceImpl trackedRace, Leg leg, Iterable<Competitor> competitors) {
         super();
@@ -56,7 +58,12 @@ public class TrackedLegImpl implements TrackedLeg, RaceChangeListener {
             trackedLegsOfCompetitors.put(competitor, new TrackedLegOfCompetitorImpl(this, competitor));
         }
         trackedRace.addListener(this);
-        competitorTracksOrderedByRank = new HashMap<TimePoint, List<TrackedLegOfCompetitor>>();
+        competitorTracksOrderedByRank = new HashMap<>();
+    }
+    
+    private void readObject(ObjectInputStream ois) throws ClassNotFoundException, IOException {
+        ois.defaultReadObject();
+        competitorTracksOrderedByRank = new HashMap<>();
     }
     
     @Override
@@ -114,7 +121,6 @@ public class TrackedLegImpl implements TrackedLeg, RaceChangeListener {
             // race may be updated while calculation is going on, but each individual calculation is properly
             // synchronized, usually by read-write locks, so there is no major difference in synchronization issues
             // an the asynchronous nature of how the data is being received
-            // TODO See bug 469; competitors already disqualified may need to be ranked worst
             Collections.sort(rankedCompetitorList, new WindwardToGoComparator(this, timePoint, cache));
             rankedCompetitorList = Collections.unmodifiableList(rankedCompetitorList);
             synchronized (competitorTracksOrderedByRank) {