On Tue, 25 Jul 2023 11:34:03 GMT, Renjith Kannath Pariyangad <d...@openjdk.org> 
wrote:

>>> @aivanov-jdk, Thank you for your time and reviews, `CoInitializeEx(NULL, 
>>> 0)` also resolving this problem because as per document **The default is 
>>> COINIT_MULTITHREADED**.
>> 
>> This is what I expected, however, the documentation for 
>> [`COINIT`](https://learn.microsoft.com/en-us/windows/win32/api/objbase/ne-objbase-coinit)
>>  doesn't spell out the value for `COINIT_MULTITHREADED`.
>> 
>>> In my debug session noticed `DirectSoundCaptureEnumerate` getting called 
>>> ahead of thread call or ::CoInitialize and that was the root cause this 
>>> failure.
>> 
>> If it is the case, I can't see that you have ensured that 
>> `DirectSoundCaptureEnumerate` is called after `CoInitialize`. Do I miss 
>> anything?
>> 
>>> For resolving this found 3 ways and all works:
>>> 
>>> 1. Use COM multi-threading  :- Simplest approach with respect to other 
>>> solutions
>> 
>> Probably… But it is still incorrect. Initialising COM on *a thread* doesn't 
>> mean you can call COM object methods from **any thread** in your application.
>> 
>> As the documentation says, “You need to initialize the COM library on a 
>> thread before you call any of the library functions…” (I have this very 
>> quote in [my comment 
>> above](https://github.com/openjdk/jdk/pull/14898#discussion_r1272027291).)
>> 
>>> 2. Hold the call till CoInitialize : - Can end up in deadlock situation so 
>>> that need to be taken care.
>> 
>> It's a challenge but it is the only right way, as far as I can see… Perhaps 
>> not to wait till `::CoInitialize` is called on a thread (see above) but to 
>> transfer a call for handling to a thread where COM is initialised.
>> 
>>> 3. Do another CoInitialize before call  :- not a good approach
>> 
>> Yet it does not break the rules: you can initialise COM, call an API and 
>> then call 
>> [`CoUninitialize`](https://learn.microsoft.com/en-us/windows/win32/api/combaseapi/nf-combaseapi-couninitialize).
>>  If you don't keep any references to COM objects, it is valid.
>> 
>> 
>>> _This implies that any thread which creates DirectSound objects must 
>>> initialise COM first. I can't see it happening. Do I miss anything?_ 
>>> `DS_StartBufferHelper` class initializing COM in thread and all member 
>>> functions checking initialization status before making any calls. So 
>>> another solution may be restructure the code for accessing 
>>> `isInitialized()` method from outer methods and proceed. (approach 2 in 
>>> better way)
>>> 
>>> Tried this approach with `::CoInitialize` and observed _truncated names_, 
>>> but its not failing for lock unlock workflow.
>> 
>> As I explained above, it would still be inco...
>
>> > @aivanov-jdk, Thank you for your time and reviews, `CoInitializeEx(NULL, 
>> > 0)` also resolving this problem because as per document **The default is 
>> > COINIT_MULTITHREADED**.
>>
>> This is what I expected, however, the documentation for 
>> [`COINIT`](https://learn.microsoft.com/en-us/windows/win32/api/objbase/ne-objbase-coinit)
>>  doesn't spell out the value for `COINIT_MULTITHREADED`.
> 
> As per the declaration yes its 0 
> [tagCOINIT](https://github.com/wine-mirror/wine/blob/master/include/objbase.h)
>  
> 
>> > In my debug session noticed `DirectSoundCaptureEnumerate` getting called 
>> > ahead of thread call or ::CoInitialize and that was the root cause this 
>> > failure.
> 
>> If it is the case, I can't see that you have ensured that 
>> `DirectSoundCaptureEnumerate` is called after `CoInitialize`. Do I miss 
>> anything?
> 
> Yes, You are right in main thread no CoInitialize
> 
>> > For resolving this found 3 ways and all works:
>> > 
>> > 1. Use COM multi-threading  :- Simplest approach with respect to other 
>> > solutions
>>
>> Probably… But it is still incorrect. Initialising COM on _a thread_ doesn't 
>> mean you can call COM object methods from **any thread** in your application.
>> 
>> As the documentation says, “You need to initialize the COM library on a 
>> thread before you call any of the library functions…” (I have this very 
>> quote in [my comment 
>> above](https://github.com/openjdk/jdk/pull/14898#discussion_r1272027291).)
>> 
>> > 2. Hold the call till CoInitialize : - Can end up in deadlock situation so 
>> > that need to be taken care.
>> 
>> It's a challenge but it is the only right way, as far as I can see… Perhaps 
>> not to wait till `::CoInitialize` is called on a thread (see above) but to 
>> transfer a call for handling to a thread where COM is initialised.
> 
> These two approach didn't do  ::CoInitialize in running thread (main thread) 
> 
>> > 3. Do another CoInitialize before call  :- not a good approach
>> 
>> Yet it does not break the rules: you can initialise COM, call an API and 
>> then call 
>> [`CoUninitialize`](https://learn.microsoft.com/en-us/windows/win32/api/combaseapi/nf-combaseapi-couninitialize).
>>  If you don't keep any references to COM objects, it is valid.
> 
> Tried this approach and it works, we may need to do the similar approach for 
> another function `DAUDIO_GetDirectAudioDeviceDescription` because this also 
> calling `DirectSoundEnumerateW `& `DirectSoundCaptureEnumerateW `
> 
>> 
>> > _This implies that any thread which creates DirectSound objects must 
>> > initialise COM first. I can't see it happening. Do I miss anything?_...

> Probably… But it is still incorrect. Initialising COM on a thread doesn't 
> mean you can call COM object methods from any thread in your application.

Note that "Objects created on a COM thread in a multithread apartment (MTA) 
must be able to receive method calls from other threads at any time", so if we 
initialisethe devices on one thread using COM they will be avaliable on other 
threads as well. But the spec for DirectSoundEnumerate says nothing about 
COM... But from some examples I see ppl wrap it by the CoInitializeEx(NULL, 
COINIT_MULTITHREADED)/CoUninitialize();

Noe sure we can call CoInitializeEx on the main thread.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14898#discussion_r1275476189

Reply via email to