<Swing Dev> [9] Review Request for 8030702: Deadlock between subclass of AbstractDocument and UndoManager

[Alexander Scherbatiy]

On 7/30/2015 3:41 PM, Semyon Sadetsky wrote:
> UndoManager stores this reference if it is assigned for an Abstract 
> document. I can wrap it with a WeakReference.
>   >>> It looks like AbstractDocument violates UndoManager 
> synchronization contract when it both use lock to work with 
> UndoManager and in the implemented undo() method.
> It is definitely about that. But what is your point?
>
> --Semyon
>
> On 7/30/2015 3:26 PM, Alexander Scherbatiy wrote:
>> On 7/28/2015 8:24 PM, Semyon Sadetsky wrote:
>>> Hello,
>>>
>>> Please review fix for JDK9:
>>> bug: https://bugs.openjdk.java.net/browse/JDK-8030702
>>> webrev: http://cr.openjdk.java.net/~ssadetsky/8030702/webrev.00/
>>>
>>> The deadlock happens if undo/redo is called on the document while 
>>> its content is being updated from another thread.
>>> The proposed solution is to reorder mutex acquisitions in the undo 
>>> manager assigned to an AbstractDocument. Document's lock is obtained 
>>> in its undo manager with a minimal change of the current API. A 
>>> stress test scenario is added to reproduce the issue.
>>
>>    UndoManager is a general class that can be used not only by 
>> AbstractDocument. It does not seem as good idea to store references 
>> from other classes that have deadlocks with UndoManager in the 
>> UndoManager.

    Consider someone writes Java Painter application where it is 
possible to draw lines and images and uses UndoManager for undo/redo 
actions.
    He might want that it was possible to work with copied images. He 
can get lock on ctrl+v action, process an image, prepare UndoableEdit 
and notify the UndoManager.
    He also can use lock/unlock in the undo action to have a consistent 
state with the processed image. If someone calls undo action during the 
image processing and gets a deadlock does it mean that link from Java 
Painter need to be added to the  UndoManager?

   Thanks,
   Alexandr.

>>    It looks like AbstractDocument violates UndoManager 
>> synchronization contract when it both use lock to work with 
>> UndoManager and in the implemented undo() method.
>>
>>   Thanks,
>>   Alexandr.
>>
>>>
>>> --Semyon
>>>
>>
>