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

[Sergey Bylokhov]

On Tue, 10 Jun 2025 02:15:40 GMT, Anass Baya <abaya at openjdk.org> wrote:

>> 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.

Do we actually need a new lock? can we reuse the
AwtToolkit::GetInstance().InvokeFunction or AwtToolkit::GetInstance().SyncCall in a similar way it was done in https://github.com/openjdk/jdk/commit/d9bb0f0d39ed322b29c1807991e82fa28c4807f0?

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

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