`miniaudio` conflicts with `tinyfiledialogs`, possible multi-threading issue

[raysan5]

Environment

• Windows 10
• miniaudio 0.11.11
• tinyfiledialogs 3.9.0
• raylib master branch (4.5-dev)

Description

Recently I found a very weird issue when working in one of my tools. I use tinyfiledialogs library to manage native file dialogs for several platforms.

Loading a dialog is a blocking function call that opens some kind of system dialog and returns control to application when resolved.

I detected that, when enabling audio device (InitAudioDevice() -> ma_device_init()), a call to tinyfd_selectFolderDialog() just completely hangs the application. No output, no control, no debug info; Visual Studio debugging also hangs and needs to be restarted.

If application does not use audio (no InitAudioDevice()), tinyfd_selectFolderDialog() works as expected, it opens a folder select system dialog and user can choose a directory to return from the function.

Very weird issue... but due to the nature of it, I thought it could be related to threading... common to both libraries.

Investigating the issue I got to the Windows system call SHBrowseForFolderW() from system library shell32.lib. Reading the docs, I saw:

If you initialize COM using CoInitializeEx, you must set the COINIT_APARTMENTTHREADED flag in its dwCoInit parameter.

tinyfd_selectFolderDialogW() is explicitly calling:

lHResult = CoInitializeEx(NULL, COINIT_APARTMENTTHREADED);

I checked if the same convention was used by miniaudio and I found it was using:

ma_CoInitializeEx(pDevice->pContext, NULL, MA_COINIT_VALUE);`

in two functions and MA_COINIT_VALUE is mapped to COINIT_MULTITHREADED:

#define MA_COINIT_VALUE    0   /* 0 = COINIT_MULTITHREADED */

So, I just tried to change MA_COINIT_VALUE to COINIT_APARTMENTTHREADED... and magically everything worked as expected.

I'm afraid I don't know the cause of the issue. I don't know if I should change that value or if the issue is on miniaudio or tinyfiledialogs side.

I'm sure you have more knowledge than me on multi-threading and could provide some more light on this issue. Thanks!

[raysan5]

Just did a quick search and I found a related issue: https://github.com/mackron/miniaudio/issues/28

[raysan5]

Just found a user with similar concerns: CoInitializeEx COINIT_MULTITHREADED and ShellExecute. It includes a detailed answer.

My main concern is: if I use:

#define MA_COINIT_VALUE  2   // [2] COINIT_APARTMENTTHREADED

Is there any unexpected/undesired effect on miniaudio performance?

[meshula]

I got hit with this as well in my project, where the user makes the case that "Apartment" is required for UWP. https://github.com/LabSound/LabSound/issues/121 (LabSound wraps RtAudio and miniaudio, so this issue comes up.)

There's no performance impact on choosing one or the other, but it's an unending can of worms as people discover corner cases.

My opinion is that no library should magically try to set this up, so miniaudio, LabSound and tinyfiledialogs, in trying to "automagically" work simply open a can of worms.

RtAudio attempts to do the right thing

  HRESULT hr = CoInitialize( NULL ); 
  if ( FAILED(hr) ) {
    errorText_ = "RtApiAsio::ASIO requires a single-threaded apartment. Call CoInitializeEx(0,COINIT_APARTMENTTHREADED)";
    error( RTAUDIO_WARNING );
  }

which might gave an idea on adding a runtime diagnostic, but I haven't got anything concrete to offer as a solution, except to note that switching to Apartment doesn't cause performance problems for miniaudio.

[raysan5]

@meshula Thanks for your answer! Good to know it's a know issue and it does not affect performance. I'm not an expert on threading, I'm investigating a bit more about the multiple initialization modes for threads...

[mackron]

@raysan5 What you're doing is fine. MA_COINIT_VALUE exists exactly for situations like this one. You're doing it the right thing. There's no known performance or stability implications.

[raysan5]

@mackron Thank you very much for your quick answer! Good to know it is the expected use case. Closing the issue.

[raysan5]

@mackron Is it ok if I enable COINIT_APARTMENTTHREADED as default for raylib?

Could it imply future conflicts with other libraries in the future?

[mackron]

I couldn't tell you what the best default option would be, but to be honest I don't think it really matters. It only effects COM objects, so unless you're using a COM object across multiple threads, which I doubt raylib is doing a lot of, I don't think it matters. If my understanding is correct, COINIT_APPARTMENTTHREADED is the safest option, and is what's used with the original CoInitialize() I believe. The MULTITHREADED option I think is for people who want to do their own synchronization, which is what I do in miniaudio which is probably why I chose it.

[raysan5]

@mackron Again, thank you very much for your answer! :)