8030118: Document listeners fired outside document lock

Reviewed-by: art, serb

src/share/classes/javax/swing/text/AbstractDocument.java

@@ -697,7 +697,6 @@ public abstract class AbstractDocument implements Document, Serializable {
             return;
         }
         DocumentFilter filter = getDocumentFilter();
-        InsertStringResult insertStringResult = null;
 
         writeLock();
 
@@ -705,23 +704,21 @@ public abstract class AbstractDocument implements Document, Serializable {
             if (filter != null) {
                 filter.insertString(getFilterBypass(), offs, str, a);
             } else {
-                insertStringResult = handleInsertString(offs, str, a);
+                handleInsertString(offs, str, a);
             }
         } finally {
             writeUnlock();
         }
-
-        processInsertStringResult(insertStringResult);
     }
 
     /**
      * Performs the actual work of inserting the text; it is assumed the
      * caller has obtained a write lock before invoking this.
      */
-    private InsertStringResult handleInsertString(int offs, String str, AttributeSet a)
+    private void handleInsertString(int offs, String str, AttributeSet a)
             throws BadLocationException {
         if ((str == null) || (str.length() == 0)) {
-            return null;
+            return;
         }
         UndoableEdit u = data.insertString(offs, str);
         DefaultDocumentEvent e =
@@ -748,29 +745,11 @@ public abstract class AbstractDocument implements Document, Serializable {
         insertUpdate(e, a);
         // Mark the edit as done.
         e.end();
-
-        InsertStringResult result = new InsertStringResult();
-
-        result.documentEvent = e;
-
+        fireInsertUpdate(e);
         // only fire undo if Content implementation supports it
         // undo for the composed text is not supported for now
         if (u != null && (a == null || !a.isDefined(StyleConstants.ComposedTextAttribute))) {
-            result.undoableEditEvent = new UndoableEditEvent(this, e);
-        }
-
-        return result;
-    }
-
-    private void processInsertStringResult(InsertStringResult insertStringResult) {
-        if (insertStringResult == null) {
-            return;
-        }
-
-        fireInsertUpdate(insertStringResult.documentEvent);
-
-        if (insertStringResult.undoableEditEvent != null) {
-            fireUndoableEditUpdate(insertStringResult.undoableEditEvent);
+            fireUndoableEditUpdate(new UndoableEditEvent(this, e));
         }
     }
 
@@ -3125,23 +3104,13 @@ public abstract class AbstractDocument implements Document, Serializable {
         public void insertString(int offset, String string,
                                  AttributeSet attr) throws
                                         BadLocationException {
-            InsertStringResult insertStringResult = handleInsertString(offset, string, attr);
-
-            processInsertStringResult(insertStringResult);
+            handleInsertString(offset, string, attr);
         }
 
         public void replace(int offset, int length, String text,
                             AttributeSet attrs) throws BadLocationException {
             handleRemove(offset, length);
-
-            InsertStringResult insertStringResult = handleInsertString(offset, text, attrs);
-
-            processInsertStringResult(insertStringResult);
+            handleInsertString(offset, text, attrs);
         }
     }
-
-    private static class InsertStringResult {
-        DefaultDocumentEvent documentEvent;
-        UndoableEditEvent undoableEditEvent;
-    }
 }

test/javax/swing/text/AbstractDocument/7146146/bug7146146.java → test/javax/swing/text/AbstractDocument/8030118/Test8030118.java

 /*
- * Copyright (c) 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2013, 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
@@ -21,57 +21,63 @@
  * questions.
  */
 
-/* @test
-   @bug 7146146
-   @summary Deadlock between subclass of AbstractDocument and UndoManager
-   @author Pavel Porvatov
-*/
-
+import javax.swing.event.DocumentEvent;
+import javax.swing.event.DocumentListener;
 import javax.swing.text.BadLocationException;
 import javax.swing.text.PlainDocument;
-import javax.swing.text.StringContent;
-import javax.swing.undo.UndoManager;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
 
-public class bug7146146 {
-    public static void main(String[] args) throws Exception {
-        for (int i = 0; i < 1000; i++) {
-            System.out.print("Iteration " + i);
+/*
+ * @test
+ * @bug 8030118
+ * @summary Tests that AbstractDocument cannot be modified from another thread
+ * @author Sergey Malenkov
+ */
 
-            test();
+public class Test8030118 implements DocumentListener, Runnable {
+    private final CountDownLatch latch = new CountDownLatch(1);
+    private final PlainDocument doc = new PlainDocument();
 
-            System.out.print(" passed");
-        }
+    private Test8030118(String string) throws Exception {
+        this.doc.addDocumentListener(this);
+        this.doc.insertString(0, string, null);
     }
 
-    private static void test() throws Exception {
-        final PlainDocument doc = new PlainDocument(new StringContent());
-        final UndoManager undoManager = new UndoManager();
-
-        doc.addUndoableEditListener(undoManager);
-        doc.insertString(0, "<Test 1>", null);
+    @Override
+    public void run() {
+        try {
+            this.doc.remove(0, this.doc.getLength());
+        } catch (BadLocationException exception) {
+            throw new Error("unexpected", exception);
+        }
+        this.latch.countDown();
+    }
 
-        Thread t1 = new Thread("Thread 1") {
-            @Override
-            public void run() {
-                try {
-                    doc.insertString(0, "<Test 2>", null);
-                } catch (BadLocationException e) {
-                    throw new RuntimeException(e);
-                }
-            }
-        };
+    @Override
+    public void insertUpdate(DocumentEvent event) {
+        new Thread(this).start();
+        try {
+            this.latch.await(10, TimeUnit.SECONDS);
+        } catch (InterruptedException exception) {
+            throw new Error("unexpected", exception);
+        }
+        try {
+            event.getDocument().getText(event.getOffset(), event.getLength());
+        } catch (BadLocationException exception) {
+            throw new Error("concurrent modification", exception);
+        }
+    }
 
-        Thread t2 = new Thread("Thread 2") {
-            @Override
-            public void run() {
-                undoManager.undo();
-            }
-        };
+    @Override
+    public void removeUpdate(DocumentEvent event) {
+    }
 
-        t1.start();
-        t2.start();
+    @Override
+    public void changedUpdate(DocumentEvent event) {
+    }
 
-        t1.join();
-        t2.join();
+    public static void main(String[] args) throws Exception {
+        new Test8030118("string");
     }
 }