From 4a0ff54d52bcc2d101d7abc10c92ffe971000bc8 Mon Sep 17 00:00:00 2001 From: Axel Uhl Date: Sun, 20 Jul 2014 16:02:46 +0200 Subject: [PATCH] bug 2077: protecting mark passing structures during TrackedRace serialization; don't serialize rank cache in TrackedLegImpl, avoiding CME --- .../tracking/impl/DynamicTrackedRaceImpl.java | 170 +++++++++--------- .../domain/tracking/impl/TrackedLegImpl.java | 14 +- 2 files changed, 100 insertions(+), 84 deletions(-) diff --git a/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/DynamicTrackedRaceImpl.java b/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/DynamicTrackedRaceImpl.java index c8511c9cc7a..caeec4852b6 100755 --- a/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/DynamicTrackedRaceImpl.java +++ b/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/DynamicTrackedRaceImpl.java @@ -581,96 +581,106 @@ DynamicTrackedRace, GPSTrackListener { @Override public void updateMarkPassings(Competitor competitor, Iterable markPassings) { - Map oldMarkPassings = new HashMap(); - MarkPassing oldStartMarkPassing = null; - TimePoint oldStartOfRace = getStartOfRace(); // getStartOfRace() may respond with a new result already after updating the mark passings - boolean requiresStartTimeUpdate = true; - final NavigableSet 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 oldMarkPassings = new HashMap(); + MarkPassing oldStartMarkPassing = null; + TimePoint oldStartOfRace = getStartOfRace(); // getStartOfRace() may respond with a new result already after + // updating the mark passings + boolean requiresStartTimeUpdate = true; + final NavigableSet 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 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 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 diff --git a/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/TrackedLegImpl.java b/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/TrackedLegImpl.java index 13307cf8299..f725b950405 100755 --- a/java/com.sap.sailing.domain/src/com/sap/sailing/domain/tracking/impl/TrackedLegImpl.java +++ b/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 trackedLegsOfCompetitors; private TrackedRaceImpl trackedRace; - private final Map> competitorTracksOrderedByRank; + private transient Map> competitorTracksOrderedByRank; public TrackedLegImpl(DynamicTrackedRaceImpl trackedRace, Leg leg, Iterable 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>(); + 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) {