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

Semyon Sadetsky semyon.sadetsky at oracle.com
Tue Sep 8 08:28:10 UTC 2015



On 9/7/2015 5:56 PM, Alexander Scherbatiy wrote:
> On 9/7/2015 5:08 PM, Semyon Sadetsky wrote:
>>
>>
>> On 9/7/2015 2:41 PM, Alexander Scherbatiy wrote:
>>> On 9/4/2015 9:00 PM, Semyon Sadetsky wrote:
>>>>
>>>>
>>>> On 9/4/2015 6:11 PM, Alexander Scherbatiy wrote:
>>>>> On 9/3/2015 10:01 PM, Semyon Sadetsky wrote:
>>>>>>
>>>>>>
>>>>>> On 8/5/2015 4:50 PM, Semyon Sadetsky wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 8/5/2015 2:39 PM, Alexander Scherbatiy wrote:
>>>>>>>> On 8/5/2015 2:00 PM, Semyon Sadetsky wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 8/5/2015 1:04 PM, Alexander Scherbatiy wrote:
>>>>>>>>>> On 8/4/2015 8:13 PM, Semyon Sadetsky wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 8/4/2015 6:17 PM, Alexander Scherbatiy wrote:
>>>>>>>>>>>> On 8/4/2015 3:13 PM, Semyon Sadetsky wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 8/4/2015 2:03 PM, Alexander Scherbatiy wrote:
>>>>>>>>>>>>>> On 8/4/2015 12:32 PM, Semyon Sadetsky wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On 8/4/2015 11:47 AM, Alexander Scherbatiy wrote:
>>>>>>>>>>>>>>>> On 8/3/2015 4:19 PM, Semyon Sadetsky wrote:
>>>>>>>>>>>>>>>>> On 8/3/2015 3:12 PM, Alexander Scherbatiy wrote:
>>>>>>>>>>>>>>>>>> On 7/31/2015 9:44 AM, Semyon Sadetsky wrote:
>>>>>>>>>>>>>>>>>>> Good question. But I did not add a concrete class.
>>>>>>>>>>>>>>>>>>> The problem is that UndoManager provided by JDK 
>>>>>>>>>>>>>>>>>>> wants to be serialized but undoable objects knows 
>>>>>>>>>>>>>>>>>>> nothing about that. The contract between UndoManager 
>>>>>>>>>>>>>>>>>>> and undoable is UndoableEditListener which only 
>>>>>>>>>>>>>>>>>>> notifies UndoManager to add a new edit. 
>>>>>>>>>>>>>>>>>>> AbstractDocument does not care about the specific 
>>>>>>>>>>>>>>>>>>> UndoManager implementation and it can contain plenty 
>>>>>>>>>>>>>>>>>>> different UndoableEditListener. That is the current 
>>>>>>>>>>>>>>>>>>> API approach.
>>>>>>>>>>>>>>>>>>> If our specific UndoManager wants to be serialized 
>>>>>>>>>>>>>>>>>>> it should also take into account that the undoable 
>>>>>>>>>>>>>>>>>>> it controls may require serialization. For that it 
>>>>>>>>>>>>>>>>>>> needs undoable's synchronization monitor and 
>>>>>>>>>>>>>>>>>>> AbstractDocument can provide it using 
>>>>>>>>>>>>>>>>>>> writeLock()/writeUnlock() methods. I assumed that in 
>>>>>>>>>>>>>>>>>>> the first turn UndoManger should work well with JDK 
>>>>>>>>>>>>>>>>>>> undoables than to serve as a general implementation. 
>>>>>>>>>>>>>>>>>>> Also I tried to preserve  the current API.
>>>>>>>>>>>>>>>>>>> And your suggestion is to change the existing 
>>>>>>>>>>>>>>>>>>> UndoableEditListener API by introducing 
>>>>>>>>>>>>>>>>>>> synchronization methods in it. Am I correctly 
>>>>>>>>>>>>>>>>>>> understand you?
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>    What I said is that UndoManager can be used not 
>>>>>>>>>>>>>>>>>> only by AbstractDocument but also in other classes 
>>>>>>>>>>>>>>>>>> which can have the same synchronization problems.
>>>>>>>>>>>>>>>>>>    There should be a way to solve these problems 
>>>>>>>>>>>>>>>>>> without storing links of external classes inside the 
>>>>>>>>>>>>>>>>>> UndoManager.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> As well as AbstractDocument can use another undo 
>>>>>>>>>>>>>>>>> managers. It can be addressed to both parties. They 
>>>>>>>>>>>>>>>>> need each others locks to serialize changes without 
>>>>>>>>>>>>>>>>> deadlock.
>>>>>>>>>>>>>>>>> AbstarctDocument is related to UndoableEditListener as 
>>>>>>>>>>>>>>>>> one to many that means a lock should be taken for each 
>>>>>>>>>>>>>>>>> undo manager before the document change.
>>>>>>>>>>>>>>>>> Undo manager does not have any methods to get its lock 
>>>>>>>>>>>>>>>>> because it is an UndoableEditListener implementation. 
>>>>>>>>>>>>>>>>> AbstarctDocument has API to receive its lock.
>>>>>>>>>>>>>>>>> Do you still propose to fix the issue on 
>>>>>>>>>>>>>>>>> AbstractDocument side? 
>>>>>>>>>>>>>>>>    Yes.
>>>>>>>>>>>>>>>>> Could you clarify how do you see such fix?
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>     Put an UndoableEdit/UndoableEditEvent/necessary 
>>>>>>>>>>>>>>>> information to a queue instead of firing the undoable 
>>>>>>>>>>>>>>>> edit event under the write lock. Do not read the queue 
>>>>>>>>>>>>>>>> under the write lock. The queue allows to preserve the 
>>>>>>>>>>>>>>>> order of UndoableEdit's adding to an UndoManager.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Is not it the same as the previous attempt to fix this 
>>>>>>>>>>>>>>> issue (see 8030118 )?
>>>>>>>>>>>>>>      8030118 fix does a strange thing like firing 
>>>>>>>>>>>>>> InsertUpdate document event out of the lock. Do not do that.
>>>>>>>>>>>>>>> Document change event need to be fired under write lock 
>>>>>>>>>>>>>>> because the change to the document should be atomic. 
>>>>>>>>>>>>>>> Queue of changes is undo manager's responsibility not 
>>>>>>>>>>>>>>> the document.
>>>>>>>>>>>>>>> And such queue in the AbstractDocument would require 
>>>>>>>>>>>>>>> complex coordination with all its undo managers queues. 
>>>>>>>>>>>>>>> What if undo called on undo manager during the doc's 
>>>>>>>>>>>>>>> queue processing? The right undo/redo requests and edit 
>>>>>>>>>>>>>>> events order need to be preserved in this case and it 
>>>>>>>>>>>>>>> would be too complex or we would have to change the 
>>>>>>>>>>>>>>> concept and make AbstractDocument to maintain its 
>>>>>>>>>>>>>>> undo/redo history internally instead of external undo 
>>>>>>>>>>>>>>> managers.
>>>>>>>>>>>>>>     It only needs to pass undoable edits in the right 
>>>>>>>>>>>>>> order from abstract document to the UndoManager.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Consider the scenario: UndoManager executes undo/redo 
>>>>>>>>>>>>> before it receives the undoable edits. As result it will 
>>>>>>>>>>>>> undo not the last edit but intermediate and it will crash 
>>>>>>>>>>>>> because the document state is changed and intermediate the 
>>>>>>>>>>>>> undoable edit cannot be applied to the final document state.
>>>>>>>>>>>>
>>>>>>>>>>>>     This is a good point. But this does not work neither 
>>>>>>>>>>>> with the current behavior nor with your proposed fix.
>>>>>>>>>>>>     Consider the following scenario:
>>>>>>>>>>>>     -----------------------
>>>>>>>>>>>>     document.insertString("AAA")   // "AAA" UndoableEdit is 
>>>>>>>>>>>> added to the UndoManager
>>>>>>>>>>>>     document.insertString("BBB")
>>>>>>>>>>>>           writeLock();
>>>>>>>>>>>>           handleInsertString();
>>>>>>>>>>>>           // a user press undo, the "AAA" UndoableEdit is 
>>>>>>>>>>>> selected in UndoManager but not executed, because of the 
>>>>>>>>>>>> write lock
>>>>>>>>>>>>           fireUndoableEditUpdate("BBB")  // UndoManager is 
>>>>>>>>>>>> waiting for the "AAA" UndoableEdit execution
>>>>>>>>>>>>           writeUnlock()  // "AAA" UndoableEdit is executed 
>>>>>>>>>>>> instead of "BBB"
>>>>>>>>>>>>                                   //  "BBB" UndoableEdit is 
>>>>>>>>>>>> added to the UndoManager
>>>>>>>>>>>>    -----------------------
>>>>>>>>>>> It will work after the fix. When undo() method is called it 
>>>>>>>>>>> will be blocked on the document lock until the edit is done 
>>>>>>>>>>> in another thread. Then undo() will acquire the document 
>>>>>>>>>>> lock and call editToBeUndone() method which will return the 
>>>>>>>>>>> actual last edit added with addEdit() during the undoable 
>>>>>>>>>>> callback execution.
>>>>>>>>>>
>>>>>>>>>>    Is it possible to use the same undo manager with several 
>>>>>>>>>> abstract documents? If so, how are you going to map a 
>>>>>>>>>> document lock with the document undoable edit without 
>>>>>>>>>> querying it?
>>>>>>>>> That scenario is possible. As well as several undo managers 
>>>>>>>>> can be assigned to the same document. I think I can improve 
>>>>>>>>> the fix in that direction when you agree with the general 
>>>>>>>>> approach.
>>>>>>>>      It is interesting how it is possible to do that without 
>>>>>>>> querying an undoable edit. Your fix is relaying that an 
>>>>>>>> abstract document lock should precede the undo manager lock but 
>>>>>>>> to get the right abstract manager lock you need to take an 
>>>>>>>> undoable edit under the undo manager lock first.
>>>>>>> We always have only two possible directions forward and backward 
>>>>>>> so it will require only 2 references.
>>>>>>
>>>>>> Hi Alexander,
>>>>>>
>>>>>> Please, take a look on the updated version which works for any 
>>>>>> number documents shared one undo manager.
>>>>>> Also I removed the reference you did not like. This has some 
>>>>>> disadvantages but I think they are negligible.
>>>>>> http://cr.openjdk.java.net/~ssadetsky/8030702/webrev.01/
>>>>>
>>>>>    You code looks like:
>>>>> ----------------------
>>>>> public void undo() throws CannotUndoException {
>>>>>   synchronized (this) {
>>>>>     lockedDoc = getLockedDocument(edit);
>>>>>   }
>>>>>
>>>>>   // TP: 1
>>>>>
>>>>>   while (!done) {
>>>>>     lockedDoc.writeLock();
>>>>>     // ...
>>>>>     lockedDoc.writeUnlock();
>>>>>   }
>>>>> }
>>>>> ----------------------
>>>>>
>>>>>   Is it possible that on the line "TP: 1" a new UndoableEdit will 
>>>>> be added to the UndoManager so the the lockedDoc will not point to 
>>>>> the latest UndoableEdit which is taken on the line 438.
>>>> No. It is not possible because of
>>>> 438                     UndoableEdit edit = editToBeUndone();
>>>> It always return the last significant edit so we preserve the 
>>>> consistency.
>>>
>>>     I see.
>>>
>>>    There is one more question about the undoOrRedo() method there 
>>> the synchronization is removed.
>>>    Lets look at the sequences of calls to an UndoManager:
>>>     addEdit(anEdit1), addEdit(anEdit2), undoOrRedo(), undoOrRedo().
>>>
>>>    The result for two undoOrRedo() calls should neutralize each 
>>> other (it is just undo and redo calls).
>>>
>>>   Is it possible that after the fix the the first undoOrRedo() from 
>>> one thread fails to do the redo and before starting to do the undo
>>>   the second undoOrRedo() call from another thread also fails to do 
>>> a redo action?
>>>   In this cases two undo actions could be called instead of undo and 
>>> redo.
>>
>> It does not make any sense to make atomic the convenience method 
>> undoOrRedo() because undo() and redo() are not atomic.
>
>      All UndoManager.undo()/redo()/undoOrRedo() have synchronization.
>
>      For the sample described above two undoOrRedo() calls always 
> invoke undo() at first and redo() at the second step.
>      After the fix it is possible that two undo() methods can be 
> called.  It means that there will be a regression in the undoOrRedo() 
> method behavior after the fix.
The spec of the method to not promise any deterministic behavior for two 
consequent calls.
>
>>
>>>
>>>>
>>>>>
>>>>>  I still think that updating the UndoableManager for one 
>>>>> particular AbstarctManager class can be made only after 
>>>>> investigation of other possibilities.
>>>>>
>>>>>    You could start with choosing behavior which you want to 
>>>>> achieve or to keep, like:
>>>>>    - fix the deadlock
>>>>>    - atomic  undo() method
>>>>>    - serialization
>>>>>    - immediate roll back action
>>>>>    - abstract document consistency after undo() action
>>>>>   - ...
>>>> We need to pay attention to the deadlock at first of cause.
>>>> Serialization and consistency are achieved. Any concrete doubts?
>>>> immediate roll back action -- ?what is that?
>>>      "if user starts a long edit operation and press undo after that 
>>> he expects when the long edit is finished it will be rolled back 
>>> immediately." - what ever does it mean.
>> Got it. It will work within the fairness. We have discussed this 
>> allready.
>>>> I sacrificed undo/redo call atomicity because you did not like doc 
>>>> references in undo manager.  I think it is not important for the 
>>>> most multithreaded undo/redo scenarios.
>>>    Could you give more details about it. Which doc references do you 
>>> mean?
>>
>> Your statement a dozen iterations ago was:  "There should be a way to 
>> solve these problems without storing links of external classes inside 
>> the UndoManager."
>> I guess you used "link" term for references. I would recommend to use 
>> standard terminology: reference to the object, dependency on the 
>> class, etc... to avoid misunderstanding.
>> Usually "link" is in a browser document or a tool that produces 
>> executables after compilation.
>>
>>>>
>>>>>
>>>>>  and look which of the following approaches can better solve them 
>>>>> (where the fist is more preferred and the last is less preferred 
>>>>> case):
>>>>>   - using UndoManager as is without adding links from some 
>>>>> specific classes to it
>>>>>   - provide an API for UndoManager to work with UndoableEdit-s 
>>>>> which have synchronization for undo/redo methods
>>>>>   - adding links of external classes directly to UndoManager
>>>>>
>>>> What do you mean under link term? A reference or dependency?
>>>     There are two options. If UndoManager is a class designed to be 
>>> only used with the AbstractDocument and placed in the 
>>> javax.swing.text package it definitly can have special code to 
>>> handle an abstract document instance in a special way.
>>>    If   UndoManager is a general purpose class, it looks strange 
>>> that it handles some special classes in different way as all others. 
>>> It usually mean that there are some design problems in this class. 
>>> That is why I just asked to look at other ways at first. Only if 
>>> other solutions are not suitable it has sense to look at the way 
>>> that you are provided.
>>>
>> Correct. I introduced extra dependency. It is optional, but anyway. 
>> Of cause there is a design problem in undo javax.swing.undo package. 
>> But I cannot rewrite the API because we will get a compatibility 
>> problem then. I mentioned this several times in this thread.
>>
>>>
>>>> We are constrained by compatibility requirements.  UndoManager is a 
>>>> broadly used class we cannot change the API so drastically.
>>>
>>>    I think that you can generalize your solution just adding an 
>>> internal interface like sun.swing.UndoableEditLock.
>>>    Every UndoableEdit which implements this interface can provide a 
>>> lock for its synchronization.
>>>
>>>    If this will work it can be made public in some days so other 
>>> application can also have proper synchronization for their undo/redo 
>>> actions.
>> OK. I added it.
>> http://cr.openjdk.java.net/~ssadetsky/8030702/webrev.02/
>
>    - We can return a public class that implements an internal 
> interface, but we can't expose an internal API  in the public class 
> definition.
>      May be it is possible to wrap an UndoableEdit to the 
> UndoableEditLockSupport in the UndoableEditEvent or in some other place.
>
>    - The similar code is used both in the UndoManager.undo() and 
> redo() methods. Is it possible to move this code to one method that 
> does undo or redo depending on the given argument?
>
OK. accepted.
http://cr.openjdk.java.net/~ssadetsky/8030702/webrev.03/

> Thanks,
>   Alexandr.
>
>>
>>>
>>>   Thanks,
>>>   Alexandr.
>>>
>>>> "adding links of external classes directly to UndoManager" -  
>>>> Sorry, did not catch what are you about? Could you clarify?
>>>>
>>>> --Semyon
>>>>
>>>>> Thanks,
>>>>>   Alexandr.
>>>>>
>>>>>>
>>>>>> --Semyon
>>>>>>
>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> There  is a mistake in your scenario steps: 
>>>>>>>>>>> fireUndoableEditUpdate() is called before the freeing the 
>>>>>>>>>>> lock (see AbstractDocument.handleInsertString() method).
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Yet another argument do not do this from the user 
>>>>>>>>>>>>>>> experience: if user starts a long edit operation and 
>>>>>>>>>>>>>>> press undo after that he expects when the long edit is 
>>>>>>>>>>>>>>> finished it will be rolled back immediately. 
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>     It is not true. The first process adds his undo edit 
>>>>>>>>>>>>>> to the UndoManager. While a user trying to press undo the 
>>>>>>>>>>>>>> second long process can be started.
>>>>>>>>>>>>>
>>>>>>>>>>>>> That is what led to this issue because when undo is in 
>>>>>>>>>>>>> progress document writing should be allowed.
>>>>>>>>>>>>> Sorry but I didn't see why is "It not true"? Then what is 
>>>>>>>>>>>>> your expectation when you press undo button while edit is 
>>>>>>>>>>>>> not finished yet and there is no way to abort it?
>>>>>>>>>>>>     It would be good if it works as you described. But it 
>>>>>>>>>>>> does not work in this way with or without your fix.
>>>>>>>>>>>>     undo() action has writeLock in AbstractDocument and 
>>>>>>>>>>>> because of it  is always executed after insert string action.
>>>>>>>>>>>>     If a user sees that undo is available, he can call it 
>>>>>>>>>>>> but the second long insertString process can start earlier 
>>>>>>>>>>>> and acquire the writeLock.
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> That is what we are going to fix. And this does work after 
>>>>>>>>>>> this fix. Undo call will be blocked by the long edit until 
>>>>>>>>>>> the last is done without any deadlocks. And when edit is 
>>>>>>>>>>> done undo() will acquire the lock and prevent any new edits 
>>>>>>>>>>> until undo() is done. Please provide a scenario when in your 
>>>>>>>>>>> opinion it does not wok.
>>>>>>>>>>      The first process starts for 5 minutes. When it is 
>>>>>>>>>> finished a user sees that he can press undo. While he is 
>>>>>>>>>> pressing undo button, the second long process starts for 10 
>>>>>>>>>> minutes and acquire the write lock. The user presses undo but 
>>>>>>>>>> he needs to wait 10 more minutes until the second process is 
>>>>>>>>>> finished.
>>>>>>>>> Actually, if two or more threads are waiting for a monitor it 
>>>>>>>>> is not determined which one will get the control after the 
>>>>>>>>> signal. To order that the ReentrantLock API could be used but 
>>>>>>>>> AbstractDocument uses wait/notify for locking. I think it is 
>>>>>>>>> not worth to dig so deep. It does not cause any issues 
>>>>>>>>     The issue that is considered is "if user starts a long edit 
>>>>>>>> operation and press undo after that he expects when the long 
>>>>>>>> edit is finished it will be rolled back immediately."
>>>>>>>>     If you are agree that it is not always possible to do the 
>>>>>>>> roll back "immediately" there is no point to discussion.
>>>>>>> I agree. On that level it is not possible to predict the order 
>>>>>>> exactly in such scenario. But the state of the document will be 
>>>>>>> consistent. And it is possible to have it predictable using lock 
>>>>>>> fairness.
>>>>>>>>
>>>>>>>>> because undo() always get the last edit anyway. If it will be 
>>>>>>>>> important for somebody to preserve the execution order on that 
>>>>>>>>> level of details we will fix it.
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>>> So undo should be executed after the edit is fully 
>>>>>>>>>>>>>>> performed because the corresponding UndoableEdit which 
>>>>>>>>>>>>>>> undos this edit can be produced only after the edit is 
>>>>>>>>>>>>>>> done.
>>>>>>>>>>>>>>> I think at first we need to look on the situation 
>>>>>>>>>>>>>>> externally rather than concentrate on implementation 
>>>>>>>>>>>>>>> questions like in which class do references go.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>    Yes, please look on this situation from a user point 
>>>>>>>>>>>>>> of view which wants to implement simple Java Painter.
>>>>>>>>>>>>>
>>>>>>>>>>>>> But could you describe this scenario? Just steps when this 
>>>>>>>>>>>>> simple Painter fails under the proposed fix?I
>>>>>>>>>>>>> Note, if this Painter's content is not an AbstarctDocument 
>>>>>>>>>>>>> it will work as before the fix.
>>>>>>>>>>>>   Any application that uses UndoManager and wants to have 
>>>>>>>>>>>> the same synchronization (have the same lock both for 
>>>>>>>>>>>> UndoableEdit adding and undo() method execution) will have 
>>>>>>>>>>>> the same deadlock problems.
>>>>>>>>>>>>    As I have already written:
>>>>>>>>>>>>    ---------------
>>>>>>>>>>>>    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?
>>>>>>>>>>>>   ---------------
>>>>>>>>>>>
>>>>>>>>>>> Still do not understand the steps for your Painter scenario. 
>>>>>>>>>>> A link (reference?) can be added if it is required to 
>>>>>>>>>>> implement functionality. If the content is not an 
>>>>>>>>>>> AbstarctDocument it may be required to implement custom 
>>>>>>>>>>> UndoManager to support the same behavior.
>>>>>>>>>>
>>>>>>>>>>     What is the difference between the AbstractDocument and 
>>>>>>>>>> other classes (in Swing or user defined)? Do you mean that 
>>>>>>>>>> the UndoManager is intended only to be used with 
>>>>>>>>>> AbstractDocument and it shouldn't be used in other cases 
>>>>>>>>>> where undo/redo actions are required for non text data?
>>>>>>>>> No, undo manager can be used with any classes. But since we 
>>>>>>>>> have it assigned to AbstarctDocument so often we need to do 
>>>>>>>>> our best to make undo manager working with it correctly 
>>>>>>>>> because users do not like deadlocks usualy. For other classes 
>>>>>>>>> we cannot provide synchronization by default because there is 
>>>>>>>>> no API to get the lock. So it remains up to user how to 
>>>>>>>>> provide the undo manager synchronization with the object it 
>>>>>>>>> controls for other classes
>>>>>>>>     What we should do just to understand that the same deadlock 
>>>>>>>> can happen in an user applications because he wants to use the 
>>>>>>>> same synchronization both for the data processing and for the 
>>>>>>>> undo action. If so, there should be two investigations:
>>>>>>>>   1. Is it possible to achieve the requested goals without 
>>>>>>>> changing UndoManager? In other words The UndoManager should be 
>>>>>>>> used in proper way as it is required by its design.
>>>>>>>>   2. Is it possible to update the UndoManager API to provide 
>>>>>>>> functionality that meets new requests?
>>>>>>> With API change it is reachable. But I would preserve the 
>>>>>>> current API as less constrained. If we add some methods for 
>>>>>>> locking we will determine the way how a user should synchronize 
>>>>>>> his undoable content.  And user may not need any synchronization 
>>>>>>> at all. We should keep in mind this opportunity as well.
>>>>>>>>
>>>>>>>>   Only after this discussion there can be a reason to look to 
>>>>>>>> other ways.
>>>>>>>>
>>>>>>>>   Thanks,
>>>>>>>>   Alexandr.
>>>>>>>>
>>>>>>>>> I think our undo manager implementation do not pretend to be 
>>>>>>>>> used as the global undo manager for big complex applications 
>>>>>>>>> and it cannot cover all possible undo approaches. But some 
>>>>>>>>> basic functionality can be provided and it should be usable. 
>>>>>>>>> Without edits serialization approach it is not usable for 
>>>>>>>>> multithreaded use. So either we do not pretend to provide a 
>>>>>>>>> multithreaded undo manager and remove all synchronize keywords 
>>>>>>>>> from UndoManager class, either we need to support 
>>>>>>>>> serialization approach which does not cause deadlocks.
>>>>>>>>>>
>>>>>>>>>>  Thanks,
>>>>>>>>>>  Alexandr.
>>>>>>>>>>
>>>>>>>>>>> I don't see a contradiction here, could you point on it more 
>>>>>>>>>>> precisely?
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>  Thanks,
>>>>>>>>>>>>  Alexandr.
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>    Alexandr.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>   Alexandr.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> --Semyon
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>   Alexandr.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> --Semyon
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> On 7/30/2015 5:27 PM, Alexander Scherbatiy wrote:
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>    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
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>




More information about the swing-dev mailing list