On Thu, 3 Aug 2023 04:02:16 GMT, Renjith Kannath Pariyangad <d...@openjdk.org>
wrote:
>> src/java.desktop/windows/native/libjsound/PLATFORM_API_WinOS_DirectSound.cpp
>> line 183:
>>
>>> 181: INT32 cacheIndex;
>>> 182:
>>> 183: if (!DS_lockCache() || FAILED(::CoInitialize(NULL))) {
>>
>> I believe this is incorrect: `::CoInitialize` may return `S_OK` or
>> `S_FALSE`; the value of `S_FALSE` indicates a success too but flags that COM
>> is already initialised. The only real error is `RPC_E_CHANGED_MODE` which
>> also indicates *COM is already initialised* but with a different threading
>> model.
>>
>> Thus, we can proceed in *all* the cases but we need to call
>> `::CoUninitialize` only if `::CoInitialize` returns `S_OK` or `S_FALSE`.
>
> Microsoft recommended to use [SUCCEEDED or
> FAILED](https://learn.microsoft.com/en-us/windows/win32/learnwin32/error-handling-in-com)
> macro for error handling. Based on our finding without COM Initialization
> remaining function call may fail, I thought better not to proceed further in
> case of failure.
>
> FAILED macro defined as : #define FAILED(hr) (((HRESULT)(hr)) < 0)
> Constant defined as **S_OK =0x0** and **S_FALSE = 0x1** all other errors are
> -ve
>
> I thought this will be covered, let me know if I missed anything.
Thank you for clarification, I forgot that `FAILED` handled both `S_OK` and
`S_FALSE`.
Yet we also need to handle `RPC_E_CHANGED_MODE` error code in which case we can
proceed but we don't call `::CoUninitialize`. If another error code is
returned, we back out.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14898#discussion_r1327639556
