RFR: 8301846: Invalid TargetDataLine after screen lock when using JFileChooser or COM library [v7]

[Alexey Ivanov]

On Wed, 2 Aug 2023 03:23:33 GMT, Renjith Kannath Pariyangad <duke at openjdk.org> wrote:

>> Hi Reviewers,
>> 
>> Observations :
>> 1. Without com initialize if we access Mixer for recording, library loaded invalid GUID and clipped description in windows(ID not found in registry). With com initialization library load proper GUID (same as registry).
>> 2. For Play back device always loading proper device GUID irrespective of com Initialization.
>> 
>> Test:
>> Since screen lock and unlock workflow required for reproducing this issue, did coupe of iteration of manual testing post fix and confirmed its resolving the problem.
>> To reconfirm nothing is broken, executed all audio related test cases on test bench post fix and all are green.
>> 
>> Please review the changes and let me know your comments if any.
>> 
>> Regards,
>> Renjith.
>
> Renjith Kannath Pariyangad has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Added DS_unlockCache into failed Coinit case

Changes requested by aivanov (Reviewer).

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

src/java.desktop/windows/native/libjsound/PLATFORM_API_WinOS_DirectSound.cpp line 183:

> 181:     INT32 cacheIndex;
> 182: 
> 183:     if (!DS_lockCache() || FAILED(::CoInitialize(NULL))) {

As I [mentioned before](https://github.com/openjdk/jdk/pull/14898#discussion_r1276539431), I still think `::CoInitializeEx(NULL, COINIT_MULTITHREADED | COINIT_DISABLE_OLE1DDE)` suits this case better than `::CoInitialize(NULL)`.

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

PR Review: https://git.openjdk.org/jdk/pull/14898#pullrequestreview-1558847830
PR Review Comment: https://git.openjdk.org/jdk/pull/14898#discussion_r1281804647
PR Review Comment: https://git.openjdk.org/jdk/pull/14898#discussion_r1281807251