On Tue, 25 Jul 2023 09:10:10 GMT, Alexey Ivanov <aiva...@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`.

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?_ 
> > `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 incorrect: if enumerating sound 
> devices requires COM, it must be initialised on the thread where you 
> enumerate the devices.

Got your point.

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

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

Reply via email to