[suggestions] Best practice for threading and locking

Creating threads is very light, and starting them is somewhat more, but still not very expensive.

Telling your thread you have new data might be more expensive than starting a new one.

The simplest solution is a new thread for each computation.

If you have lots of tiny calculations you want to run on one specific thread, then the standard solution is to use a thread-safe queue that blocks until there’s a new calculation!

Great, I will transform my code to implement this approach!

But still I think the problem will persist…

A StepView class creates and fires a thread, then the StepView instance gets deleted, the thread owned by that instance lives on… does its calculations and when finished hands of its calculated buffer back to the StepView instance, but the StepView instance no longer exists…

I believe this is the core problem why my plugin crashes, but I don’t really know how to fix it.

One solution is to be allocating StepView using std::shared_ptr and then take a weak_ptr to that in the Thread.

When the thread is ready to deliver, it tries to turn the weak point into a strong pointer, and if that fails, it knows that the StepView has been deleted.

If you don’t want to change your memory allocation strategy, very reasonable not to want to, then you create a tiny class Shim that either accepts the calculation callback and passes it bacik, or throws it away if it’s turned off - then you hand out shared pointers to it.

Then in the StepView destructor, you turn off the Shim and remove the reference to yourself in it.

That sounds like a great idea!
TBH I am willing to reprogram what ever it takes to get rid of this crash, so memory allocation strategy included.

I have a single main DataModel class instance which owns a std::vector < shared_ptr< StepView >>. This is the list which gets deleted and constructed whenever I call a preset button. So my StepView class instances are definitely allocated (already) using std::shared_ptr. The way it is programmed now however, is that a stepView-thread class gets constructed with a StepView& as a constructor argument (it owns a StepView& reference). If I understand correctly I should change this type from StepView& to std::weak_ptr and pass a std::shared_ptr when initialising this member value in the constructor of the thread instance.

So should I use the shared_from_this pattern to transform the StepView into a shared_ptr when making the thread? Or how does one accomplish this?

Thank you!

I’m talking in a void here because I haven’t seen your code. (My email address is tom@swirly.com so you could just send me a URL to the code. :-D)

It is my belief that that strategy would work but it might be that you could do quite a bit better.

C++ is designed to be “close to the metal”. Shared pointers are certainly much cheaper than, say, a Python object but they are much more expensive than a raw or unique_ptr, and those are more expensive than “having the object itself”.

shared_ptr also introduces the possibility of bugs - circular references, but also the possibility that an object might get deleted on a different thread than the one that it was created on, which I have seen cause intermittent catastrophe on more than one project.

(Or not. It might be totally OK to destroy on a different thread. C++/C memory management itself is thread-safe.)

You have some structure with lots of parts, each of which is a separate smart pointer. Gnarly! What if some are deleted and others aren’t?

What I’m getting to is this: I think your basic idea of using weak pointers is perfectly correct, but perhaps it’s better to use std::shared_ptr<std::vector>?

I have a general rule to “bring my smart pointers up as high as possible” - just to have a single shared pointer at the very top if possible. It makes the rest of the code much easier and more performant (“faster”, uses less memory and generates less code).

I haven’t read back all answers in detail, but instead of creating threads, I would suggest to use existing thread classes:

For ongoing jobs (like prebuffering) implement TimeSliceClient and have TimeSliceThreads somehwere in the owning structure, so the clients are always gone by the time the threads are about to be destroyed.

For terminating jobs use ThreadPoolJob. The ThreadPool can delete the finished job automatically, or you keep them owned somewhere else, if you need to access the results (although I usually supply the ThreadPoolJob with a reference, where it can put the results safely).

Thnx Tom, I wrote you an email :slight_smile:

@daniel That sounds amazing! A tutorial on this would be so awesome. since this is all new to me…
Would be nice if you could do it on the audio programmer’s yt channel! (just an idea :slight_smile: )