diff --git a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/memory/DBTraceMemoryRegion.java b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/memory/DBTraceMemoryRegion.java index 9662e2cba4..77825e4400 100644 --- a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/memory/DBTraceMemoryRegion.java +++ b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/memory/DBTraceMemoryRegion.java @@ -154,8 +154,8 @@ public class DBTraceMemoryRegion try (LockHold hold = LockHold.lock(space.lock.writeLock())) { this.name = name; update(NAME_COLUMN); + space.trace.updateViewsChangeBlockName(this); } - space.trace.updateViewsChangeBlockName(this); space.trace.setChanged( new TraceChangeRecord<>(TraceMemoryRegionChangeType.CHANGED, space, this)); } @@ -218,8 +218,8 @@ public class DBTraceMemoryRegion oldRange = range; checkOverlapConflicts(lifespan, newRange); doSetRange(newRange); + space.trace.updateViewsChangeBlockRange(this, oldRange, newRange); } - space.trace.updateViewsChangeBlockRange(this, oldRange, newRange); space.trace.setChanged( new TraceChangeRecord<>(TraceMemoryRegionChangeType.CHANGED, space, this)); } @@ -278,8 +278,8 @@ public class DBTraceMemoryRegion this.flags.add(f); } update(FLAGS_COLUMN); + space.trace.updateViewsChangeBlockFlags(this); } - space.trace.updateViewsChangeBlockFlags(this); space.trace.setChanged( new TraceChangeRecord<>(TraceMemoryRegionChangeType.CHANGED, space, this)); } @@ -293,8 +293,8 @@ public class DBTraceMemoryRegion this.flags.add(f); } update(FLAGS_COLUMN); + space.trace.updateViewsChangeBlockFlags(this); } - space.trace.updateViewsChangeBlockFlags(this); space.trace.setChanged( new TraceChangeRecord<>(TraceMemoryRegionChangeType.CHANGED, space, this)); } @@ -308,8 +308,8 @@ public class DBTraceMemoryRegion this.flags.remove(f); } update(FLAGS_COLUMN); + space.trace.updateViewsChangeBlockFlags(this); } - space.trace.updateViewsChangeBlockFlags(this); space.trace.setChanged( new TraceChangeRecord<>(TraceMemoryRegionChangeType.CHANGED, space, this)); } diff --git a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/util/DefaultTraceTimeViewport.java b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/util/DefaultTraceTimeViewport.java index 053d51f9f6..d91720a553 100644 --- a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/util/DefaultTraceTimeViewport.java +++ b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/util/DefaultTraceTimeViewport.java @@ -81,7 +81,12 @@ public class DefaultTraceTimeViewport implements TraceTimeViewport { protected final Trace trace; protected final TraceTimeManager timeManager; - protected final List> ordered = Collections.synchronizedList(new ArrayList<>()); + /** + * NB: This is the syncing object for the viewport. If there's even a chance an operation may + * need the DB's lock, esp., considering user callbacks, then it must first acquire the + * DB lock. + */ + protected final List> ordered = new ArrayList<>(); protected final RangeSet spanSet = TreeRangeSet.create(); protected final ForSnapshotsListener listener = new ForSnapshotsListener(); protected final ListenerSet changeListeners = new ListenerSet<>(Runnable.class); @@ -107,48 +112,54 @@ public class DefaultTraceTimeViewport implements TraceTimeViewport { @Override public boolean containsAnyUpper(Range range) { - synchronized (ordered) { - // NB. This should only ever visit the first range intersecting that given - for (Range intersecting : spanSet.subRangeSet(range).asRanges()) { - if (range.contains(intersecting.upperEndpoint())) { - return true; + try (LockHold hold = trace.lockRead()) { + synchronized (ordered) { + // NB. This should only ever visit the first range intersecting that given + for (Range intersecting : spanSet.subRangeSet(range).asRanges()) { + if (range.contains(intersecting.upperEndpoint())) { + return true; + } } + return false; } - return false; } } @Override public boolean isCompletelyVisible(AddressRange range, Range lifespan, T object, Occlusion occlusion) { - synchronized (ordered) { - for (Range rng : ordered) { - if (lifespan.contains(rng.upperEndpoint())) { - return true; - } - if (occlusion.occluded(object, range, rng)) { - return false; + try (LockHold hold = trace.lockRead()) { + synchronized (ordered) { + for (Range rng : ordered) { + if (lifespan.contains(rng.upperEndpoint())) { + return true; + } + if (occlusion.occluded(object, range, rng)) { + return false; + } } + return false; } - return false; } } @Override public AddressSet computeVisibleParts(AddressSetView set, Range lifespan, T object, Occlusion occlusion) { - if (!containsAnyUpper(lifespan)) { - return new AddressSet(); - } - AddressSet remains = new AddressSet(set); - synchronized (ordered) { - for (Range rng : ordered) { - if (lifespan.contains(rng.upperEndpoint())) { - return remains; - } - occlusion.remove(object, remains, rng); - if (remains.isEmpty()) { - return remains; + try (LockHold hold = trace.lockRead()) { + if (!containsAnyUpper(lifespan)) { + return new AddressSet(); + } + AddressSet remains = new AddressSet(set); + synchronized (ordered) { + for (Range rng : ordered) { + if (lifespan.contains(rng.upperEndpoint())) { + return remains; + } + occlusion.remove(object, remains, rng); + if (remains.isEmpty()) { + return remains; + } } } } @@ -231,7 +242,9 @@ public class DefaultTraceTimeViewport implements TraceTimeViewport { protected void refreshSnapRanges() { RangeSet spanSet = TreeRangeSet.create(); List> ordered = new ArrayList<>(); - collectForkRanges(timeManager, snap, spanSet, ordered); + try (LockHold hold = trace.lockRead()) { + collectForkRanges(timeManager, snap, spanSet, ordered); + } synchronized (this.ordered) { this.spanSet.clear(); this.ordered.clear(); @@ -248,35 +261,41 @@ public class DefaultTraceTimeViewport implements TraceTimeViewport { } protected boolean checkSnapshotAddedNeedsRefresh(TraceSnapshot snapshot) { - synchronized (ordered) { - if (snapshot.getSchedule() == null) { + try (LockHold hold = trace.lockRead()) { + synchronized (ordered) { + if (snapshot.getSchedule() == null) { + return false; + } + if (spanSet.contains(snapshot.getKey())) { + return true; + } return false; } - if (spanSet.contains(snapshot.getKey())) { - return true; - } - return false; } } protected boolean checkSnapshotChangedNeedsRefresh(TraceSnapshot snapshot) { - synchronized (ordered) { - if (isLower(snapshot.getKey())) { - return true; + try (LockHold hold = trace.lockRead()) { + synchronized (ordered) { + if (isLower(snapshot.getKey())) { + return true; + } + if (spanSet.contains(snapshot.getKey()) && snapshot.getSchedule() != null) { + return true; + } + return false; } - 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; + try (LockHold hold = trace.lockRead()) { + synchronized (ordered) { + if (isLower(snapshot.getKey())) { + return true; + } + return false; } - return false; } } @@ -324,13 +343,15 @@ public class DefaultTraceTimeViewport implements TraceTimeViewport { public Iterator mergedIterator(Function> iterFunc, Comparator comparator) { List> iters; - synchronized (ordered) { - if (!isForked()) { - return iterFunc.apply(snap); + try (LockHold hold = trace.lockRead()) { + synchronized (ordered) { + if (!isForked()) { + return iterFunc.apply(snap); + } + iters = ordered.stream() + .map(rng -> iterFunc.apply(rng.upperEndpoint())) + .collect(Collectors.toList()); } - iters = ordered.stream() - .map(rng -> iterFunc.apply(rng.upperEndpoint())) - .collect(Collectors.toList()); } return new UniqIterator<>(new MergeSortingIterator<>(iters, comparator)); } @@ -338,13 +359,15 @@ public class DefaultTraceTimeViewport implements TraceTimeViewport { @Override public AddressSetView unionedAddresses(Function viewFunc) { List views; - synchronized (ordered) { - if (!isForked()) { - return viewFunc.apply(snap); + try (LockHold hold = trace.lockRead()) { + synchronized (ordered) { + if (!isForked()) { + return viewFunc.apply(snap); + } + views = ordered.stream() + .map(rng -> viewFunc.apply(rng.upperEndpoint())) + .collect(Collectors.toList()); } - views = ordered.stream() - .map(rng -> viewFunc.apply(rng.upperEndpoint())) - .collect(Collectors.toList()); } return new UnionAddressSetView(views); }