GP-1861 - Changed locking to prevent potential out-of-order events

This commit is contained in:
dragonmacher 2022-03-29 15:30:47 -04:00
parent 2d526352ee
commit 1ef3f71dd1
16 changed files with 192 additions and 197 deletions

View file

@ -28,15 +28,19 @@ public class DomainObjectEventQueues {
protected final DomainObject source; protected final DomainObject source;
protected final Lock lock; protected final Lock lock;
protected final DomainObjectChangeSupport eventQueue; protected final DomainObjectChangeSupport eventQueue;
protected final Map<EventQueueID, DomainObjectChangeSupport> privateEventQueues = CacheBuilder protected final Map<EventQueueID, DomainObjectChangeSupport> privateEventQueues =
.newBuilder().removalListener(this::privateQueueRemoved).weakKeys().build().asMap(); CacheBuilder.newBuilder()
.removalListener(this::privateQueueRemoved)
.weakKeys()
.build()
.asMap();
protected volatile boolean eventsEnabled = true; protected volatile boolean eventsEnabled = true;
public DomainObjectEventQueues(DomainObject source, int timeInterval, int bufsize, Lock lock) { public DomainObjectEventQueues(DomainObject source, int timeInterval, Lock lock) {
this.source = source; this.source = source;
this.lock = lock; this.lock = lock;
eventQueue = new DomainObjectChangeSupport(source, timeInterval, bufsize, lock); eventQueue = new DomainObjectChangeSupport(source, timeInterval, lock);
} }
private void privateQueueRemoved( private void privateQueueRemoved(
@ -51,18 +55,18 @@ public class DomainObjectEventQueues {
} }
} }
public synchronized void addListener(DomainObjectListener l) { public void addListener(DomainObjectListener l) {
eventQueue.addListener(l); eventQueue.addListener(l);
} }
public synchronized void removeListener(DomainObjectListener l) { public void removeListener(DomainObjectListener l) {
eventQueue.removeListener(l); eventQueue.removeListener(l);
} }
public EventQueueID createPrivateEventQueue(DomainObjectListener listener, int maxDelay) { public EventQueueID createPrivateEventQueue(DomainObjectListener listener, int maxDelay) {
EventQueueID id = new EventQueueID(); EventQueueID id = new EventQueueID();
DomainObjectChangeSupport privateQueue = DomainObjectChangeSupport privateQueue =
new DomainObjectChangeSupport(source, maxDelay, 1000, lock); new DomainObjectChangeSupport(source, maxDelay, lock);
privateQueue.addListener(listener); privateQueue.addListener(listener);
privateEventQueues.put(id, privateQueue); privateEventQueues.put(id, privateQueue);
return id; return id;

View file

@ -29,12 +29,12 @@ public class DBTraceUserData extends DomainObjectAdapterDB implements TraceUserD
} }
protected DBTraceUserData(DBTrace trace) throws IOException { protected DBTraceUserData(DBTrace trace) throws IOException {
super(new DBHandle(), getName(trace), 500, 1000, trace); super(new DBHandle(), getName(trace), 500, trace);
// TODO: Create database and such // TODO: Create database and such
} }
public DBTraceUserData(DBHandle dbh, DBTrace trace, TaskMonitor monitor) { public DBTraceUserData(DBHandle dbh, DBTrace trace, TaskMonitor monitor) {
super(dbh, getName(trace), 500, 1000, trace); super(dbh, getName(trace), 500, trace);
// TODO Auto-generated constructor stub // TODO Auto-generated constructor stub
} }

View file

@ -913,7 +913,7 @@ public class DBTraceProgramView implements TraceProgramView {
this.viewport.setSnap(snap); this.viewport.setSnap(snap);
this.eventQueues = this.eventQueues =
new DomainObjectEventQueues(this, TIME_INTERVAL, BUF_SIZE, trace.getLock()); new DomainObjectEventQueues(this, TIME_INTERVAL, trace.getLock());
this.regViewsByThread = new WeakValueHashMap<>(); this.regViewsByThread = new WeakValueHashMap<>();

View file

@ -61,7 +61,7 @@ public class DBTraceProgramViewRegisters implements TraceProgramView {
this.thread = codeSpace.getThread(); // TODO: Bleh, should be parameter this.thread = codeSpace.getThread(); // TODO: Bleh, should be parameter
this.eventQueues = new DomainObjectEventQueues(this, DBTraceProgramView.TIME_INTERVAL, this.eventQueues = new DomainObjectEventQueues(this, DBTraceProgramView.TIME_INTERVAL,
DBTraceProgramView.BUF_SIZE, view.trace.getLock()); view.trace.getLock());
// TODO: Make these create code/memory spaces lazily, to allow null at construction // TODO: Make these create code/memory spaces lazily, to allow null at construction
// NOTE: Use reference manager as example // NOTE: Use reference manager as example

View file

@ -38,7 +38,7 @@ public abstract class DBDomainObjectSupport extends DomainObjectAdapterDB {
protected DBDomainObjectSupport(DBHandle dbh, DBOpenMode openMode, TaskMonitor monitor, protected DBDomainObjectSupport(DBHandle dbh, DBOpenMode openMode, TaskMonitor monitor,
String name, int timeInterval, int bufSize, Object consumer) { String name, int timeInterval, int bufSize, Object consumer) {
super(dbh, name, timeInterval, bufSize, consumer); super(dbh, name, timeInterval, consumer);
this.openMode = openMode; this.openMode = openMode;
this.monitor = monitor; this.monitor = monitor;
} }

View file

@ -211,7 +211,7 @@ public class VTSessionDB extends DomainObjectAdapterDB implements VTSession, VTC
} }
private VTSessionDB(DBHandle dbHandle, Object consumer) { private VTSessionDB(DBHandle dbHandle, Object consumer) {
super(dbHandle, UNUSED_DEFAULT_NAME, EVENT_NOTIFICATION_DELAY, EVENT_BUFFER_SIZE, consumer); super(dbHandle, UNUSED_DEFAULT_NAME, EVENT_NOTIFICATION_DELAY, consumer);
propertyTable = dbHandle.getTable(PROPERTY_TABLE_NAME); propertyTable = dbHandle.getTable(PROPERTY_TABLE_NAME);
} }

View file

@ -20,24 +20,24 @@ import java.util.*;
import org.apache.commons.collections4.IteratorUtils; import org.apache.commons.collections4.IteratorUtils;
/** /**
* A set that avoids {@link ConcurrentModificationException}s by copying the internal storage * A set that avoids {@link ConcurrentModificationException}s by copying the internal storage
* <b>for every mutation operation</b>. Thus, this data structure is only efficient when the * <b>for every mutation operation</b>. Thus, this data structure is only efficient when the
* number of event notification operations significantly out numbers mutations to this structure * number of event notification operations significantly out numbers mutations to this structure
* (e.g., adding and removing items. * (e.g., adding and removing items.
* <p> * <p>
* An example use cases where using this class is a good fit would be a listener list where * An example use cases where using this class is a good fit would be a listener list where
* listeners are added during initialization, but not after that. Further, this hypothetical * listeners are added during initialization, but not after that. Further, this hypothetical
* list fires a large number of events. * list fires a large number of events.
* <p> * <p>
* A bad use of this class would be as a container to store widgets where the container the * A bad use of this class would be as a container to store widgets where the container the
* contents are changed often, but iterated over very little. * contents are changed often, but iterated over very little.
* <p> * <p>
* Finally, if this structure is only ever used from a single thread, like the Swing thread, then * Finally, if this structure is only ever used from a single thread, like the Swing thread, then
* you do not need the overhead of this class, as the Swing thread synchronous access guarantees * you do not need the overhead of this class, as the Swing thread synchronous access guarantees
* that the structure cannot be mutated while it is being iterated. See * that the structure cannot be mutated while it is being iterated. See
* {@link WeakDataStructureFactory#createSingleThreadAccessWeakSet()}. * {@link WeakDataStructureFactory#createSingleThreadAccessWeakSet()}.
* *
* @param <T> * @param <T> the type
* @see WeakSet * @see WeakSet
*/ */
class CopyOnWriteWeakSet<T> extends WeakSet<T> { class CopyOnWriteWeakSet<T> extends WeakSet<T> {

View file

@ -60,9 +60,8 @@ public abstract class DomainObjectAdapter implements DomainObject {
private ArrayList<Object> consumers; private ArrayList<Object> consumers;
protected Map<String, String> metadata = new LinkedHashMap<String, String>(); protected Map<String, String> metadata = new LinkedHashMap<String, String>();
// a flag indicating whether the domain object has changed // A flag indicating whether the domain object has changed. Any methods of this domain object
// any methods of this domain object which cause its state to // which cause its state to change must set this flag to true
// to change must set this flag to true
protected boolean changed = false; protected boolean changed = false;
// a flag indicating that this object is temporary // a flag indicating that this object is temporary
@ -73,19 +72,18 @@ public abstract class DomainObjectAdapter implements DomainObject {
/** /**
* Construct a new DomainObjectAdapter. If construction of this object fails, be sure to release * Construct a new DomainObjectAdapter. If construction of this object fails, be sure to release
* with consumer. * with consumer.
* *
* @param name name of the object * @param name name of the object
* @param timeInterval the time (in milliseconds) to wait before the event queue is flushed. If * @param timeInterval the time (in milliseconds) to wait before the event queue is flushed. If
* a new event comes in before the time expires, the timer is reset. * a new event comes in before the time expires, the timer is reset.
* @param bufsize initial size of event buffer
* @param consumer the object that created this domain object * @param consumer the object that created this domain object
*/ */
protected DomainObjectAdapter(String name, int timeInterval, int bufsize, Object consumer) { protected DomainObjectAdapter(String name, int timeInterval, Object consumer) {
if (consumer == null) { if (consumer == null) {
throw new IllegalArgumentException("Consumer must not be null"); throw new IllegalArgumentException("Consumer must not be null");
} }
this.name = name; this.name = name;
docs = new DomainObjectChangeSupport(this, timeInterval, bufsize, lock); docs = new DomainObjectChangeSupport(this, timeInterval, lock);
consumers = new ArrayList<Object>(); consumers = new ArrayList<Object>();
consumers.add(consumer); consumers.add(consumer);
if (!UserData.class.isAssignableFrom(getClass())) { if (!UserData.class.isAssignableFrom(getClass())) {
@ -94,9 +92,6 @@ public abstract class DomainObjectAdapter implements DomainObject {
} }
} }
/**
* @see ghidra.framework.model.DomainObject#release(java.lang.Object)
*/
@Override @Override
public void release(Object consumer) { public void release(Object consumer) {
synchronized (consumers) { synchronized (consumers) {
@ -115,9 +110,6 @@ public abstract class DomainObjectAdapter implements DomainObject {
return lock; return lock;
} }
/**
* @see ghidra.framework.model.DomainObject#getDomainFile()
*/
@Override @Override
public DomainFile getDomainFile() { public DomainFile getDomainFile() {
return domainFile; return domainFile;
@ -126,7 +118,7 @@ public abstract class DomainObjectAdapter implements DomainObject {
/** /**
* Returns the hidden user-filesystem associated with this objects domain file, or null if * Returns the hidden user-filesystem associated with this objects domain file, or null if
* unknown. * unknown.
* *
* @return user data file system * @return user data file system
*/ */
protected FileSystem getAssociatedUserFilesystem() { protected FileSystem getAssociatedUserFilesystem() {
@ -136,17 +128,11 @@ public abstract class DomainObjectAdapter implements DomainObject {
return null; return null;
} }
/**
* @see ghidra.framework.model.DomainObject#getName()
*/
@Override @Override
public String getName() { public String getName() {
return name; return name;
} }
/**
* @see java.lang.Object#toString()
*/
@Override @Override
public String toString() { public String toString() {
String classname = getClass().getName(); String classname = getClass().getName();
@ -154,9 +140,6 @@ public abstract class DomainObjectAdapter implements DomainObject {
return name + " - " + classname; return name + " - " + classname;
} }
/**
* @see ghidra.framework.model.DomainObject#setName(java.lang.String)
*/
@Override @Override
public void setName(String newName) { public void setName(String newName) {
synchronized (this) { synchronized (this) {
@ -180,25 +163,16 @@ public abstract class DomainObjectAdapter implements DomainObject {
} }
} }
/**
* @see ghidra.framework.model.DomainObject#isChanged()
*/
@Override @Override
public boolean isChanged() { public boolean isChanged() {
return changed && !temporary; return changed && !temporary;
} }
/**
* @see ghidra.framework.model.DomainObject#setTemporary(boolean)
*/
@Override @Override
public void setTemporary(boolean state) { public void setTemporary(boolean state) {
temporary = state; temporary = state;
} }
/**
* @see ghidra.framework.model.DomainObject#isTemporary()
*/
@Override @Override
public boolean isTemporary() { public boolean isTemporary() {
return temporary; return temporary;
@ -237,9 +211,6 @@ public abstract class DomainObjectAdapter implements DomainObject {
closeListeners.clear(); closeListeners.clear();
} }
/**
* @see ghidra.framework.model.DomainObject#flushEvents()
*/
@Override @Override
public void flushEvents() { public void flushEvents() {
docs.flush(); docs.flush();
@ -250,24 +221,18 @@ public abstract class DomainObjectAdapter implements DomainObject {
/** /**
* Return "changed" status * Return "changed" status
* *
* @return true if this object has changed * @return true if this object has changed
*/ */
public boolean getChangeStatus() { public boolean getChangeStatus() {
return changed; return changed;
} }
/**
* @see ghidra.framework.model.DomainObject#addListener(ghidra.framework.model.DomainObjectListener)
*/
@Override @Override
public void addListener(DomainObjectListener l) { public void addListener(DomainObjectListener l) {
docs.addListener(l); docs.addListener(l);
} }
/**
* @see ghidra.framework.model.DomainObject#removeListener(ghidra.framework.model.DomainObjectListener)
*/
@Override @Override
public void removeListener(DomainObjectListener l) { public void removeListener(DomainObjectListener l) {
docs.removeListener(l); docs.removeListener(l);
@ -286,7 +251,7 @@ public abstract class DomainObjectAdapter implements DomainObject {
@Override @Override
public EventQueueID createPrivateEventQueue(DomainObjectListener listener, int maxDelay) { public EventQueueID createPrivateEventQueue(DomainObjectListener listener, int maxDelay) {
EventQueueID eventQueueID = new EventQueueID(); EventQueueID eventQueueID = new EventQueueID();
DomainObjectChangeSupport queue = new DomainObjectChangeSupport(this, maxDelay, 1000, lock); DomainObjectChangeSupport queue = new DomainObjectChangeSupport(this, maxDelay, lock);
queue.addListener(listener); queue.addListener(listener);
changeSupportMap.put(eventQueueID, queue); changeSupportMap.put(eventQueueID, queue);
return eventQueueID; return eventQueueID;
@ -310,15 +275,12 @@ public abstract class DomainObjectAdapter implements DomainObject {
} }
} }
/**
* @see ghidra.framework.model.DomainObject#getDescription()
*/
@Override @Override
public abstract String getDescription(); public abstract String getDescription();
/** /**
* Fires the specified event. * Fires the specified event.
* *
* @param ev event to fire * @param ev event to fire
*/ */
public void fireEvent(DomainObjectChangeRecord ev) { public void fireEvent(DomainObjectChangeRecord ev) {
@ -331,9 +293,6 @@ public abstract class DomainObjectAdapter implements DomainObject {
} }
} }
/**
* @see ghidra.framework.model.DomainObject#setEventsEnabled(boolean)
*/
@Override @Override
public void setEventsEnabled(boolean v) { public void setEventsEnabled(boolean v) {
if (eventsEnabled != v) { if (eventsEnabled != v) {
@ -353,9 +312,6 @@ public abstract class DomainObjectAdapter implements DomainObject {
return eventsEnabled; return eventsEnabled;
} }
/**
* @see ghidra.framework.model.DomainObject#hasExclusiveAccess()
*/
@Override @Override
public boolean hasExclusiveAccess() { public boolean hasExclusiveAccess() {
return domainFile == null || !domainFile.isCheckedOut() || return domainFile == null || !domainFile.isCheckedOut() ||
@ -372,9 +328,6 @@ public abstract class DomainObjectAdapter implements DomainObject {
changed = state; changed = state;
} }
/**
* @see ghidra.framework.model.DomainObject#addConsumer(java.lang.Object)
*/
@Override @Override
public boolean addConsumer(Object consumer) { public boolean addConsumer(Object consumer) {
if (consumer == null) { if (consumer == null) {
@ -403,7 +356,9 @@ public abstract class DomainObjectAdapter implements DomainObject {
} }
/** /**
* Returns true if the this file is used only by the given tool * Returns true if the this file is used only by the given consumer
* @param consumer the consumer
* @return true if the this file is used only by the given consumer
*/ */
boolean isUsedExclusivelyBy(Object consumer) { boolean isUsedExclusivelyBy(Object consumer) {
synchronized (consumers) { synchronized (consumers) {
@ -430,7 +385,7 @@ public abstract class DomainObjectAdapter implements DomainObject {
/** /**
* Set default content type * Set default content type
* *
* @param doClass default domain object implementation * @param doClass default domain object implementation
*/ */
public static synchronized void setDefaultContentClass(Class<?> doClass) { public static synchronized void setDefaultContentClass(Class<?> doClass) {
@ -450,9 +405,10 @@ public abstract class DomainObjectAdapter implements DomainObject {
/** /**
* Get the ContentHandler associated with the specified content-type. * Get the ContentHandler associated with the specified content-type.
* *
* @param contentType domain object content type * @param contentType domain object content type
* @return content handler * @return content handler
* @throws IOException if no content handler can be found
*/ */
static synchronized ContentHandler getContentHandler(String contentType) throws IOException { static synchronized ContentHandler getContentHandler(String contentType) throws IOException {
checkContentHandlerMaps(); checkContentHandlerMaps();
@ -465,9 +421,10 @@ public abstract class DomainObjectAdapter implements DomainObject {
/** /**
* Get the ContentHandler associated with the specified domain object * Get the ContentHandler associated with the specified domain object
* *
* @param dobj domain object * @param dobj domain object
* @return content handler * @return content handler
* @throws IOException if no content handler can be found
*/ */
public static synchronized ContentHandler getContentHandler(DomainObject dobj) public static synchronized ContentHandler getContentHandler(DomainObject dobj)
throws IOException { throws IOException {

View file

@ -90,12 +90,10 @@ public abstract class DomainObjectAdapterDB extends DomainObjectAdapter
* @param timeInterval the time (in milliseconds) to wait before the * @param timeInterval the time (in milliseconds) to wait before the
* event queue is flushed. If a new event comes in before the time expires, * event queue is flushed. If a new event comes in before the time expires,
* the timer is reset. * the timer is reset.
* @param bufSize initial size of event buffer
* @param consumer the object that created this domain object * @param consumer the object that created this domain object
*/ */
protected DomainObjectAdapterDB(DBHandle dbh, String name, int timeInterval, int bufSize, protected DomainObjectAdapterDB(DBHandle dbh, String name, int timeInterval, Object consumer) {
Object consumer) { super(name, timeInterval, consumer);
super(name, timeInterval, bufSize, consumer);
this.dbh = dbh; this.dbh = dbh;
options = new OptionsDB(this); options = new OptionsDB(this);
transactionMgr = new DomainObjectTransactionManager(this); transactionMgr = new DomainObjectTransactionManager(this);

View file

@ -25,13 +25,24 @@ import ghidra.util.*;
import ghidra.util.datastruct.WeakDataStructureFactory; import ghidra.util.datastruct.WeakDataStructureFactory;
import ghidra.util.datastruct.WeakSet; import ghidra.util.datastruct.WeakSet;
/**
* A class to queue and send {@link DomainObjectChangeRecord} events.
* <p>
* For simplicity, this class requires all mutations to internal data structures to be locked using
* the internal write lock. Clients are not required to use any synchronization when using this
* class.
* <p>
* Internally, events are queued and will be fired on a timer.
*/
class DomainObjectChangeSupport { class DomainObjectChangeSupport {
private WeakSet<DomainObjectListener> listeners; private WeakSet<DomainObjectListener> listeners =
private DomainObject src; WeakDataStructureFactory.createSingleThreadAccessWeakSet();
private List<DomainObjectChangeRecord> changesQueue; private List<EventNotification> notificationQueue = new ArrayList<>();
private List<DomainObjectChangeRecord> recordsQueue = new ArrayList<>();
private GhidraTimer timer; private GhidraTimer timer;
private DomainObject src;
private Lock domainObjectLock; private Lock domainObjectLock;
private Lock writeLock = new Lock("DOCS Change Records Queue Lock"); private Lock writeLock = new Lock("DOCS Change Records Queue Lock");
@ -39,114 +50,95 @@ class DomainObjectChangeSupport {
/** /**
* Constructs a new DomainObjectChangeSupport object. * Constructs a new DomainObjectChangeSupport object.
* *
* @param src The object to be put as the src for all events generated. * @param src The object to be put as the src for all events generated.
* @param timeInterval The time (in milliseconds) this object will wait before flushing its * @param timeInterval The time (in milliseconds) this object will wait before flushing its
* event buffer. If a new event comes in before the time expires, the timer is reset. * event buffer. If a new event comes in before the time expires, the timer is reset.
* @param lock the lock used to verify that calls to {@link #flush()} are not performed while a * @param lock the lock used to verify that calls to {@link #flush()} are not performed while a
* lock is held; this is the lock to guard the DB * lock is held; this is the lock to guard the DB
*/ */
DomainObjectChangeSupport(DomainObject src, int timeInterval, int bufsize, Lock lock) { DomainObjectChangeSupport(DomainObject src, int timeInterval, Lock lock) {
this.src = src; this.src = Objects.requireNonNull(src);
this.domainObjectLock = Objects.requireNonNull(lock); this.domainObjectLock = Objects.requireNonNull(lock);
changesQueue = new ArrayList<>(bufsize); this.timer =
GhidraTimerFactory.getGhidraTimer(timeInterval, timeInterval, this::sendEventNow);
listeners = WeakDataStructureFactory.createCopyOnWriteWeakSet();
timer = GhidraTimerFactory.getGhidraTimer(timeInterval, timeInterval, () -> sendEventNow());
timer.setInitialDelay(25); timer.setInitialDelay(25);
timer.setDelay(500); timer.setDelay(500);
timer.setRepeats(true); timer.setRepeats(true);
} }
void addListener(DomainObjectListener listener) { // Note: must be called on the Swing thread
// Capture the pending event to send to the existing listeners. This prevents the new
// listener from getting events registered before the listener was added.
DomainObjectChangedEvent pendingEvent;
List<DomainObjectListener> previousListeners;
synchronized (src) {
pendingEvent = convertEventQueueRecordsToEvent();
previousListeners = atomicAddListener(listener);
}
/*
* Do outside the synchronized block, so that we do not get this deadlock:
* Thread1 has Domain Lock -> wants AWT lock
* Swing has AWT lock -> wants Domain lock
*/
SystemUtilities.runIfSwingOrPostSwingLater(
() -> notifyEvent(previousListeners, pendingEvent));
}
void removeListener(DomainObjectListener listener) {
synchronized (src) {
listeners.remove(listener);
}
}
private void sendEventNow() { private void sendEventNow() {
DomainObjectChangedEvent ev = convertEventQueueRecordsToEvent(); List<EventNotification> notifications = withLock(() -> {
notifyEvent(listeners, ev);
}
private DomainObjectChangedEvent convertEventQueueRecordsToEvent() { DomainObjectChangedEvent e = createEventFromQueuedRecords();
notificationQueue.add(new EventNotification(e, new ArrayList<>(listeners.values())));
DomainObjectChangedEvent event = lockQueue(() -> { List<EventNotification> existingNotifications = new ArrayList<>(notificationQueue);
notificationQueue.clear();
if (changesQueue.isEmpty()) { return existingNotifications;
timer.stop();
return null;
}
DomainObjectChangedEvent e = new DomainObjectChangedEvent(src, changesQueue);
changesQueue = new ArrayList<>();
return e;
}); });
return event; for (EventNotification notification : notifications) {
notification.doNotify();
}
} }
// This version of notify takes in the listeners to notify so that we can send events to // Note: must be called inside of withLock()
// some listeners, but not all of them (like flushing when adding new listeners) private DomainObjectChangedEvent createEventFromQueuedRecords() {
private void notifyEvent(Iterable<DomainObjectListener> listenersToNotify,
DomainObjectChangedEvent ev) {
if (ev == null) { if (recordsQueue.isEmpty()) {
return; // this implies there we no changes when the timer expired timer.stop();
return null;
} }
DomainObjectChangedEvent e = new DomainObjectChangedEvent(src, recordsQueue);
recordsQueue = new ArrayList<>();
return e;
}
void addListener(DomainObjectListener listener) {
if (isDisposed) { if (isDisposed) {
return; return;
} }
for (DomainObjectListener dol : listenersToNotify) { withLock(() -> {
try {
dol.domainObjectChanged(ev); // Capture the pending event to send to the existing listeners. This prevents the new
} // listener from getting events registered before the listener was added.
catch (Exception exc) { DomainObjectChangedEvent pendingEvent = createEventFromQueuedRecords();
Msg.showError(this, null, "Error", "Error in Domain Object listener", exc); List<DomainObjectListener> previousListeners = new ArrayList<>(listeners.values());
} listeners.add(listener);
notificationQueue.add(new EventNotification(pendingEvent, previousListeners));
timer.start();
});
}
void removeListener(DomainObjectListener listener) {
if (isDisposed) {
return;
} }
withLock(() -> listeners.remove(listener));
} }
void flush() { void flush() {
Thread lockOwner = domainObjectLock.getOwner(); Thread lockOwner = domainObjectLock.getOwner();
if (lockOwner == Thread.currentThread()) { if (lockOwner == Thread.currentThread()) {
/* //
* We have decided that flushing events with a lock can lead to deadlocks. There // We have decided that flushing events with a lock can lead to deadlocks. There
* should be no reason to flush events while holding a lock. This is the // should be no reason to flush events while holding a lock. This is the potential
* potential deadlock: // deadlock:
* Thread1 has Domain Lock -> wants AWT lock // Thread1 has Domain Lock -> wants AWT lock
* Swing has AWT lock -> wants Domain lock // Swing has AWT lock -> wants Domain lock
*/ //
throw new IllegalStateException("Cannot call flush() with locks!"); throw new IllegalStateException("Cannot call flush() with locks!");
} }
SystemUtilities.runSwingNow(() -> sendEventNow()); Swing.runNow(this::sendEventNow);
} }
void fireEvent(DomainObjectChangeRecord docr) { void fireEvent(DomainObjectChangeRecord docr) {
@ -155,15 +147,20 @@ class DomainObjectChangeSupport {
return; return;
} }
lockQueue(() -> { withLock(() -> {
changesQueue.add(docr); recordsQueue.add(docr);
timer.start(); timer.start();
}); });
} }
void fatalErrorOccurred(final Throwable t) { void fatalErrorOccurred(Throwable t) {
List<DomainObjectListener> listenersCopy = new ArrayList<>(listeners.values()); if (isDisposed) {
return;
}
List<DomainObjectListener> listenersCopy =
withLock(() -> new ArrayList<>(listeners.values()));
dispose(); dispose();
@ -176,42 +173,35 @@ class DomainObjectChangeSupport {
l.domainObjectChanged(ev); l.domainObjectChanged(ev);
} }
catch (Throwable t2) { catch (Throwable t2) {
// I guess we don't care (probably because some other fatal error has // We don't care (probably because some other fatal error has already happened)
// already happened)
} }
} }
}; };
SystemUtilities.runSwingLater(errorTask); Swing.runLater(errorTask);
} }
void dispose() { void dispose() {
lockQueue(() -> { if (isDisposed) {
isDisposed = true; return;
timer.stop();
changesQueue.clear();
});
listeners.clear();
}
private List<DomainObjectListener> atomicAddListener(DomainObjectListener l) {
List<DomainObjectListener> previousListeners = new ArrayList<>();
for (DomainObjectListener listener : listeners) {
previousListeners.add(listener);
} }
listeners.add(l); withLock(() -> {
return previousListeners; isDisposed = true;
timer.stop();
recordsQueue.clear();
notificationQueue.clear();
listeners.clear();
});
} }
//================================================================================================== //=================================================================================================
// Lock Methods // Lock Methods
//================================================================================================== //=================================================================================================
private void lockQueue(Runnable r) { // Note: all clients of lockQueue() must not call external APIs that could use locking
private void withLock(Runnable r) {
try { try {
writeLock.acquire(); writeLock.acquire();
@ -222,7 +212,8 @@ class DomainObjectChangeSupport {
} }
} }
private <T> T lockQueue(Callable<T> c) { // Note: all clients of lockQueue() must not call external APIs that could use locking
private <T> T withLock(Callable<T> c) {
try { try {
writeLock.acquire(); writeLock.acquire();
@ -241,4 +232,49 @@ class DomainObjectChangeSupport {
writeLock.release(); writeLock.release();
} }
} }
//=================================================================================================
// Inner Classes
//=================================================================================================
/**
* This class allows us to bind the given event with the given listeners. This is used to
* send events to the correct listeners as listeners are added. In other words, new listeners
* will not receive pre-existing buffered events. Also, using this class allows us to ensure
* events are processed linearly by processing each of these notification objects linearly
* from a single queue.
*
* Note: this class shall perform no synchronization; that shall be handled by the client
*/
private class EventNotification {
private DomainObjectChangedEvent event;
private List<DomainObjectListener> receivers;
EventNotification(DomainObjectChangedEvent event, List<DomainObjectListener> recievers) {
this.event = event;
this.receivers = recievers;
}
// Note: must be called on the Swing thread; must be called outside of lockQueue()
void doNotify() {
if (isDisposed) {
return;
}
if (event == null) {
return; // this implies there were no changes when the timer expired
}
for (DomainObjectListener dol : receivers) {
try {
dol.domainObjectChanged(event);
}
catch (Exception exc) {
Msg.showError(this, null, "Error", "Error in Domain Object listener", exc);
}
}
}
}
} }

View file

@ -1774,7 +1774,7 @@ public class GhidraFileData {
private class GenericDomainObjectDB extends DomainObjectAdapterDB { private class GenericDomainObjectDB extends DomainObjectAdapterDB {
protected GenericDomainObjectDB(DBHandle dbh) throws IOException { protected GenericDomainObjectDB(DBHandle dbh) throws IOException {
super(dbh, "Generic", 500, 1000, GhidraFileData.this); super(dbh, "Generic", 500, GhidraFileData.this);
loadMetadata(); loadMetadata();
} }

View file

@ -29,7 +29,7 @@ public class DummyDomainObject extends DomainObjectAdapterDB {
} }
public DummyDomainObject(String name, Object consumer) throws IOException { public DummyDomainObject(String name, Object consumer) throws IOException {
super(new DBHandle(), name, 10, 1, consumer); super(new DBHandle(), name, 10, consumer);
} }
@Override @Override

View file

@ -28,7 +28,7 @@ public class GenericDomainObjectDB extends DomainObjectAdapterDB {
List<String> transactionsList = new ArrayList<String>(); List<String> transactionsList = new ArrayList<String>();
public GenericDomainObjectDB(Object consumer) throws IOException { public GenericDomainObjectDB(Object consumer) throws IOException {
super(new DBHandle(), "Generic", 500, 1000, consumer); super(new DBHandle(), "Generic", 500, consumer);
} }
@Override @Override

View file

@ -100,7 +100,7 @@ public class DataTypeArchiveDB extends DomainObjectAdapterDB
*/ */
public DataTypeArchiveDB(DomainFolder folder, String name, Object consumer) public DataTypeArchiveDB(DomainFolder folder, String name, Object consumer)
throws IOException, DuplicateNameException, InvalidNameException { throws IOException, DuplicateNameException, InvalidNameException {
super(new DBHandle(), name, 500, 1000, consumer); super(new DBHandle(), name, 500, consumer);
this.name = name; this.name = name;
recordChanges = false; recordChanges = false;
@ -153,7 +153,7 @@ public class DataTypeArchiveDB extends DomainObjectAdapterDB
public DataTypeArchiveDB(DBHandle dbh, int openMode, TaskMonitor monitor, Object consumer) public DataTypeArchiveDB(DBHandle dbh, int openMode, TaskMonitor monitor, Object consumer)
throws IOException, VersionException, CancelledException { throws IOException, VersionException, CancelledException {
super(dbh, "Untitled", 500, 1000, consumer); super(dbh, "Untitled", 500, consumer);
if (monitor == null) { if (monitor == null) {
monitor = TaskMonitorAdapter.DUMMY_MONITOR; monitor = TaskMonitorAdapter.DUMMY_MONITOR;
} }

View file

@ -218,7 +218,7 @@ public class ProgramDB extends DomainObjectAdapterDB implements Program, ChangeM
*/ */
public ProgramDB(String name, Language language, CompilerSpec compilerSpec, Object consumer) public ProgramDB(String name, Language language, CompilerSpec compilerSpec, Object consumer)
throws IOException { throws IOException {
super(new DBHandle(), name, 500, 1000, consumer); super(new DBHandle(), name, 500, consumer);
if (!(compilerSpec instanceof BasicCompilerSpec)) { if (!(compilerSpec instanceof BasicCompilerSpec)) {
throw new IllegalArgumentException( throw new IllegalArgumentException(
@ -287,7 +287,7 @@ public class ProgramDB extends DomainObjectAdapterDB implements Program, ChangeM
public ProgramDB(DBHandle dbh, int openMode, TaskMonitor monitor, Object consumer) public ProgramDB(DBHandle dbh, int openMode, TaskMonitor monitor, Object consumer)
throws IOException, VersionException, LanguageNotFoundException, CancelledException { throws IOException, VersionException, LanguageNotFoundException, CancelledException {
super(dbh, "Untitled", 500, 1000, consumer); super(dbh, "Untitled", 500, consumer);
if (monitor == null) { if (monitor == null) {
monitor = TaskMonitor.DUMMY; monitor = TaskMonitor.DUMMY;

View file

@ -122,7 +122,7 @@ class ProgramUserDataDB extends DomainObjectAdapterDB implements ProgramUserData
} }
public ProgramUserDataDB(ProgramDB program) throws IOException { public ProgramUserDataDB(ProgramDB program) throws IOException {
super(new DBHandle(), getName(program), 500, 1000, program); super(new DBHandle(), getName(program), 500, program);
this.program = program; this.program = program;
this.language = program.getLanguage(); this.language = program.getLanguage();
languageID = language.getLanguageID(); languageID = language.getLanguageID();
@ -162,7 +162,7 @@ class ProgramUserDataDB extends DomainObjectAdapterDB implements ProgramUserData
public ProgramUserDataDB(DBHandle dbh, ProgramDB program, TaskMonitor monitor) public ProgramUserDataDB(DBHandle dbh, ProgramDB program, TaskMonitor monitor)
throws IOException, VersionException, LanguageNotFoundException, CancelledException { throws IOException, VersionException, LanguageNotFoundException, CancelledException {
super(dbh, getName(program), 500, 1000, program); super(dbh, getName(program), 500, program);
this.program = program; this.program = program;
if (monitor == null) { if (monitor == null) {
monitor = TaskMonitorAdapter.DUMMY_MONITOR; monitor = TaskMonitorAdapter.DUMMY_MONITOR;