GP-0: Various UI test fixes, incl. viewport syncing

GP-0: Fixed Listing tests wrt/ location label

GP-0: Trying to synchronize view creation and memory layout

GP-0: Trying to fix CME

GP-0: Got viewport synchronized, and it resolved test.

GP-0: Fixing regions tests - involved listing selections

GP-0: Fixed modules/sections provider tests

GP-0: Fixed static-mappings provider tests.

Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/DBTrace.java

@@ -525,19 +525,23 @@ public class DBTrace extends DBCachedDomainObjectAdapter implements Trace, Trace
 
 	@Override
 	public DBTraceProgramView getFixedProgramView(long snap) {
-		DBTraceProgramView view = fixedProgramViews.computeIfAbsent(snap, t -> {
-			Msg.debug(this, "Creating fixed view at snap=" + snap);
-			return new DBTraceProgramView(this, snap, baseCompilerSpec);
-		});
-		return view;
+		synchronized (fixedProgramViews) {
+			DBTraceProgramView view = fixedProgramViews.computeIfAbsent(snap, t -> {
+				Msg.debug(this, "Creating fixed view at snap=" + snap);
+				return new DBTraceProgramView(this, snap, baseCompilerSpec);
+			});
+			return view;
+		}
 	}
 
 	@Override
 	public DBTraceVariableSnapProgramView createProgramView(long snap) {
-		DBTraceVariableSnapProgramView view =
-			new DBTraceVariableSnapProgramView(this, snap, baseCompilerSpec);
-		programViews.put(view, null);
-		return view;
+		synchronized (programViews) {
+			DBTraceVariableSnapProgramView view =
+				new DBTraceVariableSnapProgramView(this, snap, baseCompilerSpec);
+			programViews.put(view, null);
+			return view;
+		}
 	}
 
 	@Override
@@ -675,10 +679,14 @@ public class DBTrace extends DBCachedDomainObjectAdapter implements Trace, Trace
 	}
 
 	protected void allViews(Consumer<DBTraceProgramView> action) {
-		for (DBTraceProgramView view : programViews.keySet()) {
-			action.accept(view);
+		Collection<DBTraceProgramView> all = new ArrayList<>();
+		synchronized (programViews) {
+			all.addAll(programViews.keySet());
 		}
-		for (DBTraceProgramView view : fixedProgramViews.values()) {
+		synchronized (fixedProgramViews) {
+			all.addAll(fixedProgramViews.values());
+		}
+		for (DBTraceProgramView view : all) {
 			action.accept(view);
 		}
 	}

Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/util/DefaultTraceTimeViewport.java

@@ -56,30 +56,20 @@ public class DefaultTraceTimeViewport implements TraceTimeViewport {
 		}
 
 		private void snapshotAdded(TraceSnapshot snapshot) {
-			if (snapshot.getSchedule() == null) {
-				return;
-			}
-			if (spanSet.contains(snapshot.getKey())) {
-				recomputeSnapRanges();
-				return;
+			if (checkSnapshotAddedNeedsRefresh(snapshot)) {
+				refreshSnapRanges();
 			}
 		}
 
 		private void snapshotChanged(TraceSnapshot snapshot) {
-			if (isLower(snapshot.getKey())) {
-				recomputeSnapRanges();
-				return;
-			}
-			if (spanSet.contains(snapshot.getKey()) && snapshot.getSchedule() != null) {
-				recomputeSnapRanges();
-				return;
+			if (checkSnapshotChangedNeedsRefresh(snapshot)) {
+				refreshSnapRanges();
 			}
 		}
 
 		private void snapshotDeleted(TraceSnapshot snapshot) {
-			if (isLower(snapshot.getKey())) {
-				recomputeSnapRanges();
-				return;
+			if (checkSnapshotDeletedNeedsRefresh(snapshot)) {
+				refreshSnapRanges();
 			}
 		}
 
@@ -117,27 +107,31 @@ public class DefaultTraceTimeViewport implements TraceTimeViewport {
 
 	@Override
 	public boolean containsAnyUpper(Range<Long> range) {
-		// NB. This should only ever visit the first range intersecting that given
-		for (Range<Long> intersecting : spanSet.subRangeSet(range).asRanges()) {
-			if (range.contains(intersecting.upperEndpoint())) {
-				return true;
+		synchronized (ordered) {
+			// NB. This should only ever visit the first range intersecting that given
+			for (Range<Long> intersecting : spanSet.subRangeSet(range).asRanges()) {
+				if (range.contains(intersecting.upperEndpoint())) {
+					return true;
+				}
 			}
+			return false;
 		}
-		return false;
 	}
 
 	@Override
 	public <T> boolean isCompletelyVisible(AddressRange range, Range<Long> lifespan, T object,
 			Occlusion<T> occlusion) {
-		for (Range<Long> rng : ordered) {
-			if (lifespan.contains(rng.upperEndpoint())) {
-				return true;
-			}
-			if (occlusion.occluded(object, range, rng)) {
-				return false;
+		synchronized (ordered) {
+			for (Range<Long> rng : ordered) {
+				if (lifespan.contains(rng.upperEndpoint())) {
+					return true;
+				}
+				if (occlusion.occluded(object, range, rng)) {
+					return false;
+				}
 			}
+			return false;
 		}
-		return false;
 	}
 
 	@Override
@@ -147,13 +141,15 @@ public class DefaultTraceTimeViewport implements TraceTimeViewport {
 			return new AddressSet();
 		}
 		AddressSet remains = new AddressSet(set);
-		for (Range<Long> rng : ordered) {
-			if (lifespan.contains(rng.upperEndpoint())) {
-				return remains;
-			}
-			occlusion.remove(object, remains, rng);
-			if (remains.isEmpty()) {
-				return remains;
+		synchronized (ordered) {
+			for (Range<Long> rng : ordered) {
+				if (lifespan.contains(rng.upperEndpoint())) {
+					return remains;
+				}
+				occlusion.remove(object, remains, rng);
+				if (remains.isEmpty()) {
+					return remains;
+				}
 			}
 		}
 		// This condition should have been detected by !containsAnyUpper
@@ -161,14 +157,17 @@ public class DefaultTraceTimeViewport implements TraceTimeViewport {
 	}
 
 	protected boolean isLower(long lower) {
-		Range<Long> range = spanSet.rangeContaining(lower);
-		if (range == null) {
-			return false;
+		synchronized (ordered) {
+			Range<Long> range = spanSet.rangeContaining(lower);
+			if (range == null) {
+				return false;
+			}
+			return range.lowerEndpoint().longValue() == lower;
 		}
-		return range.lowerEndpoint().longValue() == lower;
 	}
 
-	protected boolean addSnapRange(long lower, long upper) {
+	protected static boolean addSnapRange(long lower, long upper, RangeSet<Long> spanSet,
+			List<Range<Long>> ordered) {
 		if (spanSet.contains(lower)) {
 			return false;
 		}
@@ -178,7 +177,7 @@ public class DefaultTraceTimeViewport implements TraceTimeViewport {
 		return true;
 	}
 
-	protected TraceSnapshot locateMostRecentFork(long from) {
+	protected static TraceSnapshot locateMostRecentFork(TraceTimeManager timeManager, long from) {
 		while (true) {
 			TraceSnapshot prev = timeManager.getMostRecentSnapshot(from);
 			if (prev == null) {
@@ -203,11 +202,23 @@ public class DefaultTraceTimeViewport implements TraceTimeViewport {
 		}
 	}
 
-	protected void traverseAndAddForkRanges(long curSnap) {
+	/**
+	 * Construct the ranges (set and ordered)
+	 * 
+	 * <p>
+	 * NOTE: I cannot hold the lock during this, because I also require the DB's read lock. There
+	 * are other operations, e.g., addRegion, that will hold the DB's write lock, and then also
+	 * require the viewport's lock to check if it is visible. That would cause the classic tango of
+	 * death.
+	 * 
+	 * @param curSnap the seed snap
+	 */
+	protected static void collectForkRanges(TraceTimeManager timeManager, long curSnap,
+			RangeSet<Long> spanSet, List<Range<Long>> ordered) {
 		while (true) {
-			TraceSnapshot fork = locateMostRecentFork(curSnap);
+			TraceSnapshot fork = locateMostRecentFork(timeManager, curSnap);
 			long prevSnap = fork == null ? Long.MIN_VALUE : fork.getKey();
-			if (!addSnapRange(prevSnap, curSnap)) {
+			if (!addSnapRange(prevSnap, curSnap, spanSet, ordered)) {
 				return;
 			}
 			if (fork == null) {
@@ -217,71 +228,124 @@ public class DefaultTraceTimeViewport implements TraceTimeViewport {
 		}
 	}
 
-	protected void recomputeSnapRanges() {
-		spanSet.clear();
-		ordered.clear();
-		traverseAndAddForkRanges(snap);
+	protected void refreshSnapRanges() {
+		RangeSet<Long> spanSet = TreeRangeSet.create();
+		List<Range<Long>> ordered = new ArrayList<>();
+		collectForkRanges(timeManager, snap, spanSet, ordered);
+		synchronized (this.ordered) {
+			this.spanSet.clear();
+			this.ordered.clear();
+			this.spanSet.addAll(spanSet);
+			this.ordered.addAll(ordered);
+		}
 		assert !ordered.isEmpty();
 		changeListeners.fire.run();
 	}
 
 	public void setSnap(long snap) {
 		this.snap = snap;
-		recomputeSnapRanges();
+		refreshSnapRanges();
+	}
+
+	protected boolean checkSnapshotAddedNeedsRefresh(TraceSnapshot snapshot) {
+		synchronized (ordered) {
+			if (snapshot.getSchedule() == null) {
+				return false;
+			}
+			if (spanSet.contains(snapshot.getKey())) {
+				return true;
+			}
+			return false;
+		}
+	}
+
+	protected boolean checkSnapshotChangedNeedsRefresh(TraceSnapshot snapshot) {
+		synchronized (ordered) {
+			if (isLower(snapshot.getKey())) {
+				return true;
+			}
+			if (spanSet.contains(snapshot.getKey()) && snapshot.getSchedule() != null) {
+				return true;
+			}
+			return false;
+		}
+	}
+
+	protected boolean checkSnapshotDeletedNeedsRefresh(TraceSnapshot snapshot) {
+		synchronized (ordered) {
+			if (isLower(snapshot.getKey())) {
+				return true;
+			}
+			return false;
+		}
 	}
 
 	@Override
 	public boolean isForked() {
-		return ordered.size() > 1;
+		synchronized (ordered) {
+			return ordered.size() > 1;
+		}
 	}
 
 	@Override
 	public List<Long> getOrderedSnaps() {
-		return ordered
-				.stream()
-				.map(Range::upperEndpoint)
-				.collect(Collectors.toList());
+		synchronized (ordered) {
+			return ordered
+					.stream()
+					.map(Range::upperEndpoint)
+					.collect(Collectors.toList());
+		}
 	}
 
 	@Override
 	public List<Long> getReversedSnaps() {
-		return Lists.reverse(ordered)
-				.stream()
-				.map(Range::upperEndpoint)
-				.collect(Collectors.toList());
+		synchronized (ordered) {
+			return Lists.reverse(ordered)
+					.stream()
+					.map(Range::upperEndpoint)
+					.collect(Collectors.toList());
+		}
 	}
 
 	@Override
 	public <T> T getTop(Function<Long, T> func) {
-		for (Range<Long> rng : ordered) {
-			T t = func.apply(rng.upperEndpoint());
-			if (t != null) {
-				return t;
+		synchronized (ordered) {
+			for (Range<Long> rng : ordered) {
+				T t = func.apply(rng.upperEndpoint());
+				if (t != null) {
+					return t;
+				}
 			}
+			return null;
 		}
-		return null;
 	}
 
 	@Override
 	public <T> Iterator<T> mergedIterator(Function<Long, Iterator<T>> iterFunc,
 			Comparator<? super T> comparator) {
-		if (!isForked()) {
-			return iterFunc.apply(snap);
+		List<Iterator<T>> iters;
+		synchronized (ordered) {
+			if (!isForked()) {
+				return iterFunc.apply(snap);
+			}
+			iters = ordered.stream()
+					.map(rng -> iterFunc.apply(rng.upperEndpoint()))
+					.collect(Collectors.toList());
 		}
-		List<Iterator<T>> iters = ordered.stream()
-				.map(rng -> iterFunc.apply(rng.upperEndpoint()))
-				.collect(Collectors.toList());
 		return new UniqIterator<>(new MergeSortingIterator<>(iters, comparator));
 	}
 
 	@Override
 	public AddressSetView unionedAddresses(Function<Long, AddressSetView> viewFunc) {
-		if (!isForked()) {
-			return viewFunc.apply(snap);
+		List<AddressSetView> views;
+		synchronized (ordered) {
+			if (!isForked()) {
+				return viewFunc.apply(snap);
+			}
+			views = ordered.stream()
+					.map(rng -> viewFunc.apply(rng.upperEndpoint()))
+					.collect(Collectors.toList());
 		}
-		List<AddressSetView> views = ordered.stream()
-				.map(rng -> viewFunc.apply(rng.upperEndpoint()))
-				.collect(Collectors.toList());
 		return new UnionAddressSetView(views);
 	}
 }