On Thu, 7 Apr 2022 23:03:26 GMT, Sergey Bylokhov <s...@openjdk.org> wrote:
>> Maxim Kartashev has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains three commits: >> >> - Merge branch 'master' into JDK-8280468 >> - Addressed PR comments >> >> 1. Got rid of X11GraphicsDevice.configLock in favour of the AWT lock. >> 2. Reduced the use of the screen number (and, consequently, AWT lock >> contention) in XCanvasPeer. >> - 8280468: Crashes in getConfigColormap, getConfigVisualId, >> XVisualIDFromVisual on Linux >> >> This fix addresses two possible causes of crashes: >> 1. makeDefaultConfig() returning NULL. The fix is to die gracefully >> instead of crashing in an attempt to de-reference a NULL pointer. >> >> 2. A race condition when the number of screens change (this is only an >> issue with the number of xinerama screens; the number of X11 screens is >> static for the duration of the X session). >> >> The race scenario: X11GraphisDevice.makeDefaultConfiguration() is called >> on EDT so the call can race with X11GraphisDevice.invalidate() that >> re-sets the screen number of the device; the latter is invoked on the >> "AWT-XAWT" thread from X11GraphicsEnvironment.rebuildDevices(). So by >> the time makeDefaultConfiguration() makes a native call with the >> device's current screen number, the x11Screens array maintained by the >> native code could have become shorter. And the native methods like >> Java_sun_awt_X11GraphicsDevice_getConfigColormap() assume that the >> screen number passed to them is always current and valid. The AWT lock >> only protects against the changes during the native methods invocation, >> but does not protect against them being called with an outdated screen >> number. >> >> The fix is to eliminate the race by protecting X11GraphisDevice.screen >> with the AWT lock. > > src/java.desktop/unix/classes/sun/awt/X11/XCanvasPeer.java line 90: > >> 88: XToolkit.awtUnlock(); >> 89: } >> 90: } > > I am not sure I understand the purpose of this method, it requests the > current device for the GC, then requests the screen index of the device, and > then request device by the screen index, what is the reason to do that? Why > we cannot just use the device from the GC? This is just a named copy of lines 67 to 75. Since now this is the only piece of code that needs to work holding the AWT lock, I thought it best to isolate it. As I understand, the purpose of the piece is to find the "new" `X11GraphicsDevice` with the screen number same to the given "old" `GraphicsConfiguration`. ------------- PR: https://git.openjdk.java.net/jdk/pull/7182
