On Thu, 26 Jun 2025 21:22:05 GMT, Anass Baya <ab...@openjdk.org> wrote:
>> **Analysis:**
>>
>> We are encountering a race condition in the native code. While retrieving
>> the screen number by calling _getScreenImOn(), the window is disposed. As a
>> result, the AWT-Windows event loop processes the Dispose() call, which
>> triggers UnlinkObjects().
>> The race condition between the execution paths of these two native methods
>> sometimes causes an exception
>>
>> **Proposed Fix:**
>>
>> While it's possible to introduce a synchronization mechanism, it would not
>> offer any real benefit. The window will be disposed regardless, and we’ll
>> fall back to the default screen. This behavior is already handled in
>> WWindowPeer.java, where a workaround is in place to use the default device
>> when getScreenImOn() returns a non-existent screen number
>>
>>
>> public void updateGC() {
>>
>> int scrn = getScreenImOn();
>>
>> if (screenLog.isLoggable(PlatformLogger.Level.FINER)) {
>> log.finer("Screen number: " + scrn);
>> }
>>
>> // get current GD
>> Win32GraphicsDevice oldDev = winGraphicsConfig.getDevice();
>>
>> Win32GraphicsDevice newDev;
>> GraphicsDevice[] devs = GraphicsEnvironment
>> .getLocalGraphicsEnvironment()
>> .getScreenDevices();
>>
>> // Occasionally during device addition/removal getScreenImOn can return
>> // a non-existing screen number. Use the default device in this case.
>> if (scrn >= devs.length) {
>> newDev = (Win32GraphicsDevice) GraphicsEnvironment
>> .getLocalGraphicsEnvironment().getDefaultScreenDevice();
>> } else {
>> newDev = (Win32GraphicsDevice) devs[scrn];
>> }
>> }
>>
>>
>> Therefore, I propose modifying the native method getScreenImOn to return the
>> default device if the peer is being disposed :
>>
>> jint AwtWindow::_GetScreenImOn(void *param)
>> {
>> ...
>> jboolean destroyed = JNI_GET_DESTROYED(self);
>> if (destroyed == JNI_TRUE){
>> env->DeleteGlobalRef(self);
>> return AwtWin32GraphicsDevice::GetDefaultDeviceIndex();
>> }
>> ...
>>
>>
>> **Tests Summary:**
>>
>> GetGraphicsStressTest (existing test):
>>
>> Fails intermittently without the fix
>>
>> Consistently passes with the fix
>>
>> NotifyStressTest (newly added test):
>>
>> Consistently fails without the fix
>>
>> Consistently passes with the fix
>
> Anass Baya has updated the pull request incrementally with three additional
> commits since the last revision:
>
> - BogusFocusableWindowState -2-
> - Add BogusFocusableWindowState
> - Delete NotifyStressTest
test/jdk/java/awt/Frame/BogusFocusableWindowState/BogusFocusableWindowState.java
line 33:
> 31: */
> 32:
> 33: public final class BogusFocusableWindowState {
please split the long line to 80 chars, also the empty line above is not really
necessary.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25619#discussion_r2170063306
