Best practise on sharing pointers amongst classes (std::shared_ptr vs the rest)

I am making a large project: a mobile DAW with a few hundred classes and some of the most important classes need to have their pointers shared around the place. These important classes need to be able to hold pointers to each other, and even to hold pointers to themselves so that they can pass that pointer along to their client classes to have access to their functionality. It is a very complex web of classes.

So, I have mostly been using std::shared_ptr as a means of sharing these pointers around, but this can lead to problems as the app is shutting down: Class A cannot call its destructor because Class B is holding a shared_ptr of Class A. And Class B will not shut down and destroy its pointer to Class A because Class A is still holding a share_ptr of Class B.

My current solution is to create a shutdown() method on each class, where it will destroy all the pointers it owns. But this is an imperfect solution, especially as my company grows and the project expands rapidly. Anybody willing to share their methods of dealing with similar issues? I am considering using Juce::WeakReference instead of std::shared_ptr so that I can take control of when my pointers get deleted.

WeakReference will definitely be your friend. Weak references are used extensively in native Objective-C code (which uses reference counting for all objects) specifically to avoid this problem, which Apple refers to as a retain cycle. If some object is owned by another object, it should definitely not have a strong reference to its owner.

However, I would encourage you to re-evaluate your code design - bidirectional references in a hierarchical (or semi hierarchical!) structure might be a sign of some good opportunities to factor out data lower into the hierarchy such that children don’t necessarily need to know about their parents. Obviously I can’t see your code so I can’t really judge, if it’s managed to scale to a few hundred classes and it’s just starting to become a problem you must be doing something right. :slight_smile:

3 Likes

Thanks man. Yeah its sounding like WeakReference is the way to go.

I have thought about re-evaluating the design. The problem is that OOP is both my friend and my adversary. On the one hand, it is great to be able to organise code based on its purpose. So, for example, methods for managing the audio tend to be in one place while methods of managing the database tend to be another place. But there are some processes that need access to both audio and database. And I need a mechanism for delivering the pointers to where they are needed. So, my major classes end up becoming like major train stations where all the trains (i.e. pointers) meet. The only alternative I can think of, is holding every important pointer in a static object that has only that job as its purpose: to hold onto pointers.

Know dat feel. Two approaches we have tried are:

() Hierarchical listening. Parent is a listener to child. Parent creates child, then child.addListener(this). Messages are passed up the branch, and if they need to be passed to another branch, they are first sent back to the root.
This approach still feels messy and is very laborious to maintain / tweak but seems to be more or less necessary for trees of view components.
() Singleton. Whether it’s a AudioProcessorValueTreeState or a custom singleton, this could be your switchboard. There is a two-way listener / broadcaster relationship between this object and just about every significant class in your programme.

Would love to hear more from experienced developers about helpful design patterns for JUCE.

I found @dave96’s talk on ValueTree object management to be a game changer for me. Any time I’m writing any sort of non-trivial audio app I follow this pattern, and it’s kept my code tremendously tidy while giving me tons of features for free and making inter-object communication a breeze.

The way I usually structure things is to have the ValueTree-reflected objects owned and managed by the UI thread, then any time something like a MIDI sequence or track changes I automatically regenerate a run-time model (think dumping a ValueTree of a bunch of MIDI notes to a MidiMessageSequence object) to be read by the audio thread at playback time.

It sounds like @DrTarantism’s object model is well past the point of refactoring into something like that though…

3 Likes

beware that juce WeakReference is not thread safe despites the use of an atomic reference counted object… don’t forget that when you design your classes if you plan passing references among different threads.

Hmmm… that’s a very important issue. I didn’t think about that. I think I’ll have to spend some time looking into Singleton’s and ValueTrees. I’ve never used either of them before. From what I’m reading online Singletons can be made threadsafe.

Just to add to all the good advice in this thread already, are you aware that std::shared_ptr already has support for std::weak_ptr? This works slightly differently to JUCE’s intrusive WeakReference but it means you basically get weak referencing for free if all of your lifetime’s are managed by std::shared_ptr.

The other main draw of std::weak_ptr is that it enables you to get a std::shared_ptr safely back out so you can use an object without fear of it being deleted underneath you.

One other thing to remember with the std ptrs is that although it’s thread safe to create/destroy them (i.e. increment/decrement the reference count), it’s not thread safe to assign them. You’ll need std::atomic_exchange for that until we get std::atomic<std::shared_ptr> in C++20.


More generally, I’m obviously a huge advocate of using ValueTrees for your data model and keeping everything message thread related. As @jonathonracz pointed out, you probably want a different playback graph that keeps any shared data minimal. This tends to be the easiest and safest way of doing things.

Also, if possible, try to avoid Singleton, there’s places where they can help reduce code bloat but if you’re reaching for them, be aware of the pitfalls and treat them as a last resort.

3 Likes

I’m confused. Isn’t the creation of a shared_ptr the same as assigning it? Or do you mean to say that swapping the pointer out for another pointer is not thread safe?

Use std::weak_ptr to break the cycle.
But in general, you should try to have a hierarchy of ibjects and have a unique owner of objects. If the top object owns the object, then it can safely be shared among all children by pointer or even ref. It will also speed dereferencing, so simplifying the ownership management is a win for speed and lifetime understanding (I don’t count the number of time I wished a real architect designed some complex objects that ended up using shared pointers instead of unique pointers and the code could not be changed anymore despite obvious misuse of shared pointer).

1 Like

I probably didn’t phrase that very well.
Mutating a std::shared_ptr (i.e. the actual “ptr” bit in std::shared_ptr<Obj> ptr) is not thread safe. So you can’t assign one between threads without some kind of synchronization.

However, the control block has atomic reference counts within it, so an object owned by a std::shared_ptr is guaranteed to only be deleted by a single thread.

This will probably explain it better:
http://www.modernescpp.com/index.php/atomic-smart-pointers

OK cool thanks

Hi
I assume if you have a ValueTree in your root Parent class, that passing by reference to Children is ok, as the parent owns both the child and the ValueTree? Isn’t ValueTree is a reference system itself? How would one manage passing the associated Undomanager?
Cheers

A ValueTree consists of a stack object, that contains the listeners, and inside a so called SharedObject, which is a ReferenceCountedObject.

The UndoManager is not problematic at all, each change results in a UndoableAction, that is stored in a list. Since the undoableAction keeps a reference to the SharedObject, it can undo the change on the reference. Even if the UndoManager would be the last to hold a reference, it would still work, just that the ValueTree is discarded right afterwards.

Thank you.
So is

 arrayOfType.add ( new Type() );

in a non-owned still ok practice?
Thank you again

Yes, I would say so.

As a ReferenceCountedObject the SharedObject has no single point of ownership.
If you add a ValueTree in any Array, you increment the refcount, so it will be kept alive, as long as at least one reference is alive.

The only two things to keep in mind:

  • the ListenerList is not part of the SharedObject
  • if the last reference goes out of scope in the audio thread, a deallocation will happen in that thread

THANK YOU

Hi
Excuse my pestilence, but how would one manage

  {
            XmlElement*  basicLayoutPTR =  tree.getOpennessState(true);
            .......some code using *basicLayoutPTR in this scope only
  }

using the latest practices.

{
    std::unique_ptr<XmlElement> basicLayoutPTR (tree.getOpennessState(true));
    .......some code using basicLayoutPTR->, basicLayoutPTR.get(), or (*basicLayoutPTR) in this scope only
}

I think that should work, at least on C++14 and above.

:+1: and you could even write const std::unique_ptr<XmlElement> basicLayoutPTR, but decide for yourself whether it’s worth the extra characters :blush:

1 Like