Value and Listeners


#1

I Couldn't work out why my Value listener wasn't getting calle. But turns out that copying the Value object doesn't actually copy the listeners. (I was putting the listener on a value in a struct and then putting the struct in a std::vector). 

Anyway - skimming the source it looks like the move operation on Value won't copy the listeners either. I assume that'll corrupt my stuff if my vector expands.  

Though it's a bit irrelevant because changing the capacity on the vector makes ~Value() dereference a null pointer. An assert in ReferenceCountedObjectPtr triggers. I've not figured out quite why yet.

Anyway - I"m going to put the structure on the heap and have a a pointer to it to avoid all this.  

I'm wondering if it'd be better if the Value couldn't be copied at all!  Though bound to break stuff. 

So big(ger) warnings in the documentation instead? 

 

 

 


#2

It's done that way deliberately - I think I originally wrote it to copy the listeners, but quickly realised that that caused all kinds of mayhem. I've used it in many different places, and found it's much better if listeners are only held locally.

A pattern I use a lot is this:

struct Foo : Value::Listener
{
    Foo (Value& v) : value (v) { value.addListener (this); }
    ~Foo()  {}  // no need to remove the listener

    Value value;
};

 


#3

I agree the local listener thing is great.  I use exactly the same pattern everywhere.  The problem is that Value allows a copy in a non-standard way which makes these two examples fail: 

class Run :

public Value::Listener {

public:

    struct Entry {

        Value v;

    };

    

    Run() {

        /* This doesn't result in any listeners. */

        for (int i = 0; i<20; ++i) {

            Entry e;

            e.v = 100;

            e.v.addListener(this);

            data.push_back(e);

        }

        data.front().v = 101;


        /* This triggers an assert in ReferenceCountedObjectPtr. */

        for (int i = 0; i<50; ++i) {

            Entry d;

            d.v = 100;

            data.push_back(d);

            data.back().v.addListener(this);

        }

    }

    void valueChanged(Value & v) override {

        /* This is never reached. */

        DBG ("value changed to " + v.toString());

    }

private:

    std::vector<Entry> data;

};

The first one just fails silently and is a pain to spot what's happening.  The second risks being a serious run-time bug because it occurs when the vector is resized, so won't show up with small data sets. 

I'm trying to figure out exactly what's happening there...it's all gone a bit STL in the stack-trace!

--- 

EDIT: Looks like it's possible for

    if (listeners.size() > 0)

        value->valuesWithListeners.removeValue (this);

listeners.size() to be non-zero but value to be nullptr.


#4

EDIT: Looks like it's possible for

    if (listeners.size() > 0)

        value->valuesWithListeners.removeValue (this);

listeners.size() to be non-zero but value to be nullptr.

Maybe I'm being thick, but I can't see how that could happen? The value member can never be null.


#5

Well, it's the referenceObject which is null...? 

${ansi.fg.red}lldb$ ${ansi.normal}print value

(juce::ReferenceCountedObjectPtr<juce::Value::ValueSource>) $0 = {

  referencedObject = 0x0000000000000000

}

I'll step through it and see if I can make sense of what's happening.  Suspect it's a move thing.  You use ReferenceCountedObject pointers move which sets the value to nullptr, then destructor of Value is called which still has listeners and tries to deference the value...


#6

Yep.  

        {

            Value a;

            a.addListener(this);   

            Value b = std::move(a);

        }

Crashes.


#7

Suggest removing listeners during move.  And add an assertion if it has listeners, because moving an object with listeners is probably very rarely wanted.  That will go some way towards fixing my first problem of disappearing listeners.

 


#8

Yes, I was being thick!

Ok, try again now..


#9

Cheers Jules.  Update does not crash and comes up with the warning. 

There's still a good run-time bug to be had if testing doesn't make the vector big enough to resize one of the elements with a listener, but it's a lot safer than it was!