Crash when (un)plugging USB audio device during ma_engine_init()

[mfroeschl]

Hello mackron,

I have encountered a crash with Miniaudio (c153a94) on Windows that happens when the user unplugs/plugs a USB audio device during an ongoing execution of ma_engine_init().

I have created a simple test program for VisualStudio that can be used to reproduce the issue with close to 100% probability, the code can be found here:

MiniAudioTest

Just run the test program, which will basically call ma_engine_init() / ma_engine_uninit() in an infinite loop, and then plug/unplug a USB audio device until the crash happens (usually happens after 1-3 plugs)

The crash seems to happen due to a null pointer exception in ma_IAudioClient_Start() where pThis appears to be NULL:

My best guess is this happens due to lack of thread safety between the ma_worker_thread() (triggered by ma_engine_init()), where the crash happens, and the thread kicked off by ma_IMMNotificationClient_OnDefaultDeviceChanged(). Here's a parallel stack taken at the time the issue happened:

Note:

I was only able to test this on a Windows machine. My guess is however, if there are equivalents to ma_IMMNotificationClient_OnDefaultDeviceChanged() et. al on other platforms, the issue will happen there as well.

Please find attached the log taken with the MA_DEBUG_OUTPUT flag.

log.txt

I would kindly appreciate it if you could take a look

Best regards, Marcus

[mackron]

Thanks for the report and the reproduction test. I was able to replicate this and I think it should be fixed now. Are you able to give the dev branch a try?

[mfroeschl]

Hello mackron,

thank you so much for your quick reply!

I tested it again with the dev branch. While it seems that the original issue is gone, I encountered a very similar issue, this time with ma_IAudioClient_Reset():

Again, pThis is NULL. Note however that this issue seems to happen less frequent, I had to do 10+ unplugs/plugs this time.

Here are the parallel stacks:

Maybe the scope of the new rerouteLock mutex has to be extended to include ma_device_stop() as well?

In any case, here is the log:

IAudioClientReset - log.txt

Best regards, Marcus

[mackron]

Thanks. Can you try the dev branch again now. In the previous commit I updated ma_device_start() to wait on the lock, but I didn't do the same for ma_device_stop(). I changed it so that ma_device_stop() now also waits on the lock. Hopefully that might fix it... I'm not reproducing either of the crashes anymore.

[mfroeschl]

Hello mackron,

thank you very much for the quick fix.

So far, I have not been able to reproduce the crash with the latest version.

I am however still experiencing another issue, but that happens at a much lower frequency (between 20 -50 plugs & unplugs in short intervals): It seems that sometimes, the ma_IAudioClient3_InitializeSharedAudioStream() or ma_IAudioClient_Initialize() functions will not return. To confirm this, I have surrounded the calling code with logs, like so:

            ma_log_postf(ma_context_get_log(pContext), MA_LOG_LEVEL_DEBUG, "Entering IAudioClient::Initialize()...\n");
            hr = ma_IAudioClient_Initialize((ma_IAudioClient*)pData->pAudioClient, shareMode, streamFlags, bufferDuration, 0, (WAVEFORMATEX*)&wf, NULL);
            ma_log_postf(ma_context_get_log(pContext), MA_LOG_LEVEL_DEBUG, "Left IAudioClient::Initialize().\n");

And then indeed in the log I can see that `IAudioClient::Initialize()) was entered, but for some reason it never seems to return:

DEBUG: === CHANGING DEVICE ===
INFO:   繧ｹ繝斐・繧ｫ繝ｼ (3- Sennheiser USB headset) (Playback)
INFO:     Format:      32-bit IEEE Floating Point -> 32-bit IEEE Floating Point
INFO:     Channels:    2 -> 2
INFO:     Sample Rate: 48000 -> 48000
INFO:     Buffer Size: 480*3 (1440)
INFO:     Conversion:
DEBUG: [WASAPI] IAudioClient3_GetSharedModeEnginePeriod failed. Falling back to IAudioClient.
INFO:       Pre Format Conversion:  NO
DEBUG: Entering IAudioClient::Initialize()...  /// IAudioClient::Initialize() is entered, but never returns
INFO:       Post Format Conversion: NO
INFO:       Channel Routing:        NO
INFO:       Resampling:             NO
INFO:       Passthrough:            YES
INFO:       Channel Map In:         {CHANNEL_FRONT_LEFT CHANNEL_FRONT_RIGHT}
INFO:       Channel Map Out:        {CHANNEL_FRONT_LEFT CHANNEL_FRONT_RIGHT}

// log ends here

The parallel stacks seem to confirm that the ma_IMMNotificationClient_OnDefaultDeviceChanged() thread is still "stuck" in the execution of the ma_IAudioClient_Initialize() function:

In any case, I realize that those are Windows APIs, and thus outside the scope of miniaudio.

I was just wondering if you have ever seen/heard of anything like this, and if there is any solution or workaround. A quick google search did not yield much.

I will attach the full log just in case.

IAudioClient_Initialize.log

Also, I will continue testing the latest dev branch some more for the original issue.

Best regards, Marcus

[mackron]

No, that's the first I've heard of that. Perhaps it could be a situation where the reroute is happening while the device is still being initialized? I'll have a look in a bit. I haven't been able to reproduce it though.

[mackron]

By the way, I had to make a change to those earlier changes I made. I accidentally had some Windows-specific code in the cross-platform section.

[mfroeschl]

Hi mackron,

thank you so much for your replies!

No, that's the first I've heard of that.

Understood. It could also be something specific to my sound device.

By the way, I had to make a change to those earlier changes I made.

Thanks for letting me know, I will continue to test with the latest version. Since I don't have access to that particular PC today, it will take until Friday though.

Cheers, Marcus

[mfroeschl]

Hi mackron,

FYI after some more testing I can indeed confirm that I am unable to reproduce the crash any longer with the latest version. Thank you again for the fix!

The deadlock in IAudioClient::Initialize() still happens, though.

However if I return from ma_IMMNotificationClient_OnDeviceStateChanged and ma_IMMNotificationClient_OnDefaultDeviceChanged immediately, basically disabling the code in those functions, the deadlock will not happen. So it is probably related to IAudioClient::Initialize() being invoked from a different thread (the IMM notification callback thread). Just an idea, but maybe the code could be executed on the worker thread instead?

I understand that for most miniaudio users, disabling the code in the IMM notification callbacks is not an option. In my case however, the app has its own IMM notification callbacks and basically I will be restarting miniaudio from the app. I assume it should be OK as a workaround to disable this code when I will be restarting miniaudio in any case?

Thank you for your support!

Best regards, Marcus

[mackron]

Yeah to me it sounds like it could be a similar issue as the play/stop case. I wonder if maybe I could apply the rerouting lock to ma_device_init()? The thing is, the reroute callbacks are configured right at the end of the init process (or so I though?) so I would have thought initialization would have to be finished before a reroute callback was fired. Maybe there's a subtle hole there that I need to address.

If you're able to work around automatic rerouting, an option might be to just disable it via your device config: deviceConfig.wasapi.noAutoStreamRouting = MA_TRUE;.

[mfroeschl]

Hi mackron,

Thank you for telling me about noAutoStreamRouting . For my app, all is well now.

However if you come up with a fix for the potential deadlock in IAudioClient::Initialize(), I'd still be happy to test it. Since the original issue (crash) was resolved IDK if you wish to keep this issue open or rather create a new one for the deadlock.

Cheers, Marcus

[mackron]

I'm happy to keep this open for the time being if only to remind me to look at it.

I just had a look at the code now, and the reroute notification callbacks are indeed the very last things that are being set when initializing the WASAPI stream. I'm not entirely sure what would be going here. I wonder if the most reliable solution might be to do initialization of the device on a single thread by making use of a job queue (post a job to the queue to initialize the device, process the jobs on a separate thread, use a synchronization primitive to wait for the job to complete).

[mackron]

I'm going to go ahead and close this one so I can get the issue tracked cleaned up, but I've added an item to my internal notes to look at this further if needed.

Thanks for your reports about this and testing.