From f9a1bf8c28c90fe701fcfd54d10eae8f3c5a648d Mon Sep 17 00:00:00 2001 From: malenkov Date: Mon, 23 Dec 2013 16:24:09 +0400 Subject: [PATCH] 8030118: Document listeners fired outside document lock Reviewed-by: art, serb --- .../javax/swing/text/AbstractDocument.java | 45 ++-------- .../AbstractDocument/7146146/bug7146146.java | 77 ----------------- .../AbstractDocument/8030118/Test8030118.java | 83 +++++++++++++++++++ 3 files changed, 90 insertions(+), 115 deletions(-) delete mode 100644 test/javax/swing/text/AbstractDocument/7146146/bug7146146.java create mode 100644 test/javax/swing/text/AbstractDocument/8030118/Test8030118.java diff --git a/src/share/classes/javax/swing/text/AbstractDocument.java b/src/share/classes/javax/swing/text/AbstractDocument.java index f8a4332a8..db56eacdf 100644 --- a/src/share/classes/javax/swing/text/AbstractDocument.java +++ b/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; - } } diff --git a/test/javax/swing/text/AbstractDocument/7146146/bug7146146.java b/test/javax/swing/text/AbstractDocument/7146146/bug7146146.java deleted file mode 100644 index f4d5bc743..000000000 --- a/test/javax/swing/text/AbstractDocument/7146146/bug7146146.java +++ /dev/null @@ -1,77 +0,0 @@ -/* - * Copyright (c) 2012, 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 7146146 - @summary Deadlock between subclass of AbstractDocument and UndoManager - @author Pavel Porvatov -*/ - -import javax.swing.text.BadLocationException; -import javax.swing.text.PlainDocument; -import javax.swing.text.StringContent; -import javax.swing.undo.UndoManager; - -public class bug7146146 { - public static void main(String[] args) throws Exception { - for (int i = 0; i < 1000; i++) { - System.out.print("Iteration " + i); - - test(); - - System.out.print(" passed"); - } - } - - private static void test() throws Exception { - final PlainDocument doc = new PlainDocument(new StringContent()); - final UndoManager undoManager = new UndoManager(); - - doc.addUndoableEditListener(undoManager); - doc.insertString(0, "", null); - - Thread t1 = new Thread("Thread 1") { - @Override - public void run() { - try { - doc.insertString(0, "", null); - } catch (BadLocationException e) { - throw new RuntimeException(e); - } - } - }; - - Thread t2 = new Thread("Thread 2") { - @Override - public void run() { - undoManager.undo(); - } - }; - - t1.start(); - t2.start(); - - t1.join(); - t2.join(); - } -} diff --git a/test/javax/swing/text/AbstractDocument/8030118/Test8030118.java b/test/javax/swing/text/AbstractDocument/8030118/Test8030118.java new file mode 100644 index 000000000..02bb2a09f --- /dev/null +++ b/test/javax/swing/text/AbstractDocument/8030118/Test8030118.java @@ -0,0 +1,83 @@ +/* + * 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 + * 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. + */ + +import javax.swing.event.DocumentEvent; +import javax.swing.event.DocumentListener; +import javax.swing.text.BadLocationException; +import javax.swing.text.PlainDocument; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +/* + * @test + * @bug 8030118 + * @summary Tests that AbstractDocument cannot be modified from another thread + * @author Sergey Malenkov + */ + +public class Test8030118 implements DocumentListener, Runnable { + private final CountDownLatch latch = new CountDownLatch(1); + private final PlainDocument doc = new PlainDocument(); + + private Test8030118(String string) throws Exception { + this.doc.addDocumentListener(this); + this.doc.insertString(0, string, null); + } + + @Override + public void run() { + try { + this.doc.remove(0, this.doc.getLength()); + } catch (BadLocationException exception) { + throw new Error("unexpected", exception); + } + this.latch.countDown(); + } + + @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); + } + } + + @Override + public void removeUpdate(DocumentEvent event) { + } + + @Override + public void changedUpdate(DocumentEvent event) { + } + + public static void main(String[] args) throws Exception { + new Test8030118("string"); + } +} -- GitLab