cubeb_jack: Replace all mutexes with atomics

[zamaudio]

This patch removes all pthread mutexes in the jack backend, especially that ugly trylock in the realtime audio thread. These are replaced with atomic variables. Passes all tests, although JACK xruns when test_sanity is being called in the cubeb.stream_position test where there are massive 500ms delays.

Users note that there is still a bug with Firefox itself that causes cubeb never to shutdown (#390). This can cause xruns on firefox quit until it is fixed. However, this patch should see an improvement over the previous backend code. Partially fixes #154

[jyavenard]

FWIW: making all variables atomic isn't an equivalent to the use of a single mutex.
It no longer appears to keep consistency between all group of variables.

[zamaudio]

"making all variables atomic isn't an equivalent to the use of a single mutex."

I do not make all variables atomic.

"It no longer appears to keep consistency between all group of variables."

Can you please elaborate, the variables I made atomic are used in such a way that only one thread is allowed to write to one atomic variable and the next time the other thread executes it reads the value so that two threads are never writing to the same variable and getting out of sync.

[kinetiknz]

What concrete problem does this change solve? The review and maintenance burden of verifying attempts at lock free implementations likely outweighs any benefit we'll ever see from this change, in my opinion. Sorry, but I don't think we will be merging this without an incredibly strong reason to do so.

[szanni]

What concrete problem does this change solve?

It is trying to solve a design problem. The current code is broken. Using mutexes in real time context is a big no. No audio library I am aware of does this. All use lockless buffers implemented through atomics. Some real time audio rules here.

The reason the current code works is because the code uses mutex_trylock, so it will not block. The buffer is instead filled with silence when the mutex_trylock fails. How can that be considered correct behaviour? Especially since it can be prevented and jack is geared towards audio professionals.

That being said, the current code does seem to work on most machines without hiccups. Or I could at least not find any bug reports about audio glitches. And I understand the effort involved in verifying lockless algorithms. I have written code like this myself. It is a pain. And we do seem to get lucky most of the time as mutex contentions is low. I added some debug symbols. So far I have not been able to hit the insert silence code path. I do wonder how many people do hit that code path from time to time on other hardware and less performant systems though. I am commenting here as I got concerned about the mutex calls myself while reading through the current code relating to another issue.

All that being said, this PR is beyond broken. Line 145 of the PR tells me the submitter has no idea how atomics work. A bool is not an atomic. Looking at the rest of the code: it does not seem to solve anything either. It is just the attempt at implementing a mutex through atomics and does not solve anything!? You would need a lockless ring buffer with a reader and a writer thread or something similar.

[zamaudio]

@szanni You're completely right. The current code is broken because it has trylocks in the realtime thread. This PR was an attempt to fix it, but as you described, it uses booleans in place of a mutex instead of a lockless ring buffer mechanism. I urge anyone who knows how to fix this to do so, and I will close this PR soon. I don't know how to fix it properly and I also don't have time to spend on it further.

[szanni]

I do in theory know how to fix this, as I have written push & callback style back ends for alsa, coreaudio, sdl, &&
But this looks like a bigger time sink. Understanding the cubeb API and then rewriting the whole jack back end entirely. Because looking at the current state of the code I have zero faith in other stuff not being broken too and making incorrect assumptions. I mean this is how I got here in the first place. The current implementation crashes hard on xruns trying to access memory that is out of bounds. That's why I opened #607, @zamaudio you actually seem to have fixed the subtraction logic in this PR as a side effect as well.

If somebody wants to open a bug bounty, I might find the time to write a proper jack back end. I would however recommend just switching Firefox to SDL2.0 as that is battle tested, supports pretty much all platforms imaginable, and is released under the very permissive zlib license. I am not a fan of the SDL API, but at least the library is not broken and the developers seem understand how audio works.

@zamaudio maybe close this PR and open up an issue instead lamenting the mutex stuff and linking to this thread? Or I could do that too. And I hope my language wasn't too strong criticising your PR :)