Memory corruption in std::vector

Hi, I’m tryng to fill a vector with elements of costum class “Circle”, but when I try to execute the constructor of “RaceLight”, which calls the vector’s push_back function, I have a memory corruption error. However, if “RaceLIght” doesn’t inherit from the Juce class “timer”, I don’t have this problem.
Here’s the error and the Source files: , RaceLight and Circle:

Error in `/home/ilaria/TNDSproject/Builds/LinuxMakefile/build/NewProject’: malloc(): memory corruption: 0x0871d528 ***

#ifndef CIRCLE_H_INCLUDED
#define CIRCLE_H_INCLUDED

#include “JuceHeader.h”

class Circle : public Component
{
public:
Circle(unsigned int);
~Circle();

void paint (Graphics&);
void resized();
void SetColour(unsigned int c);

private:

Colour colour;

//=========================================================================

JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (Circle)

};

#endif // CIRCLE_H_INCLUDED

#include “Circle.h”

//==============================================================================
Circle::Circle(unsigned int c):colour©
{
// In your constructor, you should add any child components, and
setSize (600, 400);
// initialise any special settings that your component needs.

}

Circle::~Circle()
{
}

void Circle::paint (Graphics& g)
{
/* This demo code just fills the component’s background and
draws some placeholder text to get you started.

   You should replace everything in this method with your own
   drawing code..
*/
  // clear the background
g.setColour (colour);
g.fillEllipse (0,0,getWidth(),getHeight()); 

}

void Circle::resized()
{
// This method is where you should set the bounds of any child
// components that your component contains…

}

void Circle::SetColour(unsigned int c)
{
colour=Colour©;
repaint();
}

class RaceLight : public Component,
private Timer
{
public:
//==============================================================================
RaceLight (int N_lights);
~RaceLight();

//==============================================================================
//[UserMethods]     -- You can add your own custom methods in this section.
void light();
//[/UserMethods]
void paint (Graphics& g);
void resized();

private:
//[UserVariables] – You can add your own custom variables in this section.
int NumberOfLights;
int TimeOfLight=1000;
vector <ScopedPointer> RedLight;
vector <ScopedPointer> GreenLight;
void timerCallback() override;
//[/UserVariables]

//==============================================================================
//==============================================================================
JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (RaceLight)

};

//[EndFile] You can add extra defines here…
//[/EndFile]

#endif // JUCE_HEADER_531018019E54512

#include “RaceLight.h”

//[MiscUserDefs] You can add your own user definitions and misc code here…
//[/MiscUserDefs]

//==============================================================================
RaceLight::RaceLight (int N_lights)
: NumberOfLights(N_lights)
{
//[Constructor_pre] You can add your own custom stuff here…
RedLight.reserve(NumberOfLights);
GreenLight.reserve(NumberOfLights);
for(int i=0;i<NumberOfLights;++i){
RedLight.push_back(new Circle(0xffb1082a));
GreenLight.push_back(new Circle(0xff09240a));
addAndMakeVisible(RedLight[i]);
addAndMakeVisible(GreenLight[i]);
}

Thank you very much!

Could you use JUCE OwnedArray<> rather than vector<ScopedPointer> instead?

You have to give ScopedPointer a template argument:

ScopedPointer<Circle> or ScopedPointer<Component>

I suspect that it is vector<ScopedPointer<Component>> or vector<ScopedPointer<Circle>> in the OP original code as it wasn’t formatted as preformatted text in the post.

The problem is related to this:

I think you must have been “lucky” when it worked the first time. Do something like this:

class MainContentComponent   : public Component
{
public:
    //==============================================================================
    MainContentComponent()
    {
        comps.add (new Slider());
        comps.add (new Slider());
        comps.add (new TextButton ("Button 1"));
        comps.add (new TextButton ("Button 2"));

        for (auto comp : comps)
            addAndMakeVisible (comp);
        
        setSize (600, 400);
    }
    
    ~MainContentComponent()           { }
    void paint (Graphics& g) override { g.fillAll (Colours::grey); }
    
    void resized() override
    {
        Rectangle<int> r = getLocalBounds();
        
        for (auto comp : comps)
            comp->setBounds (r.removeFromTop (30).reduced (4));
    }
    
private:
    OwnedArray<Component> comps;
};

If you want to use std containers then something like this is what you want:

class MainContentComponent   : public Component
{
public:
    //==============================================================================
    MainContentComponent()
    {
        comps.emplace_back (new Slider());
        comps.emplace_back (new Slider());
        comps.emplace_back (new TextButton ("Button 1"));
        comps.emplace_back (new TextButton ("Button 2"));

        for (auto& comp : comps)
            addAndMakeVisible (comp.get());
        
        setSize (600, 400);
    }
    
    ~MainContentComponent()           { }
    void paint (Graphics& g) override { g.fillAll (Colours::grey); }
    
    void resized() override
    {
        Rectangle<int> r = getLocalBounds();
        
        for (auto& comp : comps)
            comp->setBounds (r.removeFromTop (30).reduced (4));
    }
    
private:
    vector<unique_ptr<Component>> comps;
};

Notice:

  • use emplace_back() — to move the unique_ptr into the vector
  • notice the auto& in the iterators (athough using operator[] returns a reference too)
1 Like

I tried to use your advice but it didn’t work. These are the changes I made:

class RaceLight  : public Component,
               private Timer
{
public:

RaceLight (int N_lights);
~RaceLight();


void light();

void paint (Graphics& g);
void resized();



private:
    int NumberOfLights;
    int TimeOfLight=1000;
    vector <unique_ptr<Circle>> RedLight;
    vector <unique_ptr<Circle>> GreenLight;
    void timerCallback() override;
    
    JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (RaceLight)
};

    RaceLight::RaceLight (int N_lights)
    : NumberOfLights(N_lights)
{

   for(int i=0;i<NumberOfLights;++i){
        RedLight.emplace_back(new Circle(0xffb1082a));
        GreenLight.emplace_back(new Circle(0xff09240a));
   }
    for(auto& num:RedLight)
        addAndMakeVisible(num.get());
    for(auto& num:GreenLight)
        addAndMakeVisible(num.get());
 
    setSize (600, 200);
}

I tried also OwnedArray but it was the same.

Well the only thing you know if you have “memory corruption” is that some time earlier, somewhere in your code a memory corruption bug occurred. From the info you posted there’s no way of knowing where and when, and it may be not related to your RaceLight class at all.

I don’t think the ScopedPointer behaves like an std::unique_ptr. Look at the assignment operator: the ScopedPointer has an overload accepting a lvalue reference (ScopedPointer &other), which implements a move instead of an assignment. The unique_ptr only accepts an rvalue (as in unique_ptr &&other) which also implements a move, but that is expected in this case.

My guess is that, since it predates C++11, it behaves like a std::auto_ptr, and it shouldn’t be used in standard containers at all, not even with emplace_back.

1 Like

Well, if you get the same problem using OwnedArray then the corruption problem is something else.

Could you reformat the code in your original post so it’s preformatted?

Sure!

class Circle    : public Component
{
public:
    Circle(unsigned int);
    ~Circle();

    void paint (Graphics&);
    void resized();
    void SetColour(unsigned int c);


private:
    
    Colour colour;
    
    //=========================================================================
    
    JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (Circle)
  
};


Circle::Circle(unsigned int c):colour(c)
{
setSize (600, 400);
}

Circle::~Circle()
{
}

void Circle::paint (Graphics& g)
{
   

g.setColour (colour);
g.fillEllipse (0,0,getWidth(),getHeight()); 

}

void Circle::resized()
{}

void Circle::SetColour(unsigned int c)
{
colour=Colour(c);
repaint();
}

`class RaceLight  : public Component,
                   private Timer
{
public:

    RaceLight (int N_lights);
    ~RaceLight();
    void light();
    void paint (Graphics& g);
    void resized();



private:
   
    int NumberOfLights;
    int TimeOfLight=1000;
    vector <unique_ptr<Circle>> RedLight;
    vector <unique_ptr<Circle>> GreenLight;
    void timerCallback() override;
    
    JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (RaceLight)
};
    RaceLight::RaceLight (int N_lights)
        : NumberOfLights(N_lights)
    {
       
       for(int i=0;i<NumberOfLights;++i){
            RedLight.emplace_back(new Circle(0xffb1082a));
            GreenLight.emplace_back(new Circle(0xff09240a));
       }
        for(auto& num:RedLight)
            addAndMakeVisible(num.get());
        for(auto& num:GreenLight)
            addAndMakeVisible(num.get());
      
        setSize (600, 200);

    }
`

What are you doing in the timerCallback()?

timerCallback and light should light a RedLight’s circle every second and then light all the GreenLight’s circles.

void RaceLight::timerCallback()
        {
            RedLight[NumberLightsOn]->SetColour(0xffff0000);
            ++NumberLightsOn;
            repaint();
            if (NumberLightsOn==NumberOfLights)
                stopTimer();

        }
void RaceLight::light() {
    startTimer(TimeOfLight);
    for (int NumberGreenLightsOn=0; NumberGreenLightsOn<NumberOfLights;++NumberGreenLightsOn){
        GreenLight[NumberGreenLightsOn]->SetColour(0xff008000);

    }
}

I’ve solved the problem: there was an error in the parent component. Thank you for your help!

One thing to mention, you should really be using std::make_unique rather than those raw new operators when allocating the std::unique_ptr.
Example:

myVec.emplace_back(std::make_unique<Circle>(args))

…assuming C++14 and later, not C++11