8239894: Xserver crashes when the wrong high refresh rate is used

[mrserb]

We use two different approaches to listing the display modes on the system:

• For single-screen configuration use version 1.1 protocol XRRGetScreenInfo/XRRConfigRates/XRRConfigSizes
• For multi-screen configuration use version 1.2 protocol: XRRGetScreenResources/XRRGetOutputInfo/XRRModeInfo

Unfortunately, the current code does not work for some specific double scan modes, it doubles the real refresh rate. If this refresh rate will be passed to the Xserver later it could hang/crash.

The same bug existed in xrandr utility before version 1.4:
https://bugs.freedesktop.org/show_bug.cgi?id=37043
https://gitlab.freedesktop.org/xorg/app/xrandr/commit/7fd4f18b649f22fad4dbf9fc64b69b3e7f172207

As suggested in the bug report I have tried to unify the code for single and multiscreen systems and use updated code currently used on the multiscreen.

But unfortunately, I have found that the multi-screen code is even more broken. The problematic code is this:

    int scr = 0, out = 0;
    if (usingXinerama && XScreenCount(awt_display) > 0) {
        out = screen;
    } else {
        scr = screen;
    }

The idea of the code is: if the passed screen is Xinerama index used by the java2d, then the "scr" will be zero, and "out" will be the "virtual" index for that screen. If the screen is X11 screen then "src" will be the correct screen and "out" will be zero.

The problem is that the code assumes that xrandr expect the "virtual" index as the "out" parameter, but this is not. This parameter is the index of the "output" such as DP1, DP2, other ports/etc.

So sometimes the code works when the first screen is connected to the first port on the video card and the second screen to the second port, otherwise, we just report data for the wrong screen or report nothing.

I did not found a reliable way to map Xinerama srceens to the xrandr. It is better to rework the java2d to use the xrandr instead of xinerama.

As a fix, I suggest dropping the code which uses the incorrect mapping screen->output. This includes the code related to the display modes added in the JDK-8167486 + JDK-8022810, and also the code related to the calculation of scale factor from the "ubuntu property" which does not work on the recent ubuntu version after migration to gnome3 added in the JDK-8149115

I will file a separate two bugs to reimplement this functionality, most probably the "display modes" will be implementing when we fully migrate to the xrandr, and calculation of the scale factor will be implemented via the gtk library.

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed

Issue

• JDK-8239894: Xserver crashes when the wrong high refresh rate is used

Reviewers

• Alexander Zuev (@azuev-java - Reviewer)

Download

$ git fetch https://git.openjdk.java.net/jdk pull/2136/head:pull/2136
$ git checkout pull/2136

[bridgekeeper]

👋 Welcome back serb! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@mrserb The following labels will be automatically applied to this pull request:

• 2d
• awt

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 00: Full (c051177)

[azuev-java]

I guess not having this functionality at all is better than having a code that hangs. Can we have these new bugs about re-implementing this functionality linked in the initial bug so they can be tracked easier?

[openjdk]

@mrserb This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8239894: Xserver crashes when the wrong high refresh rate is used

Reviewed-by: kizune

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 35 new commits pushed to the master branch:

• 044bae0: 8260191: Do not include access.hpp in oop.hpp
• 77a4302: 8223056: Remove Type-Stable-Memory support for Parkers
• 35c9da7: 8259498: Reduce overhead of MD5 and SHA digests
• 27cc62a: 8259911: byteArrayViewVarHandle should throw ArrayIndexOutOfBoundsException
• 1f47de5: 8260010: UTF8ZipCoder not thread-safe since JDK-8243469
• 8b95d95: 8256895: Add support for RFC 8954: Online Certificate Status Protocol (OCSP) Nonce Extension
• 4f11ff3: 8259488: Shenandoah: Missing timing tracking for STW CLD root processing
• 0785147: 8259949: x86 32-bit build fails when -fcf-protection is passed in the compiler flags
• 5891509: 8259947: (fs) Optimize UnixPath.encode implementation
• 69f90b5: 8259843: initialize dli_fname array before calling dll_address_to_library_name
• ... and 25 more: https://git.openjdk.java.net/jdk/compare/533a2d398bd4df46b6b73f01a54622c8fb4a2545...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[mrserb]

/integrate

[openjdk]

@mrserb Since your change was applied there have been 49 commits pushed to the master branch:

• 2f47c39: 8259943: FileDescriptor.close0 does not handle EINTR
• a8073ef: 8253478: (se) epoll Selector should use eventfd for wakeup instead of pipe
• 34eb8b3: 8255765: Shenandoah: Isolate concurrent, degenerated and full GC
• c3c6662: 8259954: gc/shenandoah/mxbeans tests fail with -Xcomp
• 6ce0799: 8259851: Use boolean type for tasks in SubTasksDone
• 4bcffeb: 8260029: aarch64: fix typo in verify_oop_array
• e1de0bf: 8260043: Reduce allocation in sun.net.www.protocol.jar.Handler.parseURL
• 4dfd8cc: 8259897: gtest os.dll_address_to_function_and_library_name_vm fails on AIX
• 7f7166d: 8260035: Deproblemlist few problemlisted test
• 5940287: 8260048: Shenandoah: ShenandoahMarkingContext asserts are unnecessary
• ... and 39 more: https://git.openjdk.java.net/jdk/compare/533a2d398bd4df46b6b73f01a54622c8fb4a2545...master

Your commit was automatically rebased without conflicts.

Pushed as commit a7c2ebc.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.