<AWT Dev> [9] Review request for 8130136: Swing window sometimes fails to repaint partially when it becomes exposed

[Alexey Ivanov]

Hello Alex,

On 13.10.2015 19:54, Alexander Potochkin wrote:
> Hello Alexey
>
>> Hello Alex,
>>
>> Thank you for your review.
>>
>> Here's the updated webrev: 
>> http://cr.openjdk.java.net/~aivanov/8130136/jdk9/webrev.01/
>>
>> Please see my comments inline.
>>
>> On 12.10.2015 20:25, Alexander Potochkin wrote:
>>> Hello Alexey
>>>
>>> The fix looks reasonable.
>>> However the javadoc for RemoveDC method should be updated.
>>> Could you please describe the new logic there?
>> I updated the comment above DCList::RemoveDC to take into account the 
>> additional HWND parameter.
>>
>>> When does the GetHWnd() method return zero?
>>> Do we need to check it in RemoveDC?
>> GetHWnd() cannot return NULL in WndProc. No NULL-check is necessary.
>
> As far as I can see AwtComponent::GetHWnd can return zero.

Well, it could... but if it had done so, something bad would already 
have happened. The code in RemoveDC() won't cause an error if hWnd is 
NULL, however, an hDC could be leaked.

Anyway, if GetHWnd() returns NULL during message processing, AWT is 
already screwed up, it would not be able to work correctly.

Regards,
Alexey

>
> Thanks
> alexp
>
>>
>>>
>>> I guess you didn't add the test for a good reason,
>>> could you please comment on that?
>> It's impossible to write a regression test because the failure could 
>> be caught only from the native AWT code, no user code could detect 
>> it. Additionally, the time it takes to reproduce the issue is 
>> indefinite.
>>
>> Regards,
>> Alexey
>>>
>>> Thanks
>>> alexp
>>>
>>> On 10/12/2015 12:11, Alexey Ivanov wrote:
>>>> Hello,
>>>>
>>>> Please review the fix for jdk9:
>>>>     bug: https://bugs.openjdk.java.net/browse/JDK-8130136
>>>>     webrev: 
>>>> http://cr.openjdk.java.net/~aivanov/8130136/jdk9/webrev.00/
>>>>
>>>>
>>>> Problem description:
>>>> Sometimes a window fails to repaint when it becomes exposed.
>>>>
>>>> When repaint failure occurs, SetDIBitsToDevice() in 
>>>> GDIBlitLoops_nativeBlit() returns 0 with error 87 ("Parameter 
>>>> Error"). It happens because invalid, already released, hDC is 
>>>> passed to SetDIBitsToDevice().
>>>>
>>>>
>>>> Root cause:
>>>> When a window is disposed, the DCs associated with it are released. 
>>>> The handles can be reused for another window.
>>>>
>>>> A thread disposes the window and releases hDC. At this time, DC 
>>>> handle is reused for another window. When Disposer thread cleans up 
>>>> for a died thread, it tries to release the cached hDC. But that 
>>>> handle is already associated with another window. Thus a valid pair 
>>>> hWnd/hDC becomes invalid: painting to released DC fails.
>>>>
>>>>
>>>> The fix:
>>>> Save both hDC and hWnd in the thread local storage, and pair hDC 
>>>> with hWnd when removing it to avoid accidentally releasing hDC 
>>>> which is already reused for another window.
>>>>
>>>>
>>>> Regards,
>>>> Alexey
>>>
>>
>