On Thu, 7 Dec 2023 22:53:25 GMT, Sergey Bylokhov <s...@openjdk.org> wrote:

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

The confusion could come from #14898 where [`RPC_E_CHANGED_MODE` doesn't 
prevent](https://github.com/openjdk/jdk/pull/14898#issuecomment-1748967705) us 
from calling DirectSound API which supports both MTA and STA.

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

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

Reply via email to