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

[Anass Baya]

On Tue, 3 Jun 2025 19:05:06 GMT, Phil Race <prr at openjdk.org> wrote:

>> Anass Baya has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix - 2
>
> src/java.desktop/windows/native/libawt/windows/awt_Window.cpp line 2515:
> 
>> 2513:         return AwtWin32GraphicsDevice::GetDefaultDeviceIndex();
>> 2514:     }
>> 2515: 
> 
> The code you referred to in the description is handling the case of when a *device* (ie screen) is disconnected. 
> This is the window peer being destroyed.
> Assuming that what you described is correct - some other thread is in a race with this thread - this change looks to me to greatly reduce the likelihood but not eliminate it - ie between the call at line 2510 above and 2516 below, the destruction could happen.
> 
> So isn't the right fix to not create the Throwable in this case ? And just do the default return.
> ie maybe the above check goes after the pData == NULL case and if destroyed, do the default return, and if its something else, do the throw.
> And perhaps (not sure) that line 2521 should always have returned the default device, not 0 ?

Hello @prrace, @mrserb,
Thank you for your review.

@prrace, regarding line 2521, perhaps they were returning 0 because they assumed the default device always has index 0, which is not true. I also agree with you — with proper synchronization, checking pData after verifying whether the peer is destroyed is unnecessary.

@mrserb, yes — both situations can happen.
For the second case (i.e. parallel execution), I verified it using a volatile flag. A race condition between the EDT and the AWT-Window thread is indeed possible. I didn’t add synchronization in the first fix to avoid impacting performance since I was not running into issues by just verfying if the peer is destroyed.

However, the proper and complete fix would be:

- Add synchronization between UnlinkObjects() (which, in this case, is invoked by the AWT-Window thread) and _GetScreenImOn() (which is called on the EDT thread).

- Ensure that the peer is not destroyed inside _GetScreenImOn().

By doing both, we can be sure that no race condition occurs between _GetScreenImOn() and UnlinkObjects() that could corrupt pData, and we also protect _GetScreenImOn() from accessing a peer that has just been destroyed (as you assumed in your first point, @mrserb)

I have added that proposal so you can take a look at it.

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

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