PortAudio / portaudio

PortAudio is a cross-platform, open-source C language library for real-time audio input and output.
Other
1.39k stars 291 forks source link

pa_win_wmme.c: WriteStream infinite WAIT_TIMEOUT deadlock #883

Closed dannye closed 2 months ago

dannye commented 5 months ago

Describe the bug

Here in pa_win_wmme.c there seems to be a block deliberately unfinished:

https://github.com/PortAudio/portaudio/blob/147dd722548358763a8b649b3e4b41dfffbcfbb6/src/hostapi/wmme/pa_win_wmme.c#L3904-L3909

I don't know what may cause WaitForSingleObject() to return WAIT_TIMEOUT but this appears to happen rarely and sporadically, at least on Windows 10 and Windows 11.

And when it does happen, it happens indefinitely.

This causes my program's secondary thread to be indefinitely stuck in that do {} while loop.

I can't think of a single way for my main thread to send a signal to terminate the secondary thread while the secondary thread is stuck in the infinite loop.

If/when my main thread sends a termination signal to the portaudio thread, it then waits for the portaudio thread to terminate itself, which never happens because the portaudio thread is stuck in an infinite loop, and so my program deadlocks.

To Reproduce

The repeated/indefinite WAIT_TIMEOUT result from WaitForSingleObject() appears to happen pretty reliably after the Windows PC is put to sleep or into hibernate for several minutes while the portaudio thread is running, and then the PC is woken up.

I have also witnessed this happen extremely rarely during normal execution without the PC going to sleep, probably twice in 2 years. I currently have no way of reproducing this.

Expected behavior

As the todo comment suggests, portaudio "should give up eventually", break from the loop, and return some non-success code, to return control to my application code. Or in a more general sense, the WriteStream() function should have some way of detecting that something has gone wrong in this situation and return a non-success code.

Actual behavior

portaudio remains in the while( framesWritten < frames ) loop indefinitely as a result of the WAIT_TIMEOUT result from WaitForSingleObject().

Desktop (please complete the following information):

Additional context

I'm only using the wmme Host API by happenstance. If you think I would benefit from using a different Windows Host API, please let me know. My usage of portaudio was simply based on this example from libopenmpt: https://github.com/OpenMPT/openmpt/blob/master/examples/libopenmpt_example_cxx.cpp

If useful, my code is here: https://github.com/dannye/crystal-tracker https://github.com/dannye/crystal-tracker/blob/339a9bd/src/main-window.cpp#L1506 https://github.com/dannye/crystal-tracker/blob/339a9bd/src/main-window.cpp#L3014 https://github.com/dannye/crystal-tracker/blob/339a9bd/src/it-module.cpp#L63-L69

dannye commented 5 months ago

FWIW: While I have only ever used portaudio v19.7.0, the code in question appears to be more or less the same on the master branch:

https://github.com/PortAudio/portaudio/blob/daaf637f6f9fce670031221abfd7dfde92e5cce3/src/hostapi/wmme/pa_win_wmme.c#L3889-L3894

dannye commented 5 months ago

Sorry for triple posting, but even more additional context:

I only call _stream.write() (via the cpp binding's BlockingStream) after confirming Pa_IsStreamActive(_stream.paStream()) == 1 which I believe is all I need to do to ensure that writing to the stream is legal.

So I do not think this is user-error in the sense of calling WriteStream() while in an invalid state.

RossBencina commented 5 months ago

Thanks. Sounds like a bug of some sort. How frequently does it happen?

dannye commented 5 months ago

How frequently does it happen?

Like I said, the issue occurs with high reliability if the PC is put to sleep for several minutes while the thread is running (at least in my program).

The repeated/indefinite WAIT_TIMEOUT result from WaitForSingleObject() appears to happen pretty reliably after the Windows PC is put to sleep or into hibernate for several minutes while the portaudio thread is running, and then the PC is woken up.

RossBencina commented 3 months ago

PR #889 is a draft proposed fix.

RossBencina commented 3 months ago

You raise an interesting point about checking Pa_IsStreamActive. Currently in pa_front.c we check whether the stream is not stopped in Pa_ReadStream and Pa_WriteStream but quite possibly we should be checking that the stream is active. That said, I don't think it is currently possible for a WMME stream to become inactive without stopping it.

dannye commented 3 months ago

Thanks! The change looks very reasonable. I will try it out very soon and report back.

dannye commented 3 months ago

@RossBencina I can confirm that your proposed fix completely solves this edge case and enables my program to recover sanely! Thank you very much for looking into this, and thank you and everyone who works on PortAudio.

May I ask when the next official release of PortAudio might be? Just wondering if I should update my build instructions to say to just use the master branch of PortAudio for the foreseeable future (assuming your PR gets merged, of course).

RossBencina commented 3 months ago

@dannye

I can confirm that your proposed fix completely solves this edge case and enables my program to recover sanely!

Thanks for testing the PR. We'll review and merge it once Phil is back on deck. Using a timeout here is imperfect because if the glitch/pause is transient (but longer than the timeout) then the stream will not be robust to glitches, on the other hand we don't seem to have any other options for detecting a permanently frozen stream.

If you think I would benefit from using a different Windows Host API, please let me know.

WMME is the oldest and hence most backward compatible API. As far as I know it is stable and reliable everywhere. It is not the most performant, nor does it offer some advanced features. But if you care about your software running on very old Windows versions then it is one of your only options (DirectSound being the other). Personally WMME is still my first choice, but today the obvious alternative to WMME is WASAPI -- Microsoft's modern standard API. WASAPI was introduced in Windows Vista and had reduced quirks since Windows 7. PortAudio's WASAPI host API implementation is well supported and maintained. Your other options are DirectSound and ASIO -- there are reasons to use these APIs, but if you have to ask then they probably shouldn't be your first choice.

May I ask when the next official release of PortAudio might be?

We have not scheduled a date (our resources are too stretched to commit to a date) but Phil and I are talking about it and I think we'll soon start deferring unfinished work so we can settle the release. So the best I can say is probably sometime in the middle of this year. For now I'd suggest pointing your users at our master branch if you need this fix.

RossBencina commented 2 months ago

@dannye I've merged #889 to master now. Thanks for your assistance.