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

[Semyon Sadetsky]

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