Skip to content

8359266: Delete the usage of AppContext in the GraphicsDevice #25818

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 5 additions & 28 deletions src/java.desktop/share/classes/java/awt/GraphicsDevice.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1997, 2024, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1997, 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -27,7 +27,6 @@

import java.awt.image.ColorModel;

import sun.awt.AppContext;
import sun.awt.SunToolkit;

/**
Expand Down Expand Up @@ -75,13 +74,7 @@
*/
public abstract class GraphicsDevice {

private Window fullScreenWindow;
private AppContext fullScreenAppContext; // tracks which AppContext
// created the FS window
// this lock is used for making synchronous changes to the AppContext's
// current full screen window
private final Object fsAppContextLock = new Object();

private volatile Window fullScreenWindow;
private Rectangle windowedModeBounds;

/**
Expand Down Expand Up @@ -303,23 +296,15 @@ public void setFullScreenWindow(Window w) {
fullScreenWindow.setBounds(windowedModeBounds);
}
// Set the full screen window
synchronized (fsAppContextLock) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A synchronized block has stronger memory consistency guarantees than a volatile modifier.

Did you consider leaving the synchronized block but removing the usage of AppContext?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean by stronger in this use case?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean that there's a happens before relation each time the synchronized block is reach whereas with volatile, the happens before relation is guaranteed only when the value of the variable changes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean that there's a happens before relation each time the synchronized block is reach whereas with volatile, the happens before relation is guaranteed only when the value of the variable changes.

I do not think that this is true "guaranteed only when the value of the variable changes".

A write to a volatile field (§8.3.1.4) happens-before every subsequent read of that field.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly! A write to a volatile field.

Yet entering and leaving a synchronized block gives a happens-before relation each time.

Copy link
Member

@aivanov-jdk aivanov-jdk Jun 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I mean is two threads that called GraphicsDevice.getFullScreenWindow still established a happens-before relation for each call, which will be gone with the volatile modifier.

It may be fine… or may be not… this is why I'm asking.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I mean is two threads that called GraphicsDevice.getFullScreenWindow still established a happens-before relation for each call, which will be gone with the volatile modifier.
It may be fine… or may be not… this is why I'm asking.

That only affects the case where the second thread tries to read shared data (unrelated to GraphicsDevice) without synchronization, which was written before the first thread called getFullScreenWindow(). If visibility is required, it should be ensured by the caller. Depending on some internal and undocumented lock for that is not a good thing.

// Associate fullscreen window with current AppContext
if (w == null) {
fullScreenAppContext = null;
} else {
fullScreenAppContext = AppContext.getAppContext();
}
fullScreenWindow = w;
}
fullScreenWindow = w;
if (fullScreenWindow != null) {
windowedModeBounds = fullScreenWindow.getBounds();
// Note that we use the graphics configuration of the device,
// not the window's, because we're setting the fs window for
// this device.
final GraphicsConfiguration gc = getDefaultConfiguration();
final Rectangle screenBounds = gc.getBounds();
if (SunToolkit.isDispatchThreadForAppContext(fullScreenWindow)) {
if (EventQueue.isDispatchThread()) {
// Update graphics configuration here directly and do not wait
// asynchronous notification from the peer. Note that
// setBounds() will reset a GC, if it was set incorrectly.
Expand All @@ -342,15 +327,7 @@ public void setFullScreenWindow(Window w) {
* @since 1.4
*/
public Window getFullScreenWindow() {
Window returnWindow = null;
synchronized (fsAppContextLock) {
// Only return a handle to the current fs window if we are in the
// same AppContext that set the fs window
if (fullScreenAppContext == AppContext.getAppContext()) {
returnWindow = fullScreenWindow;
}
}
return returnWindow;
return fullScreenWindow;
}

/**
Expand Down
65 changes: 65 additions & 0 deletions test/jdk/java/awt/GraphicsDevice/FullScreenWindowRace.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/*
* Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

import java.awt.GraphicsDevice;
import java.awt.GraphicsEnvironment;
import java.awt.Window;

/**
* @test
* @key headful
* @bug 8359266
* @summary Tests for a race condition when setting a full-screen window
*/
public final class FullScreenWindowRace {

public static void main(String[] args) throws InterruptedException {
Window window = new Window(null);
GraphicsDevice gd = GraphicsEnvironment.getLocalGraphicsEnvironment()
.getDefaultScreenDevice();

Thread thread = new Thread(() -> {
while (gd.getFullScreenWindow() == null) {
// Busy wait - can be optimized away without volatile
}
});

thread.setDaemon(true);
thread.start();

// Give thread some time to start and begin the loop
Thread.sleep(2000);

gd.setFullScreenWindow(window);

thread.join(15000);

boolean alive = thread.isAlive();

gd.setFullScreenWindow(null);
window.dispose();
if (alive) {
throw new RuntimeException("Full screen window is NOT detected!");
}
}
}