This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 239364 - Failing DocumentUtilitiesTest on JDK8
Summary: Failing DocumentUtilitiesTest on JDK8
Status: VERIFIED WONTFIX
Alias: None
Product: platform
Classification: Unclassified
Component: JDK Problems (show other bugs)
Version: 8.0
Hardware: All All
: P1 normal (vote)
Assignee: Antonin Nebuzelsky
URL:
Keywords: JDK_8, JDK_SPECIFIC, TEST
Depends on:
Blocks:
 
Reported: 2013-12-11 13:19 UTC by Jiri Skrivanek
Modified: 2014-01-06 08:43 UTC (History)
1 user (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jiri Skrivanek 2013-12-11 13:19:14 UTC
Please, fix tests failing on JDK8

org.netbeans.lib.editor.util.swing.DocumentUtilitiesTest.testIsWriteLocked

To reproduce:

ant -f editor.util test-unit -Dtest.includes=**/DocumentUtilitiesTest.class

http://test4u.cz.oracle.com/view/StableBTD/job/StableBTD-trunk/lastCompletedBuild/testReport/
Comment 1 Miloslav Metelka 2013-12-12 14:14:44 UTC
It appears that there's a significant concept change in javax.swing.text.AbstractDocument in JDK8.
Prior JDK8 the document listeners were fired INSIDE of the document lock upon insert while in JDK8 they are fired OUTSIDE of the lock.
That means that the DocumentListener implementations can no longer rely that the offset and length contained in the document events correspond to the actual document state.

The decission whether to fire inside or outside of the lock is certainly important but it should only happen at the begining of the class development but not when there are already hundreds of existing apps extending the AbstractDocument.

IMO this is a P1 bug in JDK8 and the change causing this should be rolled back ASAP.

The following test demonstrates that a document listener can no longer rely on the document's content on JDK8 (the test passes fine on JDK7):

    public void testInsertedTextIsPresent() throws Exception {
        final PlainDocument doc = new PlainDocument();
        final CountDownLatch insertDone = new CountDownLatch(1);
        final CountDownLatch removeDone = new CountDownLatch(1);

        doc.addDocumentListener(new DocumentListener() {
            public void insertUpdate(DocumentEvent evt) {
                insertDone.countDown();
                try {
                    removeDone.await(1, TimeUnit.SECONDS);
                } catch (InterruptedException ex) {
                    throw new IllegalStateException(ex);
                }

                try {
                    String insertedText = evt.getDocument().getText(evt.getOffset(), evt.getLength());
                } catch (BadLocationException ex) {
                    throw new IllegalStateException(
                            "Inserted text not present in document !!! docLen=" + doc.getLength(), ex);
                }
            }
            public void removeUpdate(DocumentEvent evt) {
            }
            public void changedUpdate(DocumentEvent evt) {
            }
        });
        new Thread(new Runnable() {
            @Override
            public void run() {
                try {
                    insertDone.await();
                    doc.remove(0, doc.getLength());
                    removeDone.countDown();
                } catch (Exception ex) {
                    throw new IllegalStateException(ex);
                }
            }
        }).start();

        doc.insertString(0, "Hello", null);
        
    }
Comment 2 Miloslav Metelka 2013-12-12 14:16:45 UTC
Reassigning to platform/JDK so that a JDK issue is filed for this problem. Thanks.
Comment 3 Miloslav Metelka 2013-12-12 16:20:26 UTC
Btw the DocumentListener problem is introduced by fix of JDK8 https://bugs.openjdk.java.net/browse/JDK-7146146.
However the potential deadlock of Document<->UndoManager demonstrated in 7146146 exists in the JDK for years and the client can easily overcome it like we do in NetBeans in the following way:

class MyDocument extends AbstractDocument {

  void runAtomic(Runnable r) {
    writeLock();
    try {
      r.run();
    } finally {
      writeUnlock();
    }
  }

}

class MyUndoManager extends UndoManager {

  private final MyDocument doc; // Assigned in constructor

  @Override
  public void undo() {
    runAtomic(new Runnable() {
      public void run() {
        UndoManager.super.undo();
      }
    });
  }

  @Override
  public void redo() {
    runAtomic(new Runnable() {
      public void run() {
        UndoManager.super.redo();
      }
    });
  }

  etc.

}


However the consequences of the current fix of 7146146 are IMHO much more serious and the fix makes the DocumentListener implementations almost useless. What's DocumentEvent.getOffset() and DocumentEvent.getLength() methods good for when the data can possibly no longer be there when DocumentListener is called?? Such semantic change would have to go hand in hand with e.g. expressing the boundaries of the change as Position objects and providing inserted/removed text in the event etc.
So IMHO the current fix of 7146146 should be rolled back.
Comment 4 Quality Engineering 2013-12-13 02:50:25 UTC
Integrated into 'main-silver', will be available in build *201312130002* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)

Changeset: http://hg.netbeans.org/main-silver/rev/78b9ff11260b
User: Miloslav Metelka <mmetelka@netbeans.org>
Log: Added a test for tracking of issue #239364.
Comment 5 Antonin Nebuzelsky 2013-12-13 16:01:14 UTC
https://bugs.openjdk.java.net/browse/JDK-8030118
Comment 6 Jiri Skrivanek 2014-01-06 08:43:49 UTC
Fixed in JDK8-build122.