Consider using Library Context to allow multiple instances of PA

[dmitrykos]

Library Context will remove dependency on static variables instantiated in pa_front.c and will solve issues similar to #766. It also makes PA API more versatile, for example user would keep Library Context in a thread context which is calling PA API.

Context management functions:

typedef struct PaContextImp *PaContext;

PaError Pa_CreateContext( PaContext *context )
void Pa_DestroyContext( PaContext context )

Modified API:

PaError Pa_Initialize( PaContext context )
PaError Pa_Terminate( PaContext context )
const PaHostErrorInfo* Pa_GetLastHostErrorInfo( PaContext context )
PaHostApiIndex Pa_GetHostApiCount( PaContext context );
PaHostApiIndex Pa_GetDefaultHostApi( PaContext context );
const PaHostApiInfo * Pa_GetHostApiInfo( PaContext context, PaHostApiIndex hostApi );
PaHostApiIndex Pa_HostApiTypeIdToHostApiIndex( PaContext context, PaHostApiTypeId type );
const PaDeviceInfo* Pa_GetDeviceInfo( PaContext context, PaDeviceIndex device );
PaError Pa_OpenStream( PaContext context, 
                       PaStream** stream,
                       const PaStreamParameters *inputParameters,
                       const PaStreamParameters *outputParameters,
                       double sampleRate,
                       unsigned long framesPerBuffer,
                       PaStreamFlags streamFlags,
                       PaStreamCallback *streamCallback,
                       void *userData );
// and so on ...

PaStream would hold PaContext instance, therefore stream managing API, such as PaError Pa_StartStream( PaStream *stream ) will stay unmodified.

Context-less API (current) would stay as legacy API and use a global statically allocated instance of PaContext to provide backwards compatibility with apps not using Context-aware new API.

I can take this task if there is mutual agreement on such enhancement.

[dechamps]

will solve issues similar to #766

As discussed on that issue, I don't think it will. By the time the code gets to call your proposed API, all bets are off and it's already too late.

On the other hand, your proposal might help with thread safety (#808): we could allow concurrent calls to the PortAudio API as long as different contexts are used. I'm not sure how useful that would be in practice, though.

[dmitrykos]

@dechamps when you have 2 contexts created in 2 different processes they will not cross each other, even if application is using 2 different PA versions because there are no static variables which can be referenced from 2 instances. Therefore they will really solve the issue described in #766 without a need to introduce some static variables as semaphores (which are to my view absolutely unreliable) during the initialization stage.

I'm not sure how useful that would be in practice, though.

Per-thread context is indeed very useful as you can keep context per thread. For example, according to my proposal in #808, only thread which created context will be authorized to call PA thread-unsafe API.

[dechamps]

when you have 2 contexts created in 2 different processes they will not cross each other, even if application is using 2 different PA versions because there are no static variables which can be referenced from 2 instances

"Static variables" are only a small part of the problem. Let me take an example:

Let's say you have two PortAudio versions, version 1 and version 2.

PortAudio version 2 exposes a function called Pa_ShinyNewFeature(). This function does not exist in version 1.

Now, let's say you have an application that loads two modules, module A and module B. Both modules come bundled with their own portaudio.dll. (This is the exact scenario that played out in #766.) The one bundled with module A is PortAudio version 1, while the one bundled with module B is PortAudio version 2.

Let's say module A is loaded first, resulting in portaudio.dll version 1 being loaded.

Now module B is loaded. Because there is already a DLL with the name portaudio.dll loaded in the process, module B's version of portaudio.dll does not get loaded; instead, the Windows DLL loader links module B to the existing instance of portaudio.dll, i.e. version 1 from module A.

Now module B tries to call Pa_ShinyNewFeature(), which doesn't exist. Hilarity ensues. (Actually I don't think it will even get to that point - most likely module B will fail to load entirely because of the incomplete import table.)

A similar problem occurs if module B tries to use parameters (e.g. flags) that PortAudio version 1 does not understand, resulting in undefined/surprising behavior from the perspective of module B.

Your proposed fix will not help with this. The only way to fix this is to either (1) ensure module A or module B use different PortAudio DLL names (e.g. portaudio_a.dll and portaudio_b.dll), or (2) adjust module A and/or B to use a side-by-side assembly manifest (example) that instructs the Windows DLL loader to not reuse an existing DLL instance. None of these solutions can be implemented on the PortAudio side; it is up to PortAudio users to set them up.

they will really solve the issue described in #766 without a need to introduce some static variables as semaphores

To be clear, this "semaphore" static variable you are referring to does not "solve the issue". It only attempts to mitigate the blast radius if this problem occurs by degrading more gracefully (instead of crashing). The only way to "solve the issue" is to use one of the techniques I just described.

Per-thread context is indeed very useful as you can keep context per thread

I agree it might be useful, I'm just not convinced applications would really make use of this in practice. Are there really applications that want to run multiple PortAudio instances from multiple threads? Is that really that useful? I don't have strong feelings about this though. I agree that pretty much anything is better than global state.

[dmitrykos]

A similar problem occurs if module B tries to use parameters (e.g. flags) that PortAudio version 1 does not understand, resulting in undefined/surprising behavior from the perspective of module B.

@dechamps it is easy to fix the problem you described on per context basis: PaContext holds PA API version and pa_front.c public functions check whether PA API version matches when context is provided as one of the parameters, otherwise call gets rejected with some error code. Therefore you no longer need any static variable to prevent multiple simultaneous calls, it is automatically solved by per-context approach.

Per-context API does not solve ABI compatibility fully though (which you described), therefore to prevent unknown/incompatible ABI user has to check PA version before any other calls to PA API:

if (Pa_GetVersion() != paMakeVersionNumber(19,5,6))
{
    // fail    
}

A good example of per-context implementation is libusb:
https://github.com/libusb/libusb/blob/fcf0c710ef5911ae37fbbf1b39d48a89f6f14e8a/libusb/libusb.h#L1567-L1571

[RossBencina]

Definitely a breaking change.

[dmitrykos]

Definitely a breaking change.

@RossBencina it can be made non-breaking if context-aware functions bear different names, postfixed with Ctx (context) or Ex (extended). Legacy API would stay with its function names as-is without changes and preserving ABI compatibility fully.

PaError Pa_InitializeCtx( PaContext context )
PaError Pa_TerminateCtx( PaContext context )
const PaHostErrorInfo* Pa_GetLastHostErrorInfoCtx( PaContext context )

or

PaError Pa_InitializeEx( PaContext context )
PaError Pa_TerminateEx( PaContext context )
const PaHostErrorInfo* Pa_GetLastHostErrorInfoEx( PaContext context )