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

[Semyon Sadetsky]

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?

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