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.
This is probably cause by asynch impl of UndoRedo manager. We should make it synchronous at least from AWT thread!? testDocumentCannotBeModified fail For a while the test really starts with Kuk expected:<Kuk> but was:<Aho>
Created attachment 14407 [details] The failure
The problem is caused by the asynchronous communication between UndoRedo.Manager and CloneableEditorSupport. The asynchronous code has been introduced due to bug 8692, which I believe has been made obsolete by the sharing of AbstractDocument.lock() for most of the document operations. That is why I investigated a removal of the asynchronous code. It seems that the deadlock is not there anymore and moreover the code becomes more reliable. I'll attach my patch and wait for Petr's and Mila's review.
Created attachment 16443 [details] Making UndoRedo.Manager synchronous
I am about to commit this change and make UndoRedo.Manager synchronous again.
Checking in src/org/openide/awt/UndoRedo.java; /cvs/openide/src/org/openide/awt/UndoRedo.java,v <-- UndoRedo.java new revision: 1.25; previous revision: 1.24 done Processing log script arguments... More commits to come... Checking in src/org/openide/text/CloneableEditorSupport.java; /cvs/openide/src/org/openide/text/CloneableEditorSupport.java,v <-- CloneableEditorSupport.java new revision: 1.125; previous revision: 1.124 done Processing log script arguments... More commits to come... Checking in test/unit/src/org/openide/text/CloneableEditorSupportTest.java; /cvs/openide/test/unit/src/org/openide/text/CloneableEditorSupportTest.java,v <-- CloneableEditorSupportTest.java new revision: 1.6; previous revision: 1.5 done RCS file: /cvs/openide/test/unit/src/org/openide/text/UndoRedoCooperationTest.java,v done Checking in test/unit/src/org/openide/text/UndoRedoCooperationTest.java; /cvs/openide/test/unit/src/org/openide/text/UndoRedoCooperationTest.java,v <-- UndoRedoCooperationTest.java initial revision: 1.1
I have looked again to Swing document implementations and UndoManager and I have found a particular limitation for certain circumstances. As there are two locks - one is the UndoManager and the other one is the document's lock we need to avoid counterlocking. That may occur when Thread-1: 1. Document modification insert/remove 2. Firing undo listeners under document's lock incl. UndoManager 3. UndoManager.addEdit() -> UM's lock Thread-2: 1. UndoManager.undo() -> UM's lock 2. AbstractDocument.DefaultDocumentEvent.undo() -> doc's lock So to avoid the counterlocking we need to make both the threads above to be just a single thread - likely the AWT thread. As Jesse indicated earlier during his work on the IDE's thread model he would like to have all the document modifications/operations to be performed in AWT. I guess the UM's operations are done in AWT naturally as they are invoked by shortcuts/toolbar-icons. However I guess the current policy does not require both doc and UM to be run in AWT does it?
Nothing in UndoRedo.Manager is final, so we might completely reimplement it by ourselves if needed and force it to use Document's lock, I guess. However as stated at http://www.netbeans.org/download/dev/javadoc/OpenAPIs/org/openide/doc-files/threading.html WindowsAPI (including top component and getUndoRedo()) is supposed to be called just from AWT. However I am going to try to simulate your described deadlock and check whether there is something to do about it (I expect overriding undo and getting lock first before calling super might be enough).
cvs -q ci -m "#42005: Simulating and preventing the deadlock described by mmetelka@netbens.org in his comment in issue 42005" Checking in src/org/openide/text/CloneableEditorSupport.java; /cvs/openide/src/org/openide/text/CloneableEditorSupport.java,v <-- CloneableEditorSupport.java new revision: 1.126; previous revision: 1.125 done Processing log script arguments... More commits to come... RCS file: /cvs/openide/test/unit/src/org/openide/text/NbLikeEditorKit.java,v done Checking in test/unit/src/org/openide/text/NbLikeEditorKit.java; /cvs/openide/test/unit/src/org/openide/text/NbLikeEditorKit.java,v <-- NbLikeEditorKit.java initial revision: 1.1 done Checking in test/unit/src/org/openide/text/UndoRedoCooperationTest.java; /cvs/openide/test/unit/src/org/openide/text/UndoRedoCooperationTest.java,v <-- UndoRedoCooperationTest.java new revision: 1.2;
Regarding the case when the document impl is not instance of NbDocument.WriteLockable (e.g. when being run without the editor module being turned on) I think that we cannot use doc.render() because this would require promotion of document read lock to write lock which is not allowed: 1) doc.render() acquires document read lock 2) UM.undo() acquires UM lock 3) AbstractDocument.DefaultDocumentEvent.undo() calls writeLock() which should cause the thread to sleep and never wake up.
Thank you Mila, I've forget the render there from my previous experiments. Just run is better, even not perfect: cvs ci -m "#42005: Yet another change - cannot upgrade readlock to writelock on AbstractDocument. That is why we can do no tricks on non-NbDocument.WriteLockable documents. Maybe reflection. But until real fix is needed, we can just use no locks." /cvs/openide/src/org/openide/text/CloneableEditorSupport.java,v <-- CloneableEditorSupport.java new revision: 1.127; previous revision: 1.126 done Processing log script arguments... More commits to come... Checking in test/unit/src/org/openide/text/UndoRedoCooperationTest.java; /cvs/openide/test/unit/src/org/openide/text/UndoRedoCooperationTest.java,v <-- UndoRedoCooperationTest.java new revision: 1.3; previous revision: 1.2
*** Issue 46411 has been marked as a duplicate of this issue. ***
*** Issue 38112 has been marked as a duplicate of this issue. ***
The test failed on : 200408221800 Host : qa-frodo Operating System Name : SunOS Operating System Version : 5.9 System Architecture : sparc Java Version : 1.4.2_05 Java Vendor : Sun Microsystems Inc. User Language : en
Created attachment 17025 [details] exception stacktrace
I hope that this is fixed as well as the other issues related to undo redo in CES.
Verified.