Remove comment stating that thread-safe methods must also be realtime-safe

[micahrj]

I don't think this comment makes a whole lot of sense given the set of existing functions in the CLAP API which are marked [thread-safe]. For instance:

• clap_plugin_entry: get_factory
• clap_host_gui: resize_hints_changed, request_resize, request_show, request_hide, closed
• clap_plugin_factory: get_plugin_count, get_plugin_descriptor, create_plugin
• clap_preset_discovery_factory: count, get_descriptor, create

Ultimately, thread safety and real-time safety are simply different concerns, and any given function can satisfy one, both or neither. E.g., std::mutex::lock is thread-safe but not realtime-safe, and clap_plugin.process is realtime-safe but not thread-safe. I would support the idea of introducing a separate [realtime-safe] annotation, but it doesn't make sense for it to be conflated with [thread-safe].

[baconpaul]

so the intent was: thread safe functions can be called from the audio thread.

i think your change would require us to use another type of marker [thread-safe; !audio]

i agree this would be a useful change to the spec, but simply removing the comment is probably not the complete change.

[micahrj]

There are two kinds of correctness at play here, logical correctness and realtime correctness. The markers in the docs ([thread-safe], [main-thread & !active], etc.) specify requirements for logical correctness, not realtime correctness. The logical requirements have of course been designed to make it possible for both hosts and plugins to be implemented in a realtime-correctness way, but they do not directly specify requirements for realtime correctness.

In this context, the [thread-safe] marker merely implies that it is logically correct to call a given function from any thread, including the audio thread, not that it is necessarily realtime-correct to do so. There's nothing contradictory about this; to repeat my example from above, std::mutex::lock is a function which is logically safe but not realtime-safe to call from the audio thread.

We could go through and replace [thread-safe] with [thread-safe, !audio-thread] on every function for which it doesn't really make sense to require realtime-safety. That would amount to adding a new logical obligation which the callers of those functions would have to uphold (you may not call this function from the audio thread) and which the implementers of those functions could rely on (you may assume this function will not be called from the audio thread). However, I personally think it would make more sense to introduce a separate [realtime-safe] marker instead, which would amount to an obligation for implementers to uphold (you should not block inside this function) and which callers could rely on (you may assume this function will not block).

[baconpaul]

Yes you are correct

we have overlapped audio thread with realtime safe
And thread safe with any thread including the audio thread
Therefore thread safe as specified must be realtime safe

Introducing a realtime safe marker would solve this but it then leaves two questions

is a thread safe function which is not realtime safe safe to call from any thread? No. So we end up with the unsatisfying result that it is not safe to call a thread safe function from a thread

is there any audio thread function which is not realtime safe? No. So do we double up the markers

so we really have a four of things

audio thread
Main thread
Any non audio thread concurrently
Any thread concurrently

and the first and last would require those functions to meet realtime safety

right now thread safe means the fourth and I think you are wanting to add the third category

Not sure what change that would mean for the documentation though

[baconpaul]

Oh and you said

In this context, the [thread-safe] marker merely implies that it is logically correct to call a given function from any thread, including the audio thread, not that it is necessarily realtime-correct to do so.

The sentence you want to remove is the one which makes this false, which is why the sentence is there. The spec as written means a thread safe function is always realtime correct.

[micahrj]

The sentence you want to remove is the one which makes this false, which is why the sentence is there. The spec as written means a thread safe function is always realtime correct.

Right, I want to remove the sentence, because requiring clap_host_gui.request_resize and clap_plugin_factory.create_plugin to be realtime-safe is both unnecessary and onerous and is probably not respected in practice by most existing hosts or plugins.

[micahrj]

I'm proposing three categories of function with regard to logical correctness:

• May only be called on the audio thread ([audio-thread])
• May only be called on the main thread ([main-thread])
• May be called from any thread ([thread-safe])

And then, separately, I'm proposing two categories of function with regard to realtime correctness:

• Realtime-safe, does not block ([realtime])
• Non-realtime, may block

In most cases, any function marked [audio-thread] would also be marked [realtime] (and some small subset of [thread-safe] functions would also be marked [realtime]). However, the categories already don't perfectly overlap, as when rendering in CLAP_RENDER_OFFLINE mode, functions that are marked [audio-thread] are not required to be realtime-safe, but callers are still required to uphold the threading requirements.

[baconpaul]

The sentence you want to remove is the one which makes this false, which is why the sentence is there. The spec as written means a thread safe function is always realtime correct.

Right, I want to remove the sentence, because requiring clap_host_gui.request_resize and clap_plugin_factory.create_plugin to be realtime-safe is both unnecessary and onerous and is probably not respected in practice by most existing hosts or plugins.

Yeah I get that (although Im not sure I agree on the first - I see no reason a midi event couldn’t make a plugin request a result: even though I would bounce to the ui thread for that myself ) but just removing the sentence causes other problems. Which is what we are trying to solve!

[baconpaul]

But it seems the solution is really to either

1. have a realtime safe tag which thread safe is not so we augment every thread safe with a thread safe realtime safe. That’s a bit wonky from a documentation perspective since it means an api marked thread safe is not safe to call from every thread
2. Have a non realtime safe tag which augments thread safe so you have thread safe ! Realtime and then adjust the documentation to say “unless expressed otherwise thread safe is realtime safe”. From a read the documentation perspective this is better I think since there’s less implied knowledge and the common sense meaning of the phrase “thread safe” on its own is preserved

wdyt?

[baconpaul]

Another alternative would be to do away with thread safe altogether and use a combo of main thread audio thread concurrent safe and realtime safe on the apis which you could convince me of I bet too!

[baconpaul]

I'm proposing three categories of function with regard to logical correctness:

• May only be called on the audio thread ([audio-thread])
• May only be called on the main thread ([main-thread])
• May be called from any thread ([thread-safe])

And then, separately, I'm proposing two categories of function with regard to realtime correctness:

• Realtime-safe, does not block ([realtime])
• Non-realtime, may block

In most cases, any function marked [audio-thread] would also be marked [realtime] (and some small subset of [thread-safe] functions would also be marked [realtime]). However, the categories already don't perfectly overlap, as when rendering in CLAP_RENDER_OFFLINE mode, functions that are marked [audio-thread] are not required to be realtime-safe, but callers are still required to uphold the threading requirements.

So the thing I just can’t get over here is

Q: “hey this function is marked thread safe what does that mean”
A: “It means you can’t call it from the audio thread”

you see why that’s a problem right?

[baconpaul]

I’m becoming increasingly convinced that removing thread safe altogether and replacing it with concurrent safe and realtime safe is the way to go (with the implication that no api point is concurrent unless so marked and all audio thread are realtime safe unless marked)

[baconpaul]

Basically our tags aren’t orthogonal enough

Ugh you’ve raised a good problem even though I’m not crazy about your answer

[micahrj]

The reason I'm in favor of [thread-safe] and [realtime-safe] markers is that both of them would more or less correspond to the standard, well known definitions of those terms: "thread-safe" means "you can call this function from any thread concurrently without causing race conditions or data corruption," and "realtime-safe" means "this function will not block the thread for an unbounded amount of time." I also think it's natural for the default assumption to be that a given function has neither of those properties unless explicitly marked.

However, I can appreciate your argument that having [thread-safe] mean, in practice, "you can call this on any thread (except the audio thread) (unless rendering offline)" is insufficiently obvious in isolation. Personally, I don't think that renaming "thread-safe" to "concurrent-safe" would really change much about that. I think the alternative would be to have a flat list of threading markers so that each function has exactly one, e.g. (with better names than these, obviously):

• [audio-thread]
• [main-thread]
• [any-thread-but-audio-thread]
• [any-thread-including-audio-thread]

[baconpaul]

• [audio-thread]
• [main-thread]
• [any-thread-but-audio-thread]
• [any-thread-including-audio-thread]

Yeah I like this approach but I think it removes some of the 'concurrency' safeness implicit in thread safe.

• [audio-thread]
• [main-thread]
• [concurrent-non-realtime]
• [concurrent]

is i think what we mean. Or heck maybe even

• [audio-thread]
• [main-thread]
• [thread-safe-non-realtime]
• [thread-safe]

where thread-safe implies any thread so realtime may be in play.

[micahrj]

I am against having [thread-safe] by itself mean "you can call this on any thread (and also this method must not block)." I think that's exactly as confusing as having [thread-safe] by itself mean "you can call this on any thread (except the audio thread because it might block)".

[baconpaul]

yeah

the problem with any thread is it doesn't imply concurrency as an option necessarily but i guess we can document that

any-nonrealtime-thread-concurrently and any-thread-concurently is a bit much

hmm.

like i said the problem you are trying to fix is correct. i'm just not sure we are stumbling exactly to the solution

wonder if @abique @defiantnerd @Bremmers and other reviewers have some thoughts too

[messmerd]

As I mentioned in free-audio/clap-helpers#76, Clang 20 is introducing a RealtimeSanitizer with [[clang::nonblocking]] and [[clang::blocking]] attributes which may be a great alternative to "[realtime]" and "[non-realtime]" comments because in addition to documenting a function's real-time requirements, they can also be used to check for real-time violations at both compile-time and runtime.

[baconpaul]

realtime is also non-allocating, no io, etc... of course! does nonblocking include those? can you malloc in a tagged file?

but those sound super useful

[messmerd]

@baconpaul The documentation is here and here.

realtime is also non-allocating, no io, etc... of course! does nonblocking include those?

Yes, [[clang::nonblocking]] implies [[clang::nonallocating]] as seen here.

And here's the full set of constraints.

It offers a lot of customizability for people using it - custom error handlers, several different runtime options, and various ways of suppressing errors for individual functions, function calls, or translation units at compile time or runtime.

RtSan is not enabled by default - If CLAP API functions were marked with [[clang::nonblocking]] or [[clang::blocking]], users would need to opt-in to runtime checking by compiling with -fsanitize=realtime. I'm not sure whether compile-time static analysis is disabled without that flag, but we should probably require a macro defined for CLAP anyway - i.e. CLAP_USE_RTSAN to apply the attributes across the API.

One thing I'm unsure of is whether there will be issues using the CLAP API in a C++ program when the API functions are not marked with noexcept. I'll have to look into that.

[baconpaul]

Wow that sounds really useful