On Thu, 7 Dec 2023 15:11:24 GMT, Alexey Ivanov <aiva...@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
