8030702: Deadlock between subclass of AbstractDocument and UndoManager
Reviewed-by: alexsch, azvegint
--- a/jdk/src/java.desktop/share/classes/javax/swing/text/AbstractDocument.java Tue Dec 01 19:07:45 2015 +0300
+++ b/jdk/src/java.desktop/share/classes/javax/swing/text/AbstractDocument.java Tue Dec 01 19:21:40 2015 +0300
@@ -36,6 +36,7 @@
import sun.font.BidiUtils;
import sun.swing.SwingUtilities2;
+import sun.swing.text.UndoableEditLockSupport;
/**
* An implementation of the document interface to serve as a
@@ -275,6 +276,11 @@
* @see EventListenerList
*/
protected void fireUndoableEditUpdate(UndoableEditEvent e) {
+ if (e.getEdit() instanceof DefaultDocumentEvent) {
+ e = new UndoableEditEvent(e.getSource(),
+ new DefaultDocumentEventUndoableWrapper(
+ (DefaultDocumentEvent)e.getEdit()));
+ }
// Guaranteed to return a non-null array
Object[] listeners = listenerList.getListenerList();
// Process the listeners last to first, notifying
@@ -2952,6 +2958,88 @@
}
+ static class DefaultDocumentEventUndoableWrapper implements
+ UndoableEdit, UndoableEditLockSupport
+ {
+ final DefaultDocumentEvent dde;
+ public DefaultDocumentEventUndoableWrapper(DefaultDocumentEvent dde) {
+ this.dde = dde;
+ }
+
+ @Override
+ public void undo() throws CannotUndoException {
+ dde.undo();
+ }
+
+ @Override
+ public boolean canUndo() {
+ return dde.canUndo();
+ }
+
+ @Override
+ public void redo() throws CannotRedoException {
+ dde.redo();
+ }
+
+ @Override
+ public boolean canRedo() {
+ return dde.canRedo();
+ }
+
+ @Override
+ public void die() {
+ dde.die();
+ }
+
+ @Override
+ public boolean addEdit(UndoableEdit anEdit) {
+ return dde.addEdit(anEdit);
+ }
+
+ @Override
+ public boolean replaceEdit(UndoableEdit anEdit) {
+ return dde.replaceEdit(anEdit);
+ }
+
+ @Override
+ public boolean isSignificant() {
+ return dde.isSignificant();
+ }
+
+ @Override
+ public String getPresentationName() {
+ return dde.getPresentationName();
+ }
+
+ @Override
+ public String getUndoPresentationName() {
+ return dde.getUndoPresentationName();
+ }
+
+ @Override
+ public String getRedoPresentationName() {
+ return dde.getRedoPresentationName();
+ }
+
+ /**
+ * {@inheritDoc}
+ * @since 1.9
+ */
+ @Override
+ public void lockEdit() {
+ ((AbstractDocument)dde.getDocument()).writeLock();
+ }
+
+ /**
+ * {@inheritDoc}
+ * @since 1.9
+ */
+ @Override
+ public void unlockEdit() {
+ ((AbstractDocument)dde.getDocument()).writeUnlock();
+ }
+ }
+
/**
* This event used when firing document changes while Undo/Redo
* operations. It just wraps DefaultDocumentEvent and delegates
--- a/jdk/src/java.desktop/share/classes/javax/swing/undo/UndoManager.java Tue Dec 01 19:07:45 2015 +0300
+++ b/jdk/src/java.desktop/share/classes/javax/swing/undo/UndoManager.java Tue Dec 01 19:21:40 2015 +0300
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1997, 2014, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2015, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -28,6 +28,7 @@
import javax.swing.event.*;
import javax.swing.UIManager;
import java.util.*;
+import sun.swing.text.UndoableEditLockSupport;
/**
* {@code UndoManager} manages a list of {@code UndoableEdits},
@@ -134,6 +135,11 @@
*/
@SuppressWarnings("serial") // Same-version serialization only
public class UndoManager extends CompoundEdit implements UndoableEditListener {
+ private enum Action {
+ UNDO,
+ REDO,
+ ANY
+ }
int indexOfNextAdd;
int limit;
@@ -369,13 +375,8 @@
* @throws CannotRedoException if one of the edits throws
* <code>CannotRedoException</code>
*/
- public synchronized void undoOrRedo() throws CannotRedoException,
- CannotUndoException {
- if (indexOfNextAdd == edits.size()) {
- undo();
- } else {
- redo();
- }
+ public void undoOrRedo() throws CannotRedoException, CannotUndoException {
+ tryUndoOrRedo(Action.ANY);
}
/**
@@ -407,16 +408,8 @@
* @see #canUndo
* @see #editToBeUndone
*/
- public synchronized void undo() throws CannotUndoException {
- if (inProgress) {
- UndoableEdit edit = editToBeUndone();
- if (edit == null) {
- throw new CannotUndoException();
- }
- undoTo(edit);
- } else {
- super.undo();
- }
+ public void undo() throws CannotUndoException {
+ tryUndoOrRedo(Action.UNDO);
}
/**
@@ -452,16 +445,90 @@
* @see #canRedo
* @see #editToBeRedone
*/
- public synchronized void redo() throws CannotRedoException {
- if (inProgress) {
- UndoableEdit edit = editToBeRedone();
- if (edit == null) {
- throw new CannotRedoException();
+ public void redo() throws CannotRedoException {
+ tryUndoOrRedo(Action.REDO);
+ }
+
+ private void tryUndoOrRedo(Action action) {
+ UndoableEditLockSupport lockSupport = null;
+ boolean undo;
+ synchronized (this) {
+ if (action == Action.ANY) {
+ undo = indexOfNextAdd == edits.size();
+ } else {
+ undo = action == Action.UNDO;
+ }
+ if (inProgress) {
+ UndoableEdit edit = undo ? editToBeUndone() : editToBeRedone();
+ if (edit == null) {
+ throw undo ? new CannotUndoException() :
+ new CannotRedoException();
+ }
+ lockSupport = getEditLockSupport(edit);
+ if (lockSupport == null) {
+ if (undo) {
+ undoTo(edit);
+ } else {
+ redoTo(edit);
+ }
+ return;
+ }
+ } else {
+ if (undo) {
+ super.undo();
+ } else {
+ super.redo();
+ }
+ return;
}
- redoTo(edit);
- } else {
- super.redo();
}
+ // the edit synchronization is required
+ while (true) {
+ lockSupport.lockEdit();
+ UndoableEditLockSupport editLockSupport = null;
+ try {
+ synchronized (this) {
+ if (action == Action.ANY) {
+ undo = indexOfNextAdd == edits.size();
+ }
+ if (inProgress) {
+ UndoableEdit edit = undo ? editToBeUndone() :
+ editToBeRedone();
+ if (edit == null) {
+ throw undo ? new CannotUndoException() :
+ new CannotRedoException();
+ }
+ editLockSupport = getEditLockSupport(edit);
+ if (editLockSupport == null ||
+ editLockSupport == lockSupport) {
+ if (undo) {
+ undoTo(edit);
+ } else {
+ redoTo(edit);
+ }
+ return;
+ }
+ } else {
+ if (undo) {
+ super.undo();
+ } else {
+ super.redo();
+ }
+ return;
+ }
+ }
+ } finally {
+ if (lockSupport != null) {
+ lockSupport.unlockEdit();
+ }
+ lockSupport = editLockSupport;
+ }
+ }
+ }
+
+ private UndoableEditLockSupport getEditLockSupport(UndoableEdit anEdit) {
+ return anEdit instanceof UndoableEditLockSupport ?
+ (UndoableEditLockSupport)anEdit : null;
}
/**
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/src/java.desktop/share/classes/sun/swing/text/UndoableEditLockSupport.java Tue Dec 01 19:21:40 2015 +0300
@@ -0,0 +1,43 @@
+/*
+ * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation. Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package sun.swing.text;
+
+import javax.swing.undo.UndoableEdit;
+
+/**
+ * UndoableEdit support for undo/redo actions synchronization
+ * @since 1.9
+ */
+public interface UndoableEditLockSupport extends UndoableEdit {
+ /**
+ * lock the UndoableEdit for threadsafe undo/redo
+ */
+ void lockEdit();
+
+ /**
+ * unlock the UndoableEdit
+ */
+ void unlockEdit();
+}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/javax/swing/undo/UndoManager/AbstractDocumentUndoConcurrentTest.java Tue Dec 01 19:21:40 2015 +0300
@@ -0,0 +1,122 @@
+/*
+ * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/* @test
+ @bug 8030702
+ @summary Deadlock between subclass of AbstractDocument and UndoManager
+ @author Semyon Sadetsky
+ */
+
+import javax.swing.text.PlainDocument;
+import javax.swing.text.StringContent;
+import javax.swing.undo.UndoManager;
+import java.text.DecimalFormat;
+import java.text.Format;
+import java.util.concurrent.CyclicBarrier;
+
+public class AbstractDocumentUndoConcurrentTest {
+ static CyclicBarrier barrier = new CyclicBarrier(3);
+
+ private static PlainDocument doc1;
+ private static PlainDocument doc2;
+ private static Format format1 = new DecimalFormat("<Test1 0000>");
+ private static Format format2 = new DecimalFormat("<Test22 0000>");
+
+ public static void main(String[] args) throws Exception {
+ test();
+ System.out.println(doc1.getText(0, doc1.getLength()));
+ System.out.println(doc2.getText(0, doc2.getLength()));
+ System.out.println("ok");
+ }
+
+ private static void test() throws Exception {
+ doc1 = new PlainDocument(new StringContent());
+ final UndoManager undoManager = new UndoManager();
+
+ doc1.addUndoableEditListener(undoManager);
+ doc1.insertString(0, "<Test1 XXXX>", null);
+
+ doc2 = new PlainDocument(new StringContent());
+
+ doc2.addUndoableEditListener(undoManager);
+ doc2.insertString(0, "<Test22 XXXX>", null);
+
+ Thread t1 = new Thread("Thread doc1") {
+ @Override
+ public void run() {
+ try {
+ barrier.await();
+ for (int i = 0; i < 1000; i++) {
+ doc1.insertString(0, format1.format(i), null);
+ if(doc1.getLength() > 100) doc1.remove(0, 12);
+ }
+
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ System.out.println("t1 done");
+ }
+ };
+
+ Thread t2 = new Thread("Thread doc2") {
+ @Override
+ public void run() {
+ try {
+ barrier.await();
+ for (int i = 0; i < 1000; i++) {
+ doc2.insertString(0, format2.format(i), null);
+ if(doc2.getLength() > 100) doc2.remove(0, 13);
+ }
+
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ System.out.println("t2 done");
+ }
+ };
+
+ Thread t3 = new Thread("Undo/Redo Thread") {
+ @Override
+ public void run() {
+ try {
+ barrier.await();
+ } catch (Exception e) {
+ e.printStackTrace();
+ }
+ for (int i = 0; i < 1000; i++) {
+ undoManager.undoOrRedo();
+ undoManager.undo();
+ }
+ System.out.println("t3 done");
+ }
+ };
+
+ t1.start();
+ t2.start();
+ t3.start();
+
+ t1.join();
+ t2.join();
+ t3.join();
+ }
+}