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