Usage of non atomic operations between threads in LinkHut

[pwnified]

The method used in LinkHut to communicate info to and from the audio thread, are not atomic and will break sometimes on 32 bit platforms with torn writes. It happens when a Float64 or UInt64 value is half written then the other thread reads the value.

[pwnified]

Oh, err. never mind, I was looking at an older version of LinkHut :+1: it seems the newer version has a spinlock.

[lijon]

Hmm, I don't think it's a good idea to use locks on the audio thread! Better to use atomic operations and memory barriers, or something like AEMessageQueue from TheAmazingAudioEngine.

[edsharp]

I'd love a definitive answer on the locks front.

You've got Michael (TAAE) along with many people on the CA list saying "don't use them at all".

Then you've got Gabor (Superpowered) saying you can use them as long as you know the operations they're wrapping will complete rapidly.

LinkHut seems to be in the camp that they can be used.

Atomic op's and memory barriers feels like the safer option, but I don't know whether there really are cases where a lock is safe to use, too.

[lijon]

It's impossible to know that an operation on another thread will complete rapidly. For example, the audio thread is waiting on the lock while the main thread is holding it. but before main thread is done with the operation, it might be preempted by some other thread, etc. This creates "priority inversion": the audio thread effectively got only main-thread priority.

For a single demo-app this might not be very relevant, but devs might copy the LinkHut example. The audio thread is a global shared resource, if every music app currently running would use locks in their audio threads (waiting for each apps low-priority main threads, etc) there would be very high risk of glitches and buffer drops.

[michaeltyson]

It's also not just me and other third parties saying don't use locks, Obj-C or malloc - this comes from the Core Audio team itself. I'd back that up with a reference but I'm on my phone =)

Sent from my iPhone

On 22 Mar 2016, at 8:26 PM, Jonatan Liljedahl notifications@github.com wrote:

It's impossible to know that an operation on another thread will complete rapidly. For example, the audio thread is waiting on the lock while the main thread is holding it. but before main thread is done with the operation, it might be preempted by some other thread, etc. This creates "priority inversion": the audio thread effectively got only main-thread priority.

For a single demo-app this might not be very relevant, but devs might copy the LinkHut example. The audio thread is a global shared resource, if every music app currently running would use locks in their audio threads (waiting for each apps low-priority main threads, etc) there would be very high risk of glitches and buffer drops.

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub

[brs-ableton]

The goal of LinkHut is to demonstrate the usage of LinkKit, so we tried to make it as small and clear as possible. In the first version we didn't synchronize the data at all between the threads for this reason. After getting several questions about the validity of this, we added the spin lock to at least make it thread safe. But still this is not ideal, as has been pointed out above.

Another constraint with LinkHut is that we want it to be completely standalone and not require any additional dependencies to build and run. Otherwise, we would use a lock free queue from AAE or some other library.

I think we didn't realize that people would take this as a sort of "app template" or an example of how to do things in an iOS audio app. We probably should have invested more time in making it worthy of this, but were focused on the interaction with the ABLLink library.

We will do another iteration on this and try to make a better model for iOS apps. I'm open for suggestions as to what would be the most idiomatic way of handling this without any external libraries. If we're limited to just what's available with OSAtomic*s I would probably try either double buffering the data block and swapping pointers between the two copies or using OSAtomicEnqueue/Dequeue, although those don't look too friendly. Any thoughts would be appreciated.

[lijon]

Maybe something along the lines of this:

put all state that needs to be communicated in a struct typedef. have two pointer variables of this type, new and old.

main thread:

if(old) free(old); old = NULL; OSMemoryBarrier(); State newstate = malloc() and fill in the details.. compare-and-swap to set new = newstate;

audio thread:

if(new), read its state and compare-and-swap to move it to old.

Not sure of the details, I simply use AEMessageQueue myself :) Note that AEMessageQueue can be used standalone, all you need is AEMessageQueue.m and h. I personally ripped it out of AEAudioController of TAAE because I needed the message queue but not the rest!

Cheers /Jonatan

On Tue, Mar 22, 2016 at 3:14 PM, Brent Shields notifications@github.com wrote:

The goal of LinkHut is to demonstrate the usage of LinkKit, so we tried to make it as small and clear as possible. In the first version we didn't synchronize the data at all between the threads for this reason. After getting several questions about the validity of this, we added the spin lock to at least make it thread safe. But still this is not ideal, as has been pointed out above.

Another constraint with LinkHut is that we want it to be completely standalone and not require any additional dependencies to build and run. Otherwise, we would use a lock free queue from AAE or some other library.

I think we didn't realize that people would take this as a sort of "app template" or an example of how to do things in an iOS audio app. We probably should have invested more time in making it worthy of this, but were focused on the interaction with the ABLLink library.

We will do another iteration on this and try to make a better model for iOS apps. I'm open for suggestions as to what would be the most idiomatic way of handling this without any external libraries. If we're limited to just what's available with OSAtomic*s I would probably try either double buffering the data block and swapping pointers between the two copies or using OSAtomicEnqueue/Dequeue, although those don't look too friendly. Any thoughts would be appreciated.

— You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/Ableton/LinkKit/issues/18#issuecomment-199833706

/Jonatan http://kymatica.com

[brs-ableton]

@lijon Thanks Jonatan, that's roughly what I meant by double-buffering. As for bundling AEMessageQueue, that's another thing to consider.

[pwnified]

On Mar 22, 2016, at 2:26 AM, Jonatan Liljedahl notifications@github.com wrote: but before main thread is done with the operation, it might be preempted by some other thread, etc. This creates "priority inversion": the audio thread effectively got only main-thread priority.

Thanks. I’ve never seen the use of a spin lock in a core audio thread before, I always just used ring buffers to communicate, but your post makes it clear exactly why they are a bad idea.

On Mar 22, 2016, at 7:14 AM, Brent Shields notifications@github.com wrote: We will do another iteration on this and try to make a better model for iOS apps. I'm open for suggestions as to what would be the most idiomatic way of handling this without any external libraries. If we're limited to just what's available with OSAtomic*s I would probably try either double buffering the data block and swapping pointers between the two copies or using OSAtomicEnqueue/Dequeue, although those don't look too friendly. Any thoughts would be appreciated.

A single atomic swap is probably good, it doesn’t add any dependencies, it’s easy to read and serves as a proper example of how to do a quick communication with the realtime thread.