<AWT Dev> EventQueue changes

[Oleg Sukhodolsky]

On Tue, Sep 16, 2008 at 12:57 PM, Artem Ananiev <Artem.Ananiev at sun.com> wrote:
>
> Oleg Sukhodolsky wrote:
>>>
>>> 1. A single lock is introduced to handle all the EQ operations like push,
>>> pop, getNextEvent, etc. instead of locking all EQ objects one by one, if
>>> several.
>>
>> this change is backward incompatible :(  Why do you think that this is
>> the only way to fix synchronization problems.
>> Could you describe this problems with more details?
>
> This change is formally backwards compatible, as there is no single word
> about EQ synchronization in specification :) I clearly understand some
> libraries may be broken - see http://foxtrot.sourceforge.net/ project for
> example - however any code that relies on JDK internals is written by
> developers on their own risk.

I do not want to argue on this topic, but we both know that the
changes may break
some apps, and this is the main problem.

> A couple of words about the code. As described in 4913324, the order of
> locks may be different when using 3+ event queues which lead to deadlocks.

as far as I can see from evaluation of the bug the problem is caused
by the fact that
internal methods of EQ use "this" for synchronization.

> To resolve this problem we either need: replace many locks with a single
> one, or make all the machinery single-threaded, or always specify the order
> of locks. The second approach is hardly implementable, because AWT is
> multi-threaded. The third approach is also difficult, in particular, because
> of event queue is accessed from at least two packages: java.awt and sun.awt
> - while all the information about EQ stack is package private in java.awt

I think we should try to reconsider synchronization strategy for
internal methods,
but try to keep synchronization we have in public methods unchanged.
(I'm not sure
that this is possible,but we (you ;) should try)

> Another possible solution I haven't thought about yet is to try to eliminate
> SunToolkit$PostEventQueue usage at all. This would resolve some (but
> probably, not all) problems, but it may have some impact on security and
> performance at the same time.
>
> My general argumentation is following. Some basic usages of EQ API must not
> be broken by suggested changes. Complex libraries/applications may indeed be
> broken, however I'm not sure if they even exist - there are too many
> problems with the current implementation.

I remember similar reasoning for focus api which developed for 1.4,
and the fact is
that apps/libs which will be broken by such changes may cause VERY BIG
problems (just imagin what if some VERY BIG customer will be affected, what kind
of flame will generated about this)

Oleg.
>
> Thanks,
>
> Artem
>
>> Thanks in advance, Oleg.
>>
>> On Thu, Sep 11, 2008 at 12:10 PM, Artem Ananiev <Artem.Ananiev at sun.com>
>> wrote:
>>>
>>> Hi, Oleg,
>>>
>>> Oleg Sukhodolsky wrote:
>>>>
>>>> On Tue, Sep 9, 2008 at 3:55 PM, Artem Ananiev <Artem.Ananiev at sun.com>
>>>> wrote:
>>>>>
>>>>> Hi, AWT team,
>>>>>
>>>>> there are several issues related to EventQueue class with a long
>>>>> history.
>>>>> The number of user votes constantly grows, so I think it would be fine
>>>>> if
>>>>> we
>>>>> can get them fixed in some nearest future.
>>>>>
>>>>> The list of bugs include (but not limited to):
>>>>>
>>>>> 6424157: java.awt.EventQueue push/pop might cause threading issues
>>>>>
>>>>> 6542185: Threading issues with java.awt.EventQueue.push/pop
>>>>> (closed as not a defect, but some of described problems still exist)
>>>>>
>>>>> 4913324: Deadlock when using two event queues.
>>>>>
>>>>> 4516924: Request public access to pumpEvents(Conditional) type
>>>>> functionality.
>>>>>
>>>>> 6727884: Some Uncaught Exceptions are no longer getting sent to the
>>>>> Uncaught
>>>>> Exception Handlers
>>>>>
>>>>> Some of the described problems don't look related to each other,
>>>>> however
>>>>> after a closer look I found they really do. That's why I listed them
>>>>> here
>>>>> altogether, and would like to discuss some possible improvements:
>>>>>
>>>>> 1. Synchronization changes. Most of the problems with push/pop are
>>>>> caused
>>>>> by
>>>>> imperfect synchronization in EventQueue. Currently, all the actions
>>>>> like
>>>>> postEvent() or getNextEvent() are transferred back and forth in the
>>>>> stack
>>>>> of
>>>>> event queues, and each queue is accessed in its 'synchronized' block.
>>>>> Instead, a single lock looks more correct here.
>>>>>
>>>>> 2. EventDispatchThread lifecycle. It is a known fact, that event
>>>>> dispatch
>>>>> thread may die for some reason (for example, because of unhandled
>>>>> exception). When a new event comes, new EDT is created. Another case
>>>>> when
>>>>> EDT is switched is push/pop methods: when a new EQ is pushed/popped, a
>>>>> new
>>>>> EDT is created.
>>>>>
>>>>> I'm sure these changes of current dispatch thread is not what
>>>>> developers
>>>>> expect. Swing is considered as single-threaded toolkit, but it is
>>>>> really
>>>>> not...
>>>>>
>>>>> 3. Controllable event pump. This is what developers have been
>>>>> requesting
>>>>> for
>>>>> at least 8 years. With the current API this task cannot be solved, and
>>>>> all
>>>>> the external libs like Foxtrot are really just hacks and depend on JDK
>>>>> internals.
>>>>>
>>>>> From technical point of view, controllable event pump is just a several
>>>>> lines of code changes: we only need to make public the code which is
>>>>> used
>>>>> for modality event pumps.
>>>>>
>>>>> ----
>>>>>
>>>>> I have a prototype fix with all the three items implemented. Still, it
>>>>> would
>>>>> be fine to hear what all AWT developers think about proposed changes.
>>>>
>>>> I see list of problems, but do not see list of proposed changes :(
>>>> Did I miss something?
>>>
>>> You're right, the changes are not included, my fault... Here they are:
>>>
>>> 1. A single lock is introduced to handle all the EQ operations like push,
>>> pop, getNextEvent, etc. instead of locking all EQ objects one by one, if
>>> several.
>>>
>>> 2. EventDispatchThread is reused as much as possible. When a new EQ is
>>> pushed, it uses the old EDT instead of creating a new one. The same is
>>> true
>>> for pop().
>>>
>>> 3. pumpEvents() API is made public, probably with some minor changes.
>>>
>>> Thanks,
>>>
>>> Artem
>>>
>>>> With best regards, Oleg.
>