RFR: 8346952 : GetGraphicsStressTest.java fails: Native resources unavailable [v4]

[Sergey Bylokhov]

On Tue, 10 Jun 2025 22:17:13 GMT, Anass Baya <abaya at 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 one additional commit since the last revision:
> 
>   Remove global lock

src/java.desktop/windows/native/libawt/windows/awt_Window.cpp line 2505:

> 2503: 
> 2504:     // It's entirely possible that our native resources have been destroyed
> 2505:     // before our java peer - if we're dispose()d, for instance.

You can merge this text and "// Return the default screen." to something like:

// Our native resources may have been destroyed before the Java peer,
// e.g., if dispose() was called. In that case, return the default screen.

src/java.desktop/windows/native/libawt/windows/awt_Window.cpp line 2507:

> 2505:     // before our java peer - if we're dispose()d, for instance.
> 2506:     // Return the default screen.
> 2507:     JNI_CHECK_PEER_GOTO(self, ret);

I suggest reordering it slightly - this pattern is commonly used in most cases where JNI_CHECK_PEER_GOTO is used:

    result...;
    AwtWindow *window = NULL;

    PDATA pData;
    JNI_CHECK_PEER_GOTO(self, ret);
    window = (AwtWindow *)pData;

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/25619#discussion_r2138935690
PR Review Comment: https://git.openjdk.org/jdk/pull/25619#discussion_r2138934388