ScopedPointers can delete the pointer value twice

General JUCE discussion
1

ScopedPointers can delete the pointer value twice. Here is a fairly contrived example that reproduces the problem as simply as possible.

struct Thingy
{
    ~Thingy();
    int cnt = 0;
};

ScopedPointer<Thingy> thingy;

Thingy::~Thingy()
{
    cnt++;
    if (cnt == 2)
        printf ("Deconstructors should only be called once!\n");
    
    thingy.reset();
}

int main (int argc, char* argv[])
{
    thingy.reset (new Thingy());
    thingy.reset();
    
    return 0;
}

The issue is that while void reset (ObjectType* newObject) moves the pointer to a temp variable and then deletes it, to guard against reentrant calls to reset, void reset() does not have this guard in place. I think it should be written like this:

    void reset()
    {
        auto* oldObject = object;
        object = {};
        ContainerDeletePolicy<ObjectType>::destroy (oldObject);
    }

Ok, my example is a terrible / bizarre thing to do, but you can get into this situation pretty easily if your deconstructors call listeners. If this really is a bad thing to do then it should give an assert and not crash.

Example project here: https://github.com/FigBug/juce_bugs/blob/master/OSP_Crash/Source/Main.cpp

3 Likes
2

Oh - good catch! I’ll sort that out.

(Best to migrate everything to std::unique_ptr though!)

3

Thank you for pinpointing this issue - I’ve had occasional crashes due to this but couldn’t figure out what exactly went wrong.

4

Is this problem the result of that ScopedPointer not being owned by any particular object or being given the scope of some function or other object, and just being a free global?

Thingy::~Thingy()
{
    cnt++;
    if (cnt == 2)
        printf ("Deconstructors should only be called once!\n");
    
    thingy.reset();
}

the Thingy destructor is resetting a global variable via thingy.reset().