On Wed, 2 Aug 2023 12:00:47 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:
>> Renjith Kannath Pariyangad has updated the pull request incrementally with
>> one additional commit since the last revision:
>>
>> Added DS_unlockCache into failed Coinit case
>
> 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 your if I missed anything.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14898#discussion_r1282616416
