<Swing Dev> [9] Review Request for 8030702: Deadlock between subclass of AbstractDocument and UndoManager
Alexander Scherbatiy
alexandr.scherbatiy at oracle.com
Thu Sep 10 15:46:34 UTC 2015
On 9/10/2015 6:26 PM, Semyon Sadetsky wrote:
>
>
> On 9/10/2015 5:13 PM, Alexander Scherbatiy wrote:
>> On 9/10/2015 5:07 PM, Semyon Sadetsky wrote:
>>>
>>>
>>> On 9/10/2015 3:45 PM, Alexander Scherbatiy wrote:
>>>> On 9/10/2015 3:39 PM, Semyon Sadetsky wrote:
>>>>>
>>>>>
>>>>> On 9/10/2015 2:44 PM, Alexander Scherbatiy wrote:
>>>>>> On 9/8/2015 6:11 PM, Semyon Sadetsky wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 9/8/2015 2:10 PM, Alexander Scherbatiy wrote:
>>>>>>>> On 9/8/2015 2:06 PM, Semyon Sadetsky wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 9/8/2015 1:07 PM, Alexander Scherbatiy wrote:
>>>>>>>>>> On 9/8/2015 12:48 PM, Semyon Sadetsky wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 9/8/2015 12:26 PM, Alexander Scherbatiy wrote:
>>>>>>>>>>>> On 9/8/2015 11:28 AM, Semyon Sadetsky wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> 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.
>>>>>>>>>>>>
>>>>>>>>>>>> javadoc for UndoManager.undoOrRedo() method [1]:
>>>>>>>>>>>> "Convenience method that invokes one of undo or redo. If
>>>>>>>>>>>> any edits have been undone (the index of the next edit is
>>>>>>>>>>>> less than the length of the edits list) this invokes redo,
>>>>>>>>>>>> otherwise it invokes undo."
>>>>>>>>>>>>
>>>>>>>>>>>> For the sequence of calls addEdit(anEdit1),
>>>>>>>>>>>> addEdit(anEdit2), undoOrRedo(), undoOrRedo():
>>>>>>>>>>>> Step 1: call undoOrRedo() - there are no undone edits, the
>>>>>>>>>>>> undo should be invoked.
>>>>>>>>>>>> Step 2: call undoOrRedo() - there is one undone edit, the
>>>>>>>>>>>> redo should be invoked.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> [1]
>>>>>>>>>>>> http://docs.oracle.com/javase/8/docs/api/javax/swing/undo/UndoManager.html#undoOrRedo--
>>>>>>>>>>>>
>>>>>>>>>>> I think it is obvious that this never worked if addEdit()
>>>>>>>>>>> and undoOrRedo() are called from different threads without
>>>>>>>>>>> the external synchronization.
>>>>>>>>>>> The fix changes nothing here. It works fine in a single
>>>>>>>>>>> thread approach.
>>>>>>>>>>
>>>>>>>>>> The javadoc does not mention that it is not thread safe
>>>>>>>>>> to use the undoOrRedo() method from different treads.
>>>>>>>>>> You can get all permutations of the "addEdit(anEdit1),
>>>>>>>>>> addEdit(anEdit2), undoOrRedo(), undoOrRedo()" calls and check
>>>>>>>>>> the the fix does not break cases which worked before the fix.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> A sequence of calls can be permuted in a single thread? How?
>>>>>>>>
>>>>>>>> Why are you asking about a single thread? The UndoManager
>>>>>>>> class is designed to work with multiple threads.
>>>>>>>>
>>>>>>>> You need to look at the all possible cases which can be
>>>>>>>> called by an application from different threads like:
>>>>>>>> - addEdit(anEdit1), addEdit(anEdit2), undoOrRedo(),
>>>>>>>> undoOrRedo()
>>>>>>>> - addEdit(anEdit1), undoOrRedo(), addEdit(anEdit2),
>>>>>>>> undoOrRedo()
>>>>>>>> - ...
>>>>>>>>
>>>>>>>> and check that there are no regressions in the behavior after
>>>>>>>> the fix.
>>>>>>> Are you kidding? This UndoManager sample has never worked in
>>>>>>> multithreading because of deadlock issue we are fixing.
>>>>>>> What the regression are you talking about?
>>>>>>
>>>>>> I am sorry. I do not understand your sense of humor.
>>>>>>
>>>>>> Do you mean that the deadlock happens every time when an
>>>>>> UndoEdiale is added to a UndoManager and undo() is called and
>>>>>> because of this the the UndoManager is totally unusable?
>>>>> In you suggestion you want to run addEdit(anEdit1),
>>>>> addEdit(anEdit2), undoOrRedo(), undoOrRedo() not in single thread
>>>>> but from different threads two times: before the fix and after it
>>>>> then to compare the results. But it is just impossible to get any
>>>>> valuable results from the run without the fix because it will
>>>>> crash with the deadlock. The deadlock we are trying to fix.
>>>>> Do you see why it is not possible or you need extra explanation?
>>>>
>>>> Even though something does not work before the fix the
>>>> UndoManager has been designed to work in the multi-tread
>>>> environment and the undoOrRedo() method should work in the
>>>> predicted way.
>>> The sample is related to singlethreaded usage of cause.
>>>>
>>>> If you think that it can't be fixed with this fix you can file a
>>>> new issue that it is a problem which we aware of.
>>> We discussed that already. The order is not deterministic without
>>> the external fair lock. And I think it is not needed for 99.(9)% of
>>> usages. Nobody cares if an undo button pressed 1 ms early or 1 ms
>>> later. It shouldn't be critical otherwise it is anti-pattern. If the
>>> background editing is allowed concurrently along the user
>>> interaction one need to use an external synchronization or simply
>>> disable undo/redo until the editing is done.
>>> For me it absolutely clear that it is a degenerated scenario when a
>>> UI document is modified concurrently from two different threads, but
>>> we should not allow deadlocks anyway and have to fix it.
>>>>
>>>> Thanks,
>>>> Alexandr.
>>>>
>>>>
>>>>>> Do you think that it is a good idea to remove a synchronization
>>>>>> from the undoOrRedo method because there is the deadlock?
>>>>>> If so, may be you can just remove the synchronization keyword
>>>>>> form the undo() and redo() methods as well and the deadlock just
>>>>>> goes away?
>>>>>> No new interfaces, no locks from an abstract document. That
>>>>>> would be the simplest solution of the issue.
>>
>> Could you answer on the question why do you suggest so complicated
>> fix? Just remove the synchronization from the undo() and redo()
>> methods and you really fix the deadlock.
>> It seems that you believe that it is possible because: "If the
>> background editing is allowed concurrently along the user interaction
>> one need to use an external synchronization or simply disable
>> undo/redo until the editing is done."
> Strange suggestion. We cannot remove synchronization it is required to
> guarantee consistency.
The same is for the undoOrRedo() method. You need to provide a
synchronization for it or create an new issue on it.
Thanks,
Alexandr.
> Also synchronization is needed to provide coherency with the state of
> the document. If those two are not provided the document can be
> corrupted. This is enough most usages.
>>
>> Thanks,
>> Alexandr.
>>
>>
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Alexandr.
>>>>>>
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Alexandr.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Alexandr.
>>>>>>>>> It is
>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> 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/
>>>>>>>>>>>>
>>>>>>>>>>>> - UndoManager.undo/redo methods
>>>>>>>>>>>> In your previous fix inProgress variable and the super
>>>>>>>>>>>> call were used under the lock. It may leads to some
>>>>>>>>>>>> synchronization issues if you decide to omit it.
>>>>>>>>>>>> - UndoManager.tryUndoOrRedo()
>>>>>>>>>>>> It is possible to get rid of 'done' variable just using
>>>>>>>>>>>> an infinity loop and return the exact result where the loop
>>>>>>>>>>>> is terminated.
>>>>>>>>>>>> - AbstractDocument.DefaultDocumentEventUndoableWrapper
>>>>>>>>>>>> implements both UndoableEdit and UndoableEditLockSupport
>>>>>>>>>>>> interfaces but UndoableEditLockSupport already extends
>>>>>>>>>>>> UndoableEdit.
>>>>>>>>>>>> - "@since 1.9" javadoc for
>>>>>>>>>>>> DefaultDocumentEventUndoableWrapper.lockEdit()/unlockEdit()
>>>>>>>>>>>> methods really belongs to the UndoableEditLockSupport methods.
>>>>>>>>>>>> In this case there is no need for {@inheritDoc} tag.
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>> Alexandr.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>> 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