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.
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.
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.
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
