GP-0: Fixed deadlock involing viewport

This commit is contained in:
Dan 2021-05-06 19:32:39 +00:00
parent 6bac39e79c
commit ad33b75987
2 changed files with 85 additions and 62 deletions

View file

@ -154,8 +154,8 @@ public class DBTraceMemoryRegion
try (LockHold hold = LockHold.lock(space.lock.writeLock())) { try (LockHold hold = LockHold.lock(space.lock.writeLock())) {
this.name = name; this.name = name;
update(NAME_COLUMN); update(NAME_COLUMN);
space.trace.updateViewsChangeBlockName(this);
} }
space.trace.updateViewsChangeBlockName(this);
space.trace.setChanged( space.trace.setChanged(
new TraceChangeRecord<>(TraceMemoryRegionChangeType.CHANGED, space, this)); new TraceChangeRecord<>(TraceMemoryRegionChangeType.CHANGED, space, this));
} }
@ -218,8 +218,8 @@ public class DBTraceMemoryRegion
oldRange = range; oldRange = range;
checkOverlapConflicts(lifespan, newRange); checkOverlapConflicts(lifespan, newRange);
doSetRange(newRange); doSetRange(newRange);
space.trace.updateViewsChangeBlockRange(this, oldRange, newRange);
} }
space.trace.updateViewsChangeBlockRange(this, oldRange, newRange);
space.trace.setChanged( space.trace.setChanged(
new TraceChangeRecord<>(TraceMemoryRegionChangeType.CHANGED, space, this)); new TraceChangeRecord<>(TraceMemoryRegionChangeType.CHANGED, space, this));
} }
@ -278,8 +278,8 @@ public class DBTraceMemoryRegion
this.flags.add(f); this.flags.add(f);
} }
update(FLAGS_COLUMN); update(FLAGS_COLUMN);
space.trace.updateViewsChangeBlockFlags(this);
} }
space.trace.updateViewsChangeBlockFlags(this);
space.trace.setChanged( space.trace.setChanged(
new TraceChangeRecord<>(TraceMemoryRegionChangeType.CHANGED, space, this)); new TraceChangeRecord<>(TraceMemoryRegionChangeType.CHANGED, space, this));
} }
@ -293,8 +293,8 @@ public class DBTraceMemoryRegion
this.flags.add(f); this.flags.add(f);
} }
update(FLAGS_COLUMN); update(FLAGS_COLUMN);
space.trace.updateViewsChangeBlockFlags(this);
} }
space.trace.updateViewsChangeBlockFlags(this);
space.trace.setChanged( space.trace.setChanged(
new TraceChangeRecord<>(TraceMemoryRegionChangeType.CHANGED, space, this)); new TraceChangeRecord<>(TraceMemoryRegionChangeType.CHANGED, space, this));
} }
@ -308,8 +308,8 @@ public class DBTraceMemoryRegion
this.flags.remove(f); this.flags.remove(f);
} }
update(FLAGS_COLUMN); update(FLAGS_COLUMN);
space.trace.updateViewsChangeBlockFlags(this);
} }
space.trace.updateViewsChangeBlockFlags(this);
space.trace.setChanged( space.trace.setChanged(
new TraceChangeRecord<>(TraceMemoryRegionChangeType.CHANGED, space, this)); new TraceChangeRecord<>(TraceMemoryRegionChangeType.CHANGED, space, this));
} }

View file

@ -81,7 +81,12 @@ public class DefaultTraceTimeViewport implements TraceTimeViewport {
protected final Trace trace; protected final Trace trace;
protected final TraceTimeManager timeManager; protected final TraceTimeManager timeManager;
protected final List<Range<Long>> 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 <em>first</em> acquire the
* DB lock.
*/
protected final List<Range<Long>> ordered = new ArrayList<>();
protected final RangeSet<Long> spanSet = TreeRangeSet.create(); protected final RangeSet<Long> spanSet = TreeRangeSet.create();
protected final ForSnapshotsListener listener = new ForSnapshotsListener(); protected final ForSnapshotsListener listener = new ForSnapshotsListener();
protected final ListenerSet<Runnable> changeListeners = new ListenerSet<>(Runnable.class); protected final ListenerSet<Runnable> changeListeners = new ListenerSet<>(Runnable.class);
@ -107,48 +112,54 @@ public class DefaultTraceTimeViewport implements TraceTimeViewport {
@Override @Override
public boolean containsAnyUpper(Range<Long> range) { public boolean containsAnyUpper(Range<Long> range) {
synchronized (ordered) { try (LockHold hold = trace.lockRead()) {
// NB. This should only ever visit the first range intersecting that given synchronized (ordered) {
for (Range<Long> intersecting : spanSet.subRangeSet(range).asRanges()) { // NB. This should only ever visit the first range intersecting that given
if (range.contains(intersecting.upperEndpoint())) { for (Range<Long> intersecting : spanSet.subRangeSet(range).asRanges()) {
return true; if (range.contains(intersecting.upperEndpoint())) {
return true;
}
} }
return false;
} }
return false;
} }
} }
@Override @Override
public <T> boolean isCompletelyVisible(AddressRange range, Range<Long> lifespan, T object, public <T> boolean isCompletelyVisible(AddressRange range, Range<Long> lifespan, T object,
Occlusion<T> occlusion) { Occlusion<T> occlusion) {
synchronized (ordered) { try (LockHold hold = trace.lockRead()) {
for (Range<Long> rng : ordered) { synchronized (ordered) {
if (lifespan.contains(rng.upperEndpoint())) { for (Range<Long> rng : ordered) {
return true; if (lifespan.contains(rng.upperEndpoint())) {
} return true;
if (occlusion.occluded(object, range, rng)) { }
return false; if (occlusion.occluded(object, range, rng)) {
return false;
}
} }
return false;
} }
return false;
} }
} }
@Override @Override
public <T> AddressSet computeVisibleParts(AddressSetView set, Range<Long> lifespan, T object, public <T> AddressSet computeVisibleParts(AddressSetView set, Range<Long> lifespan, T object,
Occlusion<T> occlusion) { Occlusion<T> occlusion) {
if (!containsAnyUpper(lifespan)) { try (LockHold hold = trace.lockRead()) {
return new AddressSet(); if (!containsAnyUpper(lifespan)) {
} return new AddressSet();
AddressSet remains = new AddressSet(set); }
synchronized (ordered) { AddressSet remains = new AddressSet(set);
for (Range<Long> rng : ordered) { synchronized (ordered) {
if (lifespan.contains(rng.upperEndpoint())) { for (Range<Long> rng : ordered) {
return remains; if (lifespan.contains(rng.upperEndpoint())) {
} return remains;
occlusion.remove(object, remains, rng); }
if (remains.isEmpty()) { occlusion.remove(object, remains, rng);
return remains; if (remains.isEmpty()) {
return remains;
}
} }
} }
} }
@ -231,7 +242,9 @@ public class DefaultTraceTimeViewport implements TraceTimeViewport {
protected void refreshSnapRanges() { protected void refreshSnapRanges() {
RangeSet<Long> spanSet = TreeRangeSet.create(); RangeSet<Long> spanSet = TreeRangeSet.create();
List<Range<Long>> ordered = new ArrayList<>(); List<Range<Long>> ordered = new ArrayList<>();
collectForkRanges(timeManager, snap, spanSet, ordered); try (LockHold hold = trace.lockRead()) {
collectForkRanges(timeManager, snap, spanSet, ordered);
}
synchronized (this.ordered) { synchronized (this.ordered) {
this.spanSet.clear(); this.spanSet.clear();
this.ordered.clear(); this.ordered.clear();
@ -248,35 +261,41 @@ public class DefaultTraceTimeViewport implements TraceTimeViewport {
} }
protected boolean checkSnapshotAddedNeedsRefresh(TraceSnapshot snapshot) { protected boolean checkSnapshotAddedNeedsRefresh(TraceSnapshot snapshot) {
synchronized (ordered) { try (LockHold hold = trace.lockRead()) {
if (snapshot.getSchedule() == null) { synchronized (ordered) {
if (snapshot.getSchedule() == null) {
return false;
}
if (spanSet.contains(snapshot.getKey())) {
return true;
}
return false; return false;
} }
if (spanSet.contains(snapshot.getKey())) {
return true;
}
return false;
} }
} }
protected boolean checkSnapshotChangedNeedsRefresh(TraceSnapshot snapshot) { protected boolean checkSnapshotChangedNeedsRefresh(TraceSnapshot snapshot) {
synchronized (ordered) { try (LockHold hold = trace.lockRead()) {
if (isLower(snapshot.getKey())) { synchronized (ordered) {
return true; 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) { protected boolean checkSnapshotDeletedNeedsRefresh(TraceSnapshot snapshot) {
synchronized (ordered) { try (LockHold hold = trace.lockRead()) {
if (isLower(snapshot.getKey())) { synchronized (ordered) {
return true; if (isLower(snapshot.getKey())) {
return true;
}
return false;
} }
return false;
} }
} }
@ -324,13 +343,15 @@ public class DefaultTraceTimeViewport implements TraceTimeViewport {
public <T> Iterator<T> mergedIterator(Function<Long, Iterator<T>> iterFunc, public <T> Iterator<T> mergedIterator(Function<Long, Iterator<T>> iterFunc,
Comparator<? super T> comparator) { Comparator<? super T> comparator) {
List<Iterator<T>> iters; List<Iterator<T>> iters;
synchronized (ordered) { try (LockHold hold = trace.lockRead()) {
if (!isForked()) { synchronized (ordered) {
return iterFunc.apply(snap); 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)); return new UniqIterator<>(new MergeSortingIterator<>(iters, comparator));
} }
@ -338,13 +359,15 @@ public class DefaultTraceTimeViewport implements TraceTimeViewport {
@Override @Override
public AddressSetView unionedAddresses(Function<Long, AddressSetView> viewFunc) { public AddressSetView unionedAddresses(Function<Long, AddressSetView> viewFunc) {
List<AddressSetView> views; List<AddressSetView> views;
synchronized (ordered) { try (LockHold hold = trace.lockRead()) {
if (!isForked()) { synchronized (ordered) {
return viewFunc.apply(snap); 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); return new UnionAddressSetView(views);
} }