PortAudio / portaudio

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

Consider making the entire PortAudio API thread safe #808

Open RossBencina opened 1 year ago

RossBencina commented 1 year ago

The PortAudio API is "specified" as single-threaded: All API calls must occur on the same thread (or, with some caveats, on a non-overlapping sequence of "current main thread" protected by a mutex. There is a ticket to better document the current specification.

This ticket is here as a placeholder for discussion about making the PortAudio API threadsafe.

RossBencina commented 1 year ago

The simplest implementation would be to use a monitor mutex around the entire API. This will still be non-trivial:

It may be desirable to have more fine-grained locking for operations (starting/stopping a stream) that do not interact with the global PortAudio state.

We need to agree on a locking strategy and someone would have to implement it. Not a high priority for me personally, but don't let that stop you.

dechamps commented 1 year ago

Related: #607 dechamps/FlexASIO#124. Currently PortAudio is not just thread-unsafe - it is downright thread-hostile, because it will malfunction when called from more than one thread, even if the calls are not being made concurrently.

With regard to solutions, one thing to keep in mind while exploring the design space is reentrancy, especially calling a PortAudio function from within the stream callback. Depending on implementation, mutex locks might not be reentrant.

dmitrykos commented 1 year ago

Calling API of the library from multiple threads is a sign of suboptimal or I would say bad design to my view. By adding mutexes you increase complexity and destroy determinism of the application. On the other hand, synchronizing thread access via messaging makes behavior of your application more predictable.

There is nothing wrong or bad to synchronize calls and let PA live in a separate thread (can be the main thread of the process) with this a lot of problems are avoided, including platform-dependent, such as for example the need to call CoInitialize for every thread calling some Windows API, e.g. WASAPI backend.

The best thread-safe addition to PA to my view would be to restrict calls from multiple threads with assertions, e.g. we:

  1. save thread id in the Pa_Initialize call in pa_front.c
  2. check thread id of the caller to other functions which we treat as thread-unsafe and fail with assert() if thread id of the caller does not match thread id of the thread which initialized PA, so thread-unsafe function will in fact enforce thread safety explicitly
  3. mark in documentation every external function whether it is thread-safe or thread-unsafe
  4. describe in documentation in portaudio.h how PA must be used in relation to multiple-threads

Standardizing thread access in this way would make PA reliable by enforcing the predictable behavior in the multi-threaded application. It is also industry-common approach, for example Google Android enforces calls to UI API on main thread, the same does Apple iOS with UIKit, as well as Windows UWP for UI classes.

PA shall not try to fix the flawed design of the multi-threaded application by providing the defense against sporadic calls from different threads but really needs to enforce some specific behavior in multi-threaded application.

MichalPetryka commented 1 year ago

Personally I believe that all APIs that don't take instance data (like initialization and opening streams) should be thread safe, while APIs that do take it (all APIs operating on streams) should be thread safe as long as the instance passed in is different than the ones running on different threads. This ensures that accesses from different libraries or parts of the code you have that access Portaudio can all use it safely as long as they don't access each others instances and from what I've seen, moving thread-unsafe data storage to instance data is usually the standard practice there.