RFR: 8270269: Desktop.browse method fails if earlier CoInitialize call as COINIT_MULTITHREADED [v2]

Sergey Bylokhov serb at openjdk.org
Thu Dec 7 22:57:53 UTC 2023


On Thu, 7 Dec 2023 15:11:24 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:

>> Sergey Bylokhov has updated the pull request incrementally with three additional commits since the last revision:
>> 
>>  - 8270269: Desktop.browse method fails if earlier CoInitialize call as COINIT_MULTITHREADED
>>  - Revert "6508941: java.awt.Desktop.open causes VM to crash with video files sporadically"
>>    
>>    This reverts commit 85269470
>>  - Revert "8270269: Desktop.browse method fails if earlier CoInitialize call as COINIT_MULTITHREADED"
>>    
>>    This reverts commit 4908d9c220950683d3a5010d12ab756eff6b6fa7.
>
> src/java.desktop/windows/native/libawt/windows/awt_Desktop.cpp line 99:
> 
>> 97:             ::CoUninitialize();
>> 98:         }
>> 99:     }
> 
> I'm unsure this is the right thing to do. The documentation for [`ShellExecute`](https://learn.microsoft.com/en-us/windows/win32/api/shellapi/nf-shellapi-shellexecutew) mentions, <q cite="https://learn.microsoft.com/en-us/windows/win32/api/shellapi/nf-shellapi-shellexecutew">Some Shell extensions require the COM single-threaded apartment (STA) type.</q> I believe this is true for most shell extensions because they deal with UI.
> 
> At the same time, <q cite="https://learn.microsoft.com/en-us/windows/win32/api/shellapi/nf-shellapi-shellexecutew">There are certainly instances where `ShellExecute` does not use one of these types of Shell extension and those instances would not require COM to be initialized at all.</q>
> 
> So, if the current thread initialised COM as MTA, the call to `ShellExecute` would likely fail if it needs COM to perform the requested action as it requires STA in majority of cases.
> 
> A workaround would be to hand off the task to a special thread, or just fail straight away as the code does right now. Otherwise, the errors could be sporadic and unpredictable: some calls complete successfully whereas others fail.
> 
> Could this code use [`ComInvoker`](https://github.com/openjdk/jdk/blob/58530f4098538f490cfea58f2382d0997841c171/src/java.desktop/windows/classes/sun/awt/shell/Win32ShellFolderManager2.java#L572) from `Win32ShellFolderManager2`?

You are right! This reminded me why initial implementation uses COINIT_APARTMENTTHREADED. So jump on another thread seems the only possible solution.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17010#discussion_r1419743727


More information about the client-libs-dev mailing list