On Thu, 15 May 2025 16:48:47 GMT, Matthias Bläsing <mblaes...@openjdk.org>
wrote:
>> - Introduce a lock into WClipboard that protects the code between
>> openClipboard/closeClipboard invocations.
>> The native side does not allow to open the clipboard multiple
>> times or share the opened clipboard between multiple threads.
>>
>> - Remove of need to call openClipboard/closeClipboard from
>> getClipboardFormats by using the win32 call
>> GetUpdatedClipboardFormats
>>
>> - Prevent a race-condition by not registering the connection
>> between java and native side of clipboard multiple time, but
>> just at construction time.
>
> Matthias Bläsing has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Add crash reproducer as jtreg test
src/java.desktop/share/classes/sun/awt/datatransfer/SunClipboard.java line 208:
> 206:
> 207: try {
> 208: openClipboard(null);
Can you help me understand why this is better? Seems odd that the try/finally
block has `closeClipboard` in the finally block but the `openClipboard` was
moved out of the try.
src/java.desktop/windows/classes/sun/awt/windows/WClipboard.java line 189:
> 187: checkChange(formats);
> 188: } catch (Throwable ex) {
> 189:
> System.getLogger(WClipboard.class.getName()).log(Level.WARNING, "Failed to
> process handleContentsChanged", ex);
I'm not saying not to necessarily change this, but what are we hoping to see
with logging this warning here?
test/jdk/java/awt/Clipboard/ConcurrentClipboardAccessTest.java line 29:
> 27: @summary tests that concurrent access to the clipboard does not crash
> the JVM
> 28: @run main ConcurrentClipboardAccessTest
> 29: */
It has been brought to my attention that this test needs to be headful since
we're dealing with clipboard behavior.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24614#discussion_r2114346429
PR Review Comment: https://git.openjdk.org/jdk/pull/24614#discussion_r2114337249
PR Review Comment: https://git.openjdk.org/jdk/pull/24614#discussion_r2114349984
