Re: AOO plugin for PD on SGI Irix
Christof Ressi via Pd-list <[email protected]>
| Newsgroups | gmane.comp.multimedia.puredata.general |
|---|---|
| Message-ID | <[email protected]> |
On 8/12/2026 2:00 AM, Wolfgang Gaggl via Pd-list wrote:
> I would not expect #3 to follow from #1. #1 just means I have to try a few times due to system architecture, but a scoped_shared_lock is still a reader lock. So they should work simultaneously, you should not have to wait for a scoped_shared_lock to release the lock before the shared_lock(try_to_lock) can obtain a reader lock as long as there is no writer requesting a lock at the same time.
> Whether it's a bug or not, it does not behave as I would expect it here.
Aha, so you are saying that the shared try lock would not succeed at all
while another thread holds a shared lock? That would be very bad indeed.
>
> write_guard (always non-blocking):
>
> std::atomic<uint32_t> update_gen_{0}; // seqlock generation: even=idle, odd=write in progress
>
> struct update_write_guard {
> std::atomic<uint32_t>& gen_;
> explicit update_write_guard(std::atomic<uint32_t>& g) : gen_(g) {
> g.fetch_add(1, std::memory_order_release);
> }
> ~update_write_guard() {
> gen_.fetch_add(1, std::memory_order_release);
> }
> update_write_guard(const update_write_guard&) = delete;
> update_write_guard& operator=(const update_write_guard&) = delete;
> };
>
> Before every write lock call for update_mutex_:
> update_write_guard uwg(update_gen_);
> scoped_lock lock(update_mutex_); // writer lock!
>
> Replace reader lock try_to_lock in process() of source.cpp with this:
> auto gen_before = update_gen_.load(std::memory_order_acquire);
> if (gen_before & 1) {
> // a write is currently in progress
> LOG_DEBUG("AooSource: process blocked by active writer");
> add_xrun(nsamples);
> return kAooErrorIdle;
> }
> // we are good to go now!
> // do processing stuff here....
>
> // done with processing stuff.
> // Verify no write occurred while audio was being processed.
> if (update_gen_.load(std::memory_order_acquire) != gen_before) {
> LOG_DEBUG("AooSource: write detected during audio processing, xrun");
> add_xrun(nsamples);
> return kAooErrorIdle;
> }
> return kAooOk;
>
> Similar in sink.cpp.
>
> This assumes that typically there's generally low probability of writer lock activity going on during process() execution.
> Works well so far (at least for PD objects).
This doesn't look right. First, the seq lock only works with a single
writer, so the scoped_lock must come before the 'update_write_guard' to
prevent concurrent access. Also, the 'update_write_guard' does not look
correct. You can't write a seqlock with only acquire-release operations,
you need full barriers to prevent the compiler from reordering the
instructions in any direction. See for example:
https://github.com/rigtorp/Seqlock/blob/master/include/rigtorp/Seqlock.h
But most importantly, a seqlock can only be used for reading/writing
trivial objects in memory because there can be data races and those must
be "safe". In your case, you are accessing all kinds of shared data
structures in the process() method after the first load() succeeds, but
nothing prevents another thread from concurrently modifying the data and
wreaking havoc.
Your code only works as long as all writers run on the same thread as
the process() method. But then why do you need the seqlock in the first
place? You can just get rid of the try_lock and call it a day. However,
this is only true for [aoo_send~]. In [aoo_receive~] the network thread
might actually take a writer lock, see source_desc::handle_start.
If you want a "proper" fix, you can replace the shared_mutex with a
shared_spinlock. To avoid burning CPU cycles on contention, you can
replace the blocking shared|unique locks with shared|unique try locks in
a loop with some backoff. Since atomic RMW operations might fail
spuriously on your platform, you would need a bounded retry-loop in the
process() method, like you had before.
Christof
> ---
> [email protected] - the Pure Data mailinglist
> https://lists.iem.at/hyperkitty/list/[email protected]/message/BQSSH5QXUGFTNDVRY6NB7PSICXMVZ7OH/
>
> To unsubscribe send an email to [email protected] mailing list
> UNSUBSCRIBE and account-management -> https://lists.iem.at/
>
---
[email protected] - the Pure Data mailinglist
https://lists.iem.at/hyperkitty/list/[email protected]/message/GSRPG225B6ATTAYMZHWB5VGDTZVRXR33/
To unsubscribe send an email to [email protected] mailing list
UNSUBSCRIBE and account-management -> https://lists.iem.at/