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

2024-01-24 Thread Christoph Langer 
On Thu, 11 Jan 2024 16:52:40 GMT, Matthias Baesken  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

I agree, maybe in that kind of headless environment it could make sense to not 
initialize screens. I'll play with it and hopefully come back with something on 
[JDK-8185862](https://bugs.openjdk.org/browse/JDK-8185862). At least it seems 
we have such an environment in our configuration, so I can test there. Let's 
close this PR.

-

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

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

2024-01-24 Thread Phil Race 
On Thu, 11 Jan 2024 16:52:40 GMT, Matthias Baesken  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

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

2024-01-24 Thread Christoph Langer 
On Thu, 11 Jan 2024 16:52:40 GMT, Matthias Baesken  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

I have closed [JDK-8320405](https://bugs.openjdk.org/browse/JDK-8320405) as it 
does not match the synopsis of the issue addressed here. 
[JDK-8185862](https://bugs.openjdk.org/browse/JDK-8185862) is a better fit. I 
suggest to also close this PR and reboot on 
[JDK-8185862](https://bugs.openjdk.org/browse/JDK-8185862).

Technically, I agree, if `hBMDC` is NULL, we should return right away. However, 
the question is, how critical such a situation is. I think we need an 
understanding about the environment in which that can happen and whether we can 
accept it without assertions.

E.g. what it means when mieInfo.szDevice=`WinDisc` in a call to `HDC hDC = 
CreateDC(mieInfo.szDevice, NULL, NULL, NULL); ` which obviously yields NULL for 
hDC.

-

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

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

2024-01-11 Thread Matthias Baesken 
> 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

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/17197/files
  - new: https://git.openjdk.org/jdk/pull/17197/files/2e0d83f1..f7a5b7e7

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=17197=02
 - incr: https://webrevs.openjdk.org/?repo=jdk=17197=01-02

  Stats: 10 lines in 1 file changed: 4 ins; 0 del; 6 mod
  Patch: https://git.openjdk.org/jdk/pull/17197.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/17197/head:pull/17197

PR: https://git.openjdk.org/jdk/pull/17197

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

2024-01-10 Thread Alexey Ivanov 
On Thu, 4 Jan 2024 14:40:59 GMT, Matthias Baesken  wrote:

> > It should be extended to include the possible failure in the preceding call 
> > to ::GetDIBits.
> 
> I handle now also a failure of the first GetDIBits call; and also added 
> braces at the if's mentioned.

Sorry for my delayed reply. It's not what I expected.

What I meant is that we shouldn't even call `::CreateCompatibleBitmap` if 
`hBMDC == NULL`. Then we shouldn't call `::GetDIBits` if `hBM == NULL`. Thus in 
this case, the _fallback_ code path should be taken.

Perhaps, the code could be restructured… Yet I don't fully understand what's 
done here.

-

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

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

2024-01-04 Thread Matthias Baesken 
> 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:

  handle return value of other GetDIBits call, adjust if checks

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/17197/files
  - new: https://git.openjdk.org/jdk/pull/17197/files/cfc9088e..2e0d83f1

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk=17197=01
 - incr: https://webrevs.openjdk.org/?repo=jdk=17197=00-01

  Stats: 8 lines in 1 file changed: 4 ins; 0 del; 4 mod
  Patch: https://git.openjdk.org/jdk/pull/17197.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/17197/head:pull/17197

PR: https://git.openjdk.org/jdk/pull/17197

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

2024-01-04 Thread Matthias Baesken 
On Wed, 3 Jan 2024 15:36:16 GMT, Alexey Ivanov  wrote:

> It should be extended to include the possible failure in the preceding call 
> to ::GetDIBits.

I handle now also a failure of the first GetDIBits call; and also added braces 
at the if's mentioned.

-

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

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

2024-01-03 Thread Alexey Ivanov 
On Wed, 3 Jan 2024 08:16:12 GMT, Matthias Baesken  wrote:

> https://learn.microsoft.com/en-us/cpp/c-runtime-library/format-specification-syntax-printf-and-wprintf-functions?view=msvc-170
>  The %S is for wide chars, do you think we need the other one (%s) ?

You're right, it should be wide-char.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/17197#discussion_r1440679674

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

2024-01-03 Thread Alexey Ivanov 
On Thu, 28 Dec 2023 09:48:52 GMT, Matthias Baesken  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.

Hi Matthias,

> But still I think the coding itself should work better also in headless mode.

Then it should not only trace but fallback gracefully without calling APIs with 
the invalid parameters as discussed [in another 
thread](https://github.com/openjdk/jdk/pull/17197#discussion_r1439778065).

The traces won't fix anything. According to the provided logs `hBMDC` is 
`NULL`, therefore `hBM` also becomes `NULL`, thus `::GetDIBits` also fails.

There's already a workaround for 
[JDK-4684966](https://bugs.openjdk.org/browse/JDK-4684966):

https://github.com/openjdk/jdk/blob/fb90af881badf143163c7d0b9961152c2a12cd84/src/java.desktop/windows/native/libawt/windows/awt_Win32GraphicsDevice.cpp#L195-L199

It should be extended to include the possible failure in the preceding call to 
`::GetDIBits`.

-

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

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

2024-01-03 Thread Alexey Ivanov 
On Wed, 3 Jan 2024 00:38:19 GMT, Sergey Bylokhov  wrote:

> I think we should fallback to something. The bug itself looks similar to 
> this: https://bugs.openjdk.org/browse/JDK-8185862

[JDK-8185862](https://bugs.openjdk.org/browse/JDK-8185862) seems to be the 
exact duplicate of this one.

It references [JDK-8269529](https://bugs.openjdk.org/browse/JDK-8269529) which 
implemented [a 
workaround](https://github.com/openjdk/jdk/commit/d042029509a8cbdb723f78e2cfee4e2885775814#diff-fba4a205d0e60634be77d7a7f16a582277f7461d92a9c70cfb96d4a87068d0f1R36):

https://github.com/openjdk/jdk/blob/d042029509a8cbdb723f78e2cfee4e2885775814/test/jdk/javax/swing/reliability/HangDuringStaticInitialization.java#L36

[The same 
workaround](https://github.com/openjdk/jdk/commit/5c083e8560ce9cc78615e3149a558206724cff53#diff-3b5e63f29c2a52feda881e4115e5d09d1b8aecd4698f1d72981de94caa3a46d0R50)
 was implemented for [JDK-8266129](https://bugs.openjdk.org/browse/JDK-8266129):

https://github.com/openjdk/jdk/blob/5c083e8560ce9cc78615e3149a558206724cff53/test/jdk/tools/jpackage/windows/WinInstallerIconTest.java#L50

There's @prrace's comment [in the 
JBS](https://bugs.openjdk.org/browse/JDK-8266129?focusedId=14416854=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14416854):
 https://bugs.openjdk.org/browse/JDK-8266129?focusedId=14416854=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14416854;>There's
 a bunch of AWT implementation missing to properly work headless. Random tests 
run into it. The tests should not have to deal with it. Instead it requires an 
unfunded effort to rework headless mode support in AWT and 2D.

So, we can add `@requires !vm.debug` to the 
`MultiResolutionImageObserverTest.java` test or, better, handle the situation 
gracefully.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/17197#discussion_r1440577143

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

2024-01-03 Thread Matthias Baesken 
On Tue, 2 Jan 2024 19:33:44 GMT, Alexey Ivanov  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.
>
> src/java.desktop/windows/native/libawt/windows/awt_Win32GraphicsDevice.cpp 
> line 142:
> 
>> 140: } else {
>> 141: J2dTraceLn1(J2D_TRACE_WARNING,
>> 142:"CreateDC failed, so we cannot store the DC for 
>> later usage, mieInfo.szDevice is %S",
> 
> Is `%S` a valid format string? It should in lower case: `%s`.
> 
> It seems, “store the DC for later usage” is misleading, it isn't stored… at 
> least for a long period of time.

https://learn.microsoft.com/en-us/cpp/c-runtime-library/format-specification-syntax-printf-and-wprintf-functions?view=msvc-170
The %S is for wide chars, do you think we need the other one (%s) ?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/17197#discussion_r1440174314

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

2024-01-03 Thread Matthias Baesken 
On Thu, 28 Dec 2023 09:48:52 GMT, Matthias Baesken  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.

> Is the purpose of this review to just add traces?
> 
> The errors seems to come from _headless_ environment. A better fix would be 
> to add `@key headful` to the `MultiResolutionImageObserverTest.java` test, 
> even though the test does not display UI.

Hi Alexey, I am not against adding the key to the test, probably this makes 
sense.
But still I think the coding itself should work better also in headless mode.

-

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

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

2024-01-02 Thread Sergey Bylokhov 
On Tue, 2 Jan 2024 20:04:39 GMT, Alexey Ivanov  wrote:

>> src/java.desktop/windows/native/libawt/windows/awt_Win32GraphicsDevice.cpp 
>> line 194:
>> 
>>> 192: if (hBM == NULL) {
>>> 193: J2dTraceLn(J2D_TRACE_WARNING, 
>>> "AwtWin32GraphicsDevice::Initialize CreateCompatibleBitmap failed");
>>> 194: }
>> 
>> Does it make sense to call `::GetLastError()` and trace its value?
>> 
>> It seems that we cannot continue if `hBMDC` or `hBM` are null. Yet it seems 
>> unexpected.
>
>> It seems that we cannot continue if `hBMDC` or `hBM` are null. Yet it seems 
>> unexpected.
> 
> Should we rather modify the code to back out and not to call APIs with NULL 
> handles?

I think we should fallback to something. The bug itself looks similar to this: 
https://bugs.openjdk.org/browse/JDK-8185862

-

PR Review Comment: https://git.openjdk.org/jdk/pull/17197#discussion_r1439981113

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

2024-01-02 Thread Alexey Ivanov 
On Tue, 2 Jan 2024 19:37:53 GMT, Alexey Ivanov  wrote:

> It seems that we cannot continue if `hBMDC` or `hBM` are null. Yet it seems 
> unexpected.

Should we rather modify the code to back out and not to call APIs with NULL 
handles?

-

PR Review Comment: https://git.openjdk.org/jdk/pull/17197#discussion_r1439778065

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

2024-01-02 Thread Alexey Ivanov 
On Thu, 28 Dec 2023 09:48:52 GMT, Matthias Baesken  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.

Is the purpose of this review to just add traces?

The errors seems to come from *headless* environment. A better fix would be to 
add `@key headful` to the `MultiResolutionImageObserverTest.java` test, even 
though the test does not display UI.

src/java.desktop/windows/native/libawt/windows/awt_Win32GraphicsDevice.cpp line 
142:

> 140: } else {
> 141: J2dTraceLn1(J2D_TRACE_WARNING,
> 142:"CreateDC failed, so we cannot store the DC for 
> later usage, mieInfo.szDevice is %S",

Is `%S` a valid format string? It should in lower case: `%s`.

It seems, “store the DC for later usage” is misleading, it isn't stored… at 
least for a long period of time.

src/java.desktop/windows/native/libawt/windows/awt_Win32GraphicsDevice.cpp line 
194:

> 192: if (hBM == NULL) {
> 193: J2dTraceLn(J2D_TRACE_WARNING, 
> "AwtWin32GraphicsDevice::Initialize CreateCompatibleBitmap failed");
> 194: }

Does it make sense to call `::GetLastError()` and trace its value?

It seems that we cannot continue if `hBMDC` or `hBM` are null. Yet it seems 
unexpected.

src/java.desktop/windows/native/libawt/windows/awt_Win32GraphicsDevice.cpp line 
309:

> 307: }
> 308: if (hBM != NULL) VERIFY(::DeleteObject(hBM));
> 309: if (hBMDC != NULL) VERIFY(::DeleteDC(hBMDC));

This should rather be expanded `if` statements with braces.

-

PR Review: https://git.openjdk.org/jdk/pull/17197#pullrequestreview-1800910802
PR Review Comment: https://git.openjdk.org/jdk/pull/17197#discussion_r1439754392
PR Review Comment: https://git.openjdk.org/jdk/pull/17197#discussion_r1439757044
PR Review Comment: https://git.openjdk.org/jdk/pull/17197#discussion_r1439764934

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

2023-12-28 Thread Matthias Baesken 
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.

-

Commit messages:
 - JDK-8320405

Changes: https://git.openjdk.org/jdk/pull/17197/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk=17197=00
  Issue: https://bugs.openjdk.org/browse/JDK-8320405
  Stats: 14 lines in 1 file changed: 12 ins; 0 del; 2 mod
  Patch: https://git.openjdk.org/jdk/pull/17197.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/17197/head:pull/17197

PR: https://git.openjdk.org/jdk/pull/17197