performous / performous

An open-source music and rhythm game.
https://performous.org
Other
489 stars 106 forks source link

PortAudio Errors when stopping performous in debug mode #672

Closed Baklap4 closed 2 years ago

Baklap4 commented 3 years ago

Do you want to request a feature or report a bug?

Bug

What did you do?

Start performous Stop performous Hit a breakpoint in external code

Error message on console:

ERROR: failed PaWinUtil_CoUninitialize calling thread[8948] does not match initializing thread[16540] 
ERROR: failed PaWinUtil_CoUninitialize calling thread[8948] does not match initializing thread[16540]

What did you expect to see?

I expected performous to actually close without coughing up errors.

What did you see instead?

Errors as explained above.

Output of performous --version: (What version of Performous are you using?)

Latest git master

What is your environment & configuration (arguments, platform, ...)?

Windows - with -DCMAKE_BUILD_TYPE=Debug (different than the not defined build type). I don' t get this error when running in RelWithDebInfo or Release

If applicable, please paste the log output in DEBUG level (--logLevel=DEBUG switch)

ERROR: failed PaWinUtil_CoUninitialize calling thread[8948] does not match initializing thread[16540] 
ERROR: failed PaWinUtil_CoUninitialize calling thread[8948] does not match initializing thread[16540]

https://media.discordapp.net/attachments/273434633138470913/884818022852861962/unknown.png?width=1417&height=676

Notes

@OznOg wrote:

We spawn a thread to stop PA, and try to have the stop asynchronous, but it will hang anyway and crash the problem is that the throw will trigg a stack unwind thus will destroy the future that will wait for the thread to complete so it hangs... and when eventually, the thread finishes, the crash will occur I found that PaAbortStream exists, maybe it works fine to stop quickly

@Lord-Kamina wrote:

Starting and Stopping Streams Newly opened Streams are initially stopped. You call Pa_StartStream() to start a Stream. You can stop a running Stream using Pa_StopStream() or Pa_AbortStream() (the Stop function plays out all internally queued audio data, while Abort tries to stop as quickly as possible). An open Stream can be started and stopped multiple times. You can call Pa_IsStreamStopped() to query whether a Stream is running or stopped. By calling Pa_SetStreamFinishedCallback() it is possible to register a special PaStreamFinishedCallback that will be called when the Stream has completed playing any internally queued buffers. This can be used in conjunction with the paComplete stream callback return value (see below) to avoid blocking on a call to Pa_StopStream() while queued audio data is still playing.

Lord-Kamina commented 3 years ago

In a nutshell, I asked @Baklap4 the open this.

@Tronic I have a hunch that you added this ages ago because it was hanging for you, yes? If you were on mac at the time, that behavior was reported at least once upstream but it seems it could have been a CoreAudio limitation, rather than a Portaudio bug.

I was hoping we could try to get rid of the audiokiller and see how portaudio 19.7 behaves. If I'm correct in the motivation for it but the error persists, maybe we can keep this behavior only when CoreAudio is being used as backend?

Tronic commented 3 years ago

It was probably hanging on Linux, where I recall such problems being common. Could be related e.g. to unplugging of USB sound card while in use. I am not very confident of such problems being fixed so that the audiokiller could be entirely removed, but perhaps it could be disabled for debug builds (like we already disable some exception catching in debug).

And then make sure that releases are actually built in release mode, not in debug with optimizations which is the default for developers.

Baklap4 commented 3 years ago

I am not very confident of such problems being fixed so that the audiokiller could be entirely removed

I guess that's to be tested

And then make sure that releases are actually built in release mode

This should always be done when releasing, so yes!

Baklap4 commented 3 years ago

Just tested out removing those std::async calls within the destructors. This doesn't seem to resolve the problem that we still encounter an error on exit. Getting a Debug Assertion Failed statement within debug_heap.cpp from the appcrt so i guess it won't be an easy/fast fix ghehe

Tronic commented 3 years ago

This might also be specific to Edirol M16DX, a soundcard-mixer suitable for karaoke use but out of production for many years now. IIRC, no-one was willing to fix driver bugs on that even ten years ago or so, and the hanging of devices might've been one of those issues.

I still got one in use, though, but I haven't used it on Linux. Would need quite a bit of testing to confirm whether the hang issue is truly gone, and frankly I don't feel like doing that.

OznOg commented 2 years ago

I think keeping that "in case of" is a bad idea. Actually, if the hang is still present in some way somewhere, we'll get some bug open and it will be time to see what kind of setup causes it and see if the former workaround is still relevant.

Baklap4 commented 2 years ago

I think keeping that "in case of" is a bad idea.

Agreed. If it's still there we'll get a bug report. Else the problem is long gone ;)

I'll re-check #676 and see if the suggestion/note by @OznOg there fixes stuff