GP-1859: better choice of synchronized object

GP-1859: fix (fingers-crossed) for deadlock
This commit is contained in:
d-millar 2022-03-24 15:27:24 -04:00
parent 5f2d874bdd
commit 86620575f5
2 changed files with 35 additions and 25 deletions

View file

@ -29,10 +29,8 @@ import ghidra.util.Lock;
import ghidra.util.classfinder.ClassSearcher;
/**
* An abstract class that provides default behavior for
* DomainObject(s), specifically it handles listeners and
* change status; the derived class must provide the
* getDescription() method.
* An abstract class that provides default behavior for DomainObject(s), specifically it handles
* listeners and change status; the derived class must provide the getDescription() method.
*/
public abstract class DomainObjectAdapter implements DomainObject {
@ -72,12 +70,12 @@ public abstract class DomainObjectAdapter implements DomainObject {
private long modificationNumber = 1;
/**
* Construct a new DomainObjectAdapter.
* If construction of this object fails, be sure to release with consumer.
* Construct a new DomainObjectAdapter. If construction of this object fails, be sure to release
* with consumer.
*
* @param name name of the object
* @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.
* @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.
* @param bufsize initial size of event buffer
* @param consumer the object that created this domain object
*/
@ -125,8 +123,9 @@ public abstract class DomainObjectAdapter implements DomainObject {
}
/**
* Returns the hidden user-filesystem associated with
* this objects domain file, or null if unknown.
* Returns the hidden user-filesystem associated with this objects domain file, or null if
* unknown.
*
* @return user data file system
*/
protected FileSystem getAssociatedUserFilesystem() {
@ -250,6 +249,7 @@ public abstract class DomainObjectAdapter implements DomainObject {
/**
* Return "changed" status
*
* @return true if this object has changed
*/
public boolean getChangeStatus() {
@ -260,7 +260,7 @@ public abstract class DomainObjectAdapter implements DomainObject {
* @see ghidra.framework.model.DomainObject#addListener(ghidra.framework.model.DomainObjectListener)
*/
@Override
public synchronized void addListener(DomainObjectListener l) {
public void addListener(DomainObjectListener l) {
docs.addListener(l);
}
@ -268,7 +268,7 @@ public abstract class DomainObjectAdapter implements DomainObject {
* @see ghidra.framework.model.DomainObject#removeListener(ghidra.framework.model.DomainObjectListener)
*/
@Override
public synchronized void removeListener(DomainObjectListener l) {
public void removeListener(DomainObjectListener l) {
docs.removeListener(l);
}
@ -316,9 +316,10 @@ public abstract class DomainObjectAdapter implements DomainObject {
public abstract String getDescription();
/**
* Fires the specified event.
* @param ev event to fire
*/
* Fires the specified event.
*
* @param ev event to fire
*/
public void fireEvent(DomainObjectChangeRecord ev) {
modificationNumber++;
if (eventsEnabled) {
@ -428,6 +429,7 @@ public abstract class DomainObjectAdapter implements DomainObject {
/**
* Set default content type
*
* @param doClass default domain object implementation
*/
public static synchronized void setDefaultContentClass(Class<?> doClass) {
@ -447,6 +449,7 @@ public abstract class DomainObjectAdapter implements DomainObject {
/**
* Get the ContentHandler associated with the specified content-type.
*
* @param contentType domain object content type
* @return content handler
*/
@ -461,6 +464,7 @@ public abstract class DomainObjectAdapter implements DomainObject {
/**
* Get the ContentHandler associated with the specified domain object
*
* @param dobj domain object
* @return content handler
*/

View file

@ -39,12 +39,12 @@ class DomainObjectChangeSupport {
/**
* Constructs a new DomainObjectChangeSupport object.
*
* @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 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 lock is held; this is the lock to guard the DB
* @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.
* @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
*/
DomainObjectChangeSupport(DomainObject src, int timeInterval, int bufsize, Lock lock) {
@ -64,11 +64,15 @@ class DomainObjectChangeSupport {
// 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 = convertEventQueueRecordsToEvent();
List<DomainObjectListener> previousListeners = atomicAddListener(listener);
DomainObjectChangedEvent pendingEvent;
List<DomainObjectListener> previousListeners;
synchronized (src) {
pendingEvent = convertEventQueueRecordsToEvent();
previousListeners = atomicAddListener(listener);
}
/*
* Do later so that we do not get this deadlock:
* 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
*/
@ -77,7 +81,9 @@ class DomainObjectChangeSupport {
}
void removeListener(DomainObjectListener listener) {
listeners.remove(listener);
synchronized (src) {
listeners.remove(listener);
}
}
private void sendEventNow() {