Author: tfmorris
Date: 2008-05-28 12:31:08-0700
New Revision: 14819
Modified:
trunk/src/argouml-app/src/org/argouml/uml/ui/UMLPlainTextDocument.java
Log:
Issue 5110: Fix synchronization deadlock (hopefully). Minimize unnecessary updates. Refactor duplicated code.
Modified: trunk/src/argouml-app/src/org/argouml/uml/ui/UMLPlainTextDocument.java
Url: http://argouml.tigris.org/source/browse/argouml/trunk/src/argouml-app/src/org/argouml/uml/ui/UMLPlainTextDocument.java?view=diff&rev=14819&p1=trunk/src/argouml-app/src/org/argouml/uml/ui/UMLPlainTextDocument.java&p2=trunk/src/argouml-app/src/org/argouml/uml/ui/UMLPlainTextDocument.java&r1=14818&r2=14819
==============================================================================
--- trunk/src/argouml-app/src/org/argouml/uml/ui/UMLPlainTextDocument.java (original)
+++ trunk/src/argouml-app/src/org/argouml/uml/ui/UMLPlainTextDocument.java 2008-05-28 12:31:08-0700
@@ -1,5 +1,5 @@
// $Id$
-// Copyright (c) 1996-2007 The Regents of the University of California. All
+// Copyright (c) 1996-2008 The Regents of the University of California. All
// Rights Reserved. Permission to use, copy, modify, and distribute this
// software and its documentation without fee, and without a written
// agreement is hereby granted, provided that the above copyright notice
@@ -31,6 +31,7 @@
import javax.swing.text.PlainDocument;
import org.apache.log4j.Logger;
+import org.argouml.model.AttributeChangeEvent;
import org.argouml.model.Model;
import org.argouml.model.ModelEventPump;
import org.argouml.ui.targetmanager.TargetEvent;
@@ -64,6 +65,7 @@
/**
* True if an user edits the document directly (by typing in text)
*/
+ @Deprecated
private boolean editing = false;
/**
@@ -92,7 +94,13 @@
* @see java.beans.PropertyChangeListener#propertyChange(java.beans.PropertyChangeEvent)
*/
public void propertyChange(PropertyChangeEvent evt) {
- handleEvent();
+ // NOTE: This may be called from a different thread, so we need to be
+ // careful of the threading restrictions imposed by AbstractDocument
+ // for mutators to be sure we don't deadlock.
+ if (evt instanceof AttributeChangeEvent
+ && eventName.equals(evt.getPropertyName())) {
+ updateText((String) evt.getNewValue());
+ }
}
/**
@@ -120,7 +128,7 @@
eventPump.addModelEventListener(this, panelTarget,
getEventName());
}
- handleEvent();
+ updateText(getProperty());
}
}
@@ -130,30 +138,37 @@
*/
public void insertString(int offset, String str, AttributeSet a)
throws BadLocationException {
+
+ // Mutators hold write lock & will deadlock if use is not thread safe
super.insertString(offset, str, a);
- // TODO: This is updating model on a per character basis as
- // well as unregistering/reregistering event listeners every
- // character - very wasteful - tfm
- if (isFiring()) {
- setFiring(false);
- setProperty(getText(0, getLength()));
- Model.getPump().flushModelEvents();
- setFiring(true);
- }
-
+
+ setPropertyInternal(getText(0, getLength()));
}
+
/*
* @see javax.swing.text.Document#remove(int, int)
*/
public void remove(int offs, int len) throws BadLocationException {
+
+ // Mutators hold write lock & will deadlock if use is not thread safe
super.remove(offs, len);
+
+ setPropertyInternal(getText(0, getLength()));
+ }
+
+ /**
+ * Wrapped version of setProperty which attempts to keep us from hearing
+ * our own echo on the event listener when we change something. Also
+ * skips updates equal the current value.
+ */
+ private void setPropertyInternal(String newValue) {
// TODO: This is updating model on a per character basis as
// well as unregistering/reregistering event listeners every
// character - very wasteful - tfm
- if (isFiring()) {
+ if (isFiring() && !newValue.equals(getProperty())) {
setFiring(false);
- setProperty(getText(0, getLength()));
+ setProperty(newValue);
Model.getPump().flushModelEvents();
setFiring(true);
}
@@ -169,26 +184,49 @@
*/
protected abstract String getProperty();
- private final void setFiring(boolean f) {
+ /**
+ * Enable/disable firing of updates. As a side effect, it unregisters
+ * model event listeners during disable and registers them again during
+ * enable.
+ *
+ * @param f new firing state. Pass false to disable updates.
+ */
+ private final synchronized void setFiring(boolean f) {
ModelEventPump eventPump = Model.getPump();
if (f && panelTarget != null) {
eventPump.addModelEventListener(this, panelTarget, eventName);
- }
- else {
+ } else {
eventPump.removeModelEventListener(this, panelTarget, eventName);
}
firing = f;
}
- private final boolean isFiring() {
+ /**
+ * Return the state of the firing flag. Method is synchronized so it will
+ * return the correct value from any thread.
+ *
+ * @return true firing of updates is allowed currently. Returns false if we
+ * are in the process of doing an update so that we know to ignore
+ * any resulting events.
+ */
+ private final synchronized boolean isFiring() {
return firing;
}
- private final void handleEvent() {
+ private final void updateText(String textValue) {
try {
- setFiring(false);
- super.remove(0, getLength());
- super.insertString(0, getProperty(), null);
+ if (textValue == null) {
+ textValue = "";
+ }
+ String currentValue = getText(0, getLength());
+ if (isFiring() && !textValue.equals(currentValue)) {
+ setFiring(false);
+
+ // Mutators hold write lock & will deadlock
+ // if use is not thread-safe
+ super.remove(0, getLength());
+ super.insertString(0, textValue, null);
+ }
} catch (BadLocationException b) {
LOG.error(
"A BadLocationException happened\n"
@@ -203,7 +241,9 @@
/**
* Returns the editing.
* @return boolean
+ * @deprecated for 0.25.6 by tfmorris. This is a no-op and will be removed.
*/
+ @Deprecated
public boolean isEditing() {
return editing;
}
@@ -211,7 +251,9 @@
/**
* Sets the editing.
* @param ed The editing to set
+ * @deprecated for 0.25.6 by tfmorris. This is a no-op and will be removed.
*/
+ @Deprecated
public void setEditing(boolean ed) {
editing = ed;
}
lmpx.com only provides a reader for public news (NNTP) servers. It is not
affiliated with the servers or forums shown here and is not responsible for
the content of articles, which is written by their respective authors.