On Fri, 1 Mar 2024 08:01:08 GMT, Christoph Langer <clan...@openjdk.org> wrote:

>> The assertions reported in the bug were observed spuriously and here and 
>> there broke tests in some Windows configurations.
>> For instance [JDK-8266129](https://bugs.openjdk.org/browse/JDK-8266129), 
>> [JDK-8269529](https://bugs.openjdk.org/browse/JDK-8269529) or 
>> [JDK-8323664](https://bugs.openjdk.org/browse/JDK-8323664) came up due to 
>> this.
>> 
>> The problem is that in Windows environments without a valid display, e.g. 
>> started by system services or via PowerShell Remoting, one sees a Monitor 
>> with name 'Windisc' in `EnumDisplayMonitors`.
>> However, it seems to be some kind of a pseudo device where you can not get a 
>> DC via `CreateDC`. This behavior/monitor type doesn't seem to be well 
>> documented, though.
>> 
>> I hereby modify the device initialization code to only count/detect monitors 
>> where CreateDC returns non-NULL in Devices.cpp. I also add some more 
>> checking/error handling to AwtWin32GraphicsDevice::Initialize() for 
>> correctness.
>> 
>> Furthermore, I re-enable the test 
>> `javax/swing/reliability/HangDuringStaticInitialization.java` for Windows 
>> Debug VMs, which reverts the fix from JDK-8269529 that should not be 
>> necessary any more.
>
> Christoph Langer has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   load awt.dll before display check

It looks good to me.

The only question I have is for the fallback in `awt_Win32GraphicsDevice.cpp`.

Bailing out quickly makes the code cleaner. In this case, if `::GetDIBits` 
fails, we can bail out too.

If the fallback is needed, the code below the first call to `::GetDIBits` 
should be executed even if `GetDC` and `CreateCompatibleBitmap` fail.

Since testing didn't reveal any problems, the fallback is probably not as 
important, or not critical for a headless system.

src/java.desktop/windows/classes/sun/awt/PlatformGraphicsInfo.java line 35:

> 33: public class PlatformGraphicsInfo {
> 34: 
> 35:     private static boolean hasDisplays;

I believe it can be declared `final`, it's initialised in the static 
initialiser that follows the declaration.
Suggestion:

    private static final boolean hasDisplays;

src/java.desktop/windows/classes/sun/awt/PlatformGraphicsInfo.java line 38:

> 36: 
> 37:     static {
> 38:         loadAWTLibrary();

Can `WToolkit.loadLibraries()` be used here? The method is declared public, so 
it should be accessible.

It may create a circular dependency though.

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

PR Review: https://git.openjdk.org/jdk/pull/17614#pullrequestreview-1917750109
PR Review Comment: https://git.openjdk.org/jdk/pull/17614#discussion_r1513269942
PR Review Comment: https://git.openjdk.org/jdk/pull/17614#discussion_r1513267417

Reply via email to