fixing bug 3728 (deadlock) by copying mark passing for waypoint under the lock, then releasing, then calling into deep stacks

Change-Id: Idc28d6c79781e9a174fce8ee8646f64b08b0a11c

java/com.sap.sailing.domain/src/com/sap/sailing/domain/ranking/AbstractRankingMetric.java

@@ -1,8 +1,10 @@
 package com.sap.sailing.domain.ranking;
 
+import java.util.ArrayList;
 import java.util.Comparator;
 import java.util.HashMap;
 import java.util.Iterator;
+import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
@@ -29,6 +31,7 @@ import com.sap.sailing.domain.tracking.WindPositionMode;
 import com.sap.sailing.domain.tracking.impl.AbstractRaceRankComparator;
 import com.sap.sse.common.Duration;
 import com.sap.sse.common.TimePoint;
+import com.sap.sse.common.Util;
 
 public abstract class AbstractRankingMetric implements RankingMetric {
     private static final long serialVersionUID = -3671039530564696392L;
@@ -664,25 +667,33 @@ public abstract class AbstractRankingMetric implements RankingMetric {
         if (firstAroundMark != null) {
             result = firstAroundMark;
         } else {
-            Iterable<MarkPassing> markPassingsForLegStart = getTrackedRace().getMarkPassingsInOrder(trackedLeg.getLeg().getFrom());
+            List<MarkPassing> copyOfMarkPassingsForLegStart = new ArrayList<>();
-            Distance maxWindwardDistanceTraveled = new MeterDistance(Double.MIN_VALUE);
+            {   // scope the markPassingsForLegStart, so it is no longer used outside this block
-            Competitor competitorFarthestAlong = null;
+                final Iterable<MarkPassing> markPassingsForLegStart = getTrackedRace().getMarkPassingsInOrder(trackedLeg.getLeg().getFrom());
+                // See bug 3728: obtaining lock for mark passings in order for a waypoint before code potentially called from here,
+                // e.g., through getWindwardDistanceTraveled(...), tries to obtain the read lock for mark passings for a competitor can
+                // result in a deadlock. Therefore, we copy the mark passings for the leg start under the lock, then release it again
+                // before calling into a deep stack with getWindwardDistanceTraveled(...) which may well obtain a read lock on
+                // the mark passings for a competitor.
                 getTrackedRace().lockForRead(markPassingsForLegStart);
                 try {
-                for (MarkPassing mp : markPassingsForLegStart) {
+                    Util.addAll(markPassingsForLegStart, copyOfMarkPassingsForLegStart);
+                } finally {
+                    getTrackedRace().unlockAfterRead(markPassingsForLegStart);
+                }
+            }
+            Distance maxWindwardDistanceTraveled = new MeterDistance(Double.MIN_VALUE);
+            Competitor competitorFarthestAlong = null;
+            for (MarkPassing mp : copyOfMarkPassingsForLegStart) {
                 if (mp.getTimePoint().after(timePoint)) {
                     break;
                 }
-                    final Distance windwardDistanceTraveled = getWindwardDistanceTraveled(mp.getCompetitor(), mp.getWaypoint(),
+                final Distance windwardDistanceTraveled = getWindwardDistanceTraveled(mp.getCompetitor(), mp.getWaypoint(), timePoint, cache);
-                            timePoint, cache);
                 if (windwardDistanceTraveled.compareTo(maxWindwardDistanceTraveled) > 0) {
                     maxWindwardDistanceTraveled = windwardDistanceTraveled;
                     competitorFarthestAlong = mp.getCompetitor();
                 }
             }
-            } finally {
-                getTrackedRace().unlockAfterRead(markPassingsForLegStart);
-            }
             result = competitorFarthestAlong;
         }
         return result;

java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/TrackedRace.java

@@ -414,6 +414,10 @@ public interface TrackedRace extends Serializable, IsManagedByCache<SharedDomain
      * first obtains the mark passings read lock and later, while holding on to that lock, asks for the course's read
      * lock, a deadlock may result.<p>
      * 
+     * Furthermore, when trying to acquire both, a lock for the {@link #getMarkPassings(Competitor) mark passings for a competitor}
+     * and a lock for the {@link #getMarkPassingsInOrder(Waypoint) mark passings for a waypoint, this needs to happen in exactly
+     * this order, or a deadlock may result.<p>
+     * 
      * The {@link #unlockAfterRead(Iterable)} method will symmetrically unlock the course's read lock after releasing the
      * read lock for the mark passings.
      */