RFR: JDK-8320405: [Windows Server 2016] java/awt/image/MultiResolutionImage/MultiResolutionImageObserverTest.java shows issues in awt_Win32GraphicsDevice.cpp [v3]

[Phil Race]

On Thu, 11 Jan 2024 16:52:40 GMT, Matthias Baesken <mbaesken at openjdk.org> wrote:

>> When running with fastdebug binaries we run intermittent into the issue below in
>> jtreg test java/awt/image/MultiResolutionImage/MultiResolutionImageObserverTest.java .
>> Seems we miss checking of successful HBITMAP creation before calling GetDIBits.
>> 
>>   HDC hBMDC = this->GetDC();
>>   HBITMAP hBM = ::CreateCompatibleBitmap(hBMDC, 1, 1);
>>   VERIFY(::GetDIBits(hBMDC, hBM, 0, 1, NULL, gpBitmapInfo, DIB_RGB_COLORS));
>> 
>> in awt_Win32GraphicsDevice.cpp . Thats why the releast of hBMDC / hBM fails as well at the end of the function causing the seond and third warning.
>> 
>> 
>> Sat Nov 18 17:29:23 CET 2023
>> 
>> *********************
>> AWT Assertion Failure
>> *********************
>> ::GetDIBits(hBMDC, hBM, 0, 1, 0, gpBitmapInfo, 0)
>> File 'e:\openjdk\openjdk-21u-windows_x86_64-dbg\jdk\src\java.desktop\windows\native\libawt\windows\awt_Win32GraphicsDevice.cpp', at line 184
>> GetLastError() is 57 : The parameter is incorrect.
>> 
>> Do you want to break into the debugger?
>> *********************
>> *********************
>> AWT Assertion Failure
>> *********************
>> ::DeleteObject(hBM)
>> File 'e:\openjdk\openjdk-21u-windows_x86_64-dbg\jdk\src\java.desktop\windows\native\libawt\windows\awt_Win32GraphicsDevice.cpp', at line 297
>> GetLastError() is 57 : The parameter is incorrect.
>> 
>> So it seems, we should have some additional checks/tracing.
>
> Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision:
> 
>   do not call CreateCompatibleBitmap if hBMDC is NULL, do not call GetDIBits if hBM is NULL

There are multiple levels of problem here
At the highest level there's an entire code path we should not have entered if this is headless, but
on windows the AWT implementation is not doing a good job of this. I don't know off-hand exactly
what a fix for it would look like but changing this code without looking at the bigger picture seems premature

If we handle that, then we would not be here and would not have an issue in your environment and moreover
since we would not see the failure, we would not have an assert to deal with. 
It is then a separate issue unrelated to the debug build that we don't check for failure but I really doubt
we'd ever see a failure here on a headful config.

I do not agree with adding the headful keyword because this is a common code path and it essentially means
we'd keep adding it to a load of headful tests because of this AWT problem.

Our answer to this to date has been do not run AWT tests on debug builds.
Arguably this has "worked" as much by luck as anything else since clearly the call can fail on product builds too, but it has never caused any problems that I have heard of. That doesn't make it satisfactory, but it also doesn't make  a point fix the right fix.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/17197#issuecomment-1909073511