<AWT Dev> <Awt Dev> [9] Review Request for 8132664: closed/javax/swing/DataTransfer/DefaultNoDrop/DefaultNoDrop.java locks on Windows

Tue Sep 8 09:33:22 UTC 2015

On 08.09.15 12:04, Semyon Sadetsky wrote:
>
>
> On 9/7/2015 6:56 PM, Sergey Bylokhov wrote:
>> On 07.09.15 16:41, Semyon Sadetsky wrote:
>>>
>>>
>>> On 9/7/2015 3:28 PM, Sergey Bylokhov wrote:
>>>> On 03.09.15 17:43, Semyon Sadetsky wrote:
>>>>>
>>>>>
>>>>> On 8/5/2015 2:33 PM, Sergey Bylokhov wrote:
>>>>>> On 05.08.15 14:20, Semyon Sadetsky wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 8/5/2015 1:39 PM, Sergey Bylokhov wrote:
>>>>>>>> On 05.08.15 13:18, Semyon Sadetsky wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 8/5/2015 12:27 PM, Sergey Bylokhov wrote:
>>>>>>>>>> On 04.08.15 14:54, Semyon Sadetsky wrote:
>>>>>>>>>>> On 8/3/2015 6:05 PM, Sergey Bylokhov wrote:
>>>>>>>>>>>> Hi, Semyon
>>>>>>>>>>>> Did you try to change dwMilliseconds from INFINITE to the
>>>>>>>>>>>> timeout(10 seconds by default?) which is passed to the method?
>>>>>>>>>>>> It does not help? Because even when dnd is not used we should
>>>>>>>>>>>> not wait event for infinite time.
>>>>>>>>>>> It would not help to fix the issue because 10 seconds is too big
>>>>>>>>>>> interval. But for consistency it is not bad to have a time
>>>>>>>>>>> limit.
>>>>>>>>>>> http://cr.openjdk.java.net/~ssadetsky/8132664/webrev.01/
>>>>>>>>>> Note that syncNativeQueue is intended to wait until the native
>>>>>>>>>> event queue is flushed or until timeout will expire. So even if
>>>>>>>>>> timeout is expired we collect the native events during this
>>>>>>>>>> period
>>>>>>>>>> of time.Can you double check that the event counter is
>>>>>>>>>> incremented
>>>>>>>>>> during dnd? I do not know how we block the toolkit thread,
>>>>>>>>>> probably we create some nested loops which ignore our event
>>>>>>>>>> posted
>>>>>>>>>> from syncNativeQueue, can we change that?
>>>>>>>>> Yes, this is an internal secondary loop which waits for mouse
>>>>>>>>> release event.
>>>>>>>> Can we change the condition and process the sync event in this
>>>>>>>> loop?
>>>>>>> Why? Will receive all events on the toolkit thread when doDragDrop
>>>>>>> returns.
>>>>>>
>>>>>> When how we get dragenter/exit events?
>>>>>
>>>>> On the platform side they are not events but callbacks which are
>>>>> converted into events on the java side and added to the queue. So they
>>>>> will be detected by isEQEmpty() and waitForIdle() will be repeated.
>>>>
>>>> This is not directly related to this fix. Most of our callbacks/events
>>>> posts events to the EQ, but there is a possibility for a lag between
>>>> callback and a post events, and this is why the syncNativeQueue was
>>>> added.
>>>> Let's return to the beginning: the syncNativeQueue method according to
>>>> its specifications should try to flush the native system, track the
>>>> native activity and should not wait more than timeout parameter.
>>>
>>> It is not possible to send sync events to the DD loop. Only
>>> mouse/keyboard event can return control to the app. I would not emulate
>>> them in the syncnative.
>>> Always waiting the max time is not good. Maybe wait a bit and check if
>>> no callbacks happened?
>>
>> It will be good to check that callbacks were occurred in the same way
>> as native events. This can be done in this fix. As for question about
>> wait or not when timeout is over, is a shared problem, which affect
>> the osx also. It can be fixed separately. I guess the logic in
>> realSync can be tweaked and the default timeout(10*4 sec) is not a
>> optimal as a default value, 2 or 3 seconds without any events should
>> be enough.
> Could you explain your logic?

My logic is to follow the specification.

> Why to wait short wait period doesn't work
> if we are in DD and there are no callbacks?

"No callbacks" during what period? My point: this period should be 
passed to this method.

> No callbacks means no
> activity and in analogous situations the method returns true.

> I see a clear disadvantage: even with 2-3 second wait period the test
> will executes much longer than on Linux platform because D&D is
> triggered every 3 mouse drags . Also other D&D tests suites will demand
> much more time to pass the windows platform. Users can report bugs if
> AWT Robot starts to produce multi-seconds delay after drag events.
> Maybe you can provide an example scenario which demonstrates that it is
> really necessary to pay this price?
>
>>
>>>
>>>>>
>>>>>>
>>>>>>>>> Event counter is not changed during toolkit thread blocking of
>>>>>>>>> cause. Not sure that we can change that. But since toolkit
>>>>>>>>> queue is
>>>>>>>>> blocked we can assume that we are synced.
>>>>>>>>>>
>>>>>>>>>> The timeout value is maintained on the shared level and actually
>>>>>>>>>> this test will fail with timeout on osx as well JDK-7185258. The
>>>>>>>>>> test will fail even if the time out will be changed to ±100ms,
>>>>>>>>>> because it call realsync on each pixel move, ±200 times. This
>>>>>>>>>> problem can be fixed in different ways like tweak of timeouts and
>>>>>>>>>> numbers of iterations, or changing the test to call w4idle only
>>>>>>>>>> after the latest move(actually I think this method can be
>>>>>>>>>> moved to
>>>>>>>>>> the robot class).
>>>>>>>>>>
>>>>>>>>>> So I still think that the right fix for a deadlock, which is
>>>>>>>>>> subject of this bug, is simply change the syncNativeQueue and
>>>>>>>>>> waits using a timeout if it is positive, and waits forever if
>>>>>>>>>> timeout is negative (the same bug on osx JDK-8080504).
>>>>>>>>> I'm not sure that waiting brings any value. What do you propose to
>>>>>>>>> return if it timed out? The event counter will not be changed
>>>>>>>>> regardless of waiting.
>>>>>>>> But it should be changed, because we get native events from the
>>>>>>>> system during dnd and in each such callback we should update this
>>>>>>>> counter. If callbacks were not called=>counter was not updated then
>>>>>>>> sync assume that currently we do not process events. If callbacks
>>>>>>>> were called then sync assume that we have events in the native
>>>>>>>> queue
>>>>>>>> and should try to sync again on the next iteration.
>>>>>>> No. Events are not processed while toolkit is blocked in
>>>>>>> doDragDrop(). The application state is frozen on that period.
>>>>>>> That is
>>>>>>> Windows approach.
>>>>>>>>
>>>>>>>>> With such waiting the test will fail because of either jtreg
>>>>>>>>> timeout
>>>>>>>> Default timeout is 120 seconds for everything, the test try to sync
>>>>>>>> the queue 200 of times after each move, so yes it can fail with
>>>>>>>> timeout even if spend in nativesync 200 ms, the possible solutions
>>>>>>>> were in my previous email.
>>>>>>>>> either InfiniteLoop exception.
>>>>>>>> This exception will be disabled by default lately in jdk9
>>>>>>>> timeframe,
>>>>>>>> right now it helps to find some related issues.
>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> On 03.08.15 17:26, Semyon Sadetsky wrote:
>>>>>>>>>>>>> Hello,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Please review fix for JDK9:
>>>>>>>>>>>>>
>>>>>>>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8132664
>>>>>>>>>>>>> webrev:
>>>>>>>>>>>>> http://cr.openjdk.java.net/~ssadetsky/8132664/webrev.00/
>>>>>>>>>>>>>
>>>>>>>>>>>>> DoDragDrop() is blocking, so upon drag operation is triggered
>>>>>>>>>>>>> the toolkit thread is blocked and the WM_AWT_WAIT cannot be
>>>>>>>>>>>>> processed which in its turn blocks the AWT robot.
>>>>>>>>>>>>> The solution is to escape AWT robot waiting in
>>>>>>>>>>>>> syncNativeQueue() if drag operation is in progress.
>>>>>>>>>>>>>
>>>>>>>>>>>>> --Semyon
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>>
>

-- 
Best regards, Sergey.