> Speaking of racy, this code in pcm-sdl.c / pcm-android.c
> doesn't look very safe:
> ------------------------------------
> void pcm_play_lock(void)
> {
> if (++audio_locked == 1)
> SDL_LockMutex(audio_lock);
> }
> ------------------------------------
>

It's only used to lock out the SDL callback, not lock out other rockbox
threads, which currently are not run as preemptive/concurrent but either
single thread "fiber" or serialized by a global mutex in thread-sdl.c. If
one thread ups the count in the latter case, no other thread will call
SDL_LockMutex but instead see the count as 2 or greater.

> We should use gcc atomic operations for "audio_locked". The
> SDL_LockMutex()

No reason to per above.

> will issue a full memory barrier but if two pcm_play_lock() functions are
> called at the same time, this will fail badly.

Won't happen. Only rockbox threads, not interrupt-like SDL threads can use
pcm_play_lock/unlock. The audio callback uses the mutex directly, thus gets
blocked if any rockbox thread has claimed it.

-----

That's how it's supposed to work anyway. If you've found a hole, please
bring it to light.

Regards,
Mike
Received on 2011-12-03