Access violation error: object is deleted twice

  • Thread starter Boogie El Aceitoso
  • Start date
B

Boogie El Aceitoso

Hi,

The code below produces an access violation error, complaining that an object
is destroyed twice.

I don't understand why this happens. Any help would be appreciated.

--------main.cpp---------------------------------------------------------------------------
#include "AbstractActivator.h"


// 3 classes involved:
// TextDisplayer: Base class for a hierarchy of functors that
// display a string in a certain way.
//
// Activator: Base class for a hierarchy.
//
// ActivatorFactory: Base class of a hierarchy of functors
// taht return a pointer to a std::list<Activator>
//
int main()
{

// If I don't create any intances of
// TextDisplayer, I don't get any errors
TextDisplayer show;


// ActivatorFactory is a functor that returns
// a pointer to an stl::list<Activator>
// Each Activator instance contains a pointer
// to a TextDisplayer instance. This internal instance
// is deleted in the Activator destructor.
ActivatorFactory fact;

// Activartors is a typedef for stl::list<Activator>
Activators * acts = fact();
Activators::iterator i;

// Call the activate method of all activators
for(i = acts->begin(); i != acts->end(); ++i)
{
(*i).activate();
}

// Delete the dynamically created list<Activators>
// This is were I get into trouble. This calls
// the destructor of Activator, that tries to delete
// the internal TextDisplayer. That line creates an
// error, complaining that that memory area has already
// been deleted.
delete acts;

return 1;
}

----------------end main.cpp---------------------------------------

----------abstractactivators.cpp-----------------------------------

#include <string>
#include <list>
#include "AbstractActivator.h"

// TextDisplayers
void DialogDisplayer::eek:perator()(const std::string & aMessage)
{
//::ShowMessage(AnsiString(aMessage.c_str()));
}

// Activators
bool Activator::activate()
{
//Método base solo usado para debug
//Los específicos deben de tener en cuenta los
// posibles errores.
return true;
}

Activator::Activator(TextDisplayer * aDisplayer)
:theDisplayer(aDisplayer)
{
//do nothing
}

Activator::~Activator()
{
delete theDisplayer;
}


// Factory
Activators * ActivatorFactory::eek:perator()(const ActivatorType aType)
{

Activators * lst = new Activators;
Activator act(new TextDisplayer);
lst->push_back(act);

return lst;
}

---------end activators.cpp-----------------------------------------------

--------activators.h-----------------------------------------
#ifndef ABSTRACT_ACTIVATOR_FLAG
#define ABSTRACT_ACTIVATOR_FLAG

#include <string>
#include <list>





class TextDisplayer
{
public:
TextDisplayer(){};
virtual ~TextDisplayer() {};
virtual void operator() (const std::string & aMessage) {};

};

class DialogDisplayer
:
TextDisplayer
{
public:
DialogDisplayer():TextDisplayer(){};
virtual ~DialogDisplayer() {};
virtual void operator() (const std::string & aMessage);
};



class Activator
{
public:
Activator(TextDisplayer * aDisplayer);
virtual ~Activator();
virtual bool activate();

private:
TextDisplayer* theDisplayer;
};


enum ActivatorType
{
NullActivator
};

typedef std::list<Activator> Activators;

class ActivatorFactory
{
public:
virtual Activators * operator()(const ActivatorType aType = NullActivator);
};

#endif //ABSTRACT_ACTIVATOR_FLAG

----------end-------------------------------------------
 
G

Gianni Mariani

Boogie said:
Hi,

The code below produces an access violation error, complaining that an object
is destroyed twice.

I don't understand why this happens. Any help would be appreciated.

Classic duh problem.

It's due to this:

// Factory
Activators * ActivatorFactory::eek:perator()(const ActivatorType aType)
{

Activators * lst = new Activators;
Activator act(new TextDisplayer);
lst->push_back(act);

return lst;
}

On destruction of Activator you delete a TextDisplayer. However
lst->push_back(act) makes a copy of act. act is later destroyed (on
exit from the method) and you inadvertently just deleted the
TextDisplayer object you just created. A potential solution is to use
reference counting. Or another (example below) is simply to add the
pointer to the list once it's in the list. Yet another is that there are
other list implementations that do not use the copy constructor at all.


Try this:

// Factory
Activators * ActivatorFactory::eek:perator()(const ActivatorType aType)
{

Activators * lst = new Activators;

lst->push_back(Activator());

Activators::iterator iter = lst->end();
-- iter;
(*iter).SetDisplayer( new TextDisplayer );

return lst;
}

add this method to class Activator

virtual void SetDisplayer( TextDisplayer* d )
{
TextDisplayer* old = theDisplayer;
theDisplayer = d;
delete old;
}
 
B

Boogie El Aceitoso

On destruction of Activator you delete a TextDisplayer. However
lst->push_back(act) makes a copy of act. act is later destroyed (on
exit from the method) and you inadvertently just deleted the
TextDisplayer object you just created.

Aaarrrrgghhh!
Do all STL containers do this? If I add a pointer to a dynamically created
Activator (instead of one in the stack), will it follow the pointer and make a
copy of the object?

Maybe I should just add a copy constructor to Activator and TextDisplayer...

Thanks! :)
 
R

Ron Natalie

Boogie El Aceitoso said:
Do all STL containers do this? If I add a pointer to a dynamically created
Activator (instead of one in the stack), will it follow the pointer and make a
copy of the object?

Everythying in C++ does this (not just the standard containers). If yoiu want
to deep copy an object, you need to provide that in the copying operations.
Maybe I should just add a copy constructor to Activator and TextDisplayer...
This is precisely what you must do. Add a copy assignment operator as well.
Either that or put the data in a class such as vector or string that had the proper
copying semantics already.
 
G

Gianni Mariani

Ron said:
This is precisely what you must do. Add a copy assignment operator as well.
Either that or put the data in a class such as vector or string that had the proper
copying semantics already.

The vector comment is ambiguous, not sure what you mean.

If you can't make copies of TextDisplayer then copying the Activator
would only be portable if you reference counted (or garbage collected in
some way) the TextDisplayer objects being pointed to.
 
J

Josh Sebastian

The vector comment is ambiguous, not sure what you mean.

What he means is that instead of storing a pointer to an object, store an
actual object. Do you really need to store a pointer to a TextDisplayer?

Two uses of pointers are for C-style arrays and strings,
which can be almost drop-in replaced with std::vector and std::string,
respectively. For most other pointer uses, you can use a handle class with
deep-copying semantics.
If you can't make copies of TextDisplayer then copying the Activator
would only be portable if you reference counted (or garbage collected in
some way) the TextDisplayer objects being pointed to.

Or deep-copy the TextDisplayer.

Josh
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,744
Messages
2,569,482
Members
44,901
Latest member
Noble71S45

Latest Threads

Top