C++ design problem (memory management and polymorphism)

boyakasha (4)
Hi everyone,

There is a design problem I've run into a few times now, and I'm not sure how to get around it. I'll give you an example:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
 
class DriverInterface {
	public:
		virtual void doSomething() = 0;
};

class ConcreteDriverA : public DriverInterface {...};
class ConcreteDriverB : public DriverInterface {...};
class ConcreteDriverC : public DriverInterface {...};
	
class DriverManager {
	public:
		void registerDriver(DriverInterface* driver) { 
			_drivers.push_back(driver);
		}
	private:
		std::list<DriverInterface*> _drivers;
};

int main(int argc, char** argv){
	DriverManager driverManager;
	//solution 1, declare drivers on the stack
	{
		ConcreteDriverA a;
		ConcreteDriverB b;
		ConcreteDriverC c;
		driverManager.registerDriver(&a);
		driverManager.registerDriver(&b);
		driverManager.registerDriver(&c);
	} //problem: a,b, and c go out of scope and are destroyed

	//solution 2, declare drivers on the heap
	driverManager.registerDriver(new ConcreteDriverA);
	driverManager.registerDriver(new ConcreteDriverB);
	driverManager.registerDriver(new ConcreteDriverC);
	//problem: who is responsible for deleting the objects?
}


To summarize the code, my problem is this: I want to keep a collection of pointers to an interface class with pure virtual functions. If I instantiate the concrete classes on the stack, they get destroyed when they go out of scope and the pointers in the collection are no longer valid. If I instantiate the concrete classes on the heap, then who is responsible for deleting them?
Last edited on
awhite1159 (17)
I had a similiar problem and found the same problem with temporary objects that I created. I would create the object store the pointer to it in a collection, and the pointer would become invalid after the object went out of scope and was destroyed. I would suggest using a handle as a wrapper around your pointer which has the ability to do reference counting. That way when you place it in the collection, the collection will call your copy constructor which increments the reference count and copies the pointer. Now... ironically, I am about to make a post for a question I have about the problems I am facing using handles vs. pointers. The handle may be destroyed but the new handle that is copied remains with its reference count incremented. This handle only goes away when the reference count reaches zero and this will happen with the original handle. Sorry for the stringy response but solution 2 would be best and the handle would delete the objects when the reference count reached zero.
Last edited on
awhite1159 (17)
Didn't want this to end up here. Sorry. Not intentionally hijacking your thread. Anyway this shows how you might want to implement the handle class.

I have a problem with using handles vs. pointers. I am writing a network backup simulation program and need to have many links between the various objects which are all based on the abstract class 'DataMover'. The dilemma I have is that when I am in the constructor of an object that has handles that point to other objects, I have no way of obtaining the objects own handle. This has been driving me crazy for about two weeks now. Here is an excerpt of the code:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
 
// Necessary forward declarations
class DataMover;

//=========================== h_DM Handle class for DataMovers ======================== 
// Provides memory management, assignment, and copying for DataMover objects
// Also assists in the many network connections of datamover objects.
class h_DM {
	friend ostream& operator<<(ostream&, const h_DM&); 
	friend istream& operator>>(istream&, const h_DM&);
public:
	// Empty handle used for children nodes to be assigned later
	h_DM() : mDataMover(0), mEmpty(new int(1)), mNumHandles(new std::size_t(0)) { }
	// BkupServer or BkupNIC
	// type, name, ip, netmask, thruput, nic-thruput 
	h_DM(const char&, const string&, const string&,
		const string&, int tpkb=0, int nictpkb=0); 
	// BkupClient
	// name, ip, netmask, backup-server, thruput
	h_DM(const string&, const string&, const string&,
		const string&, int tpkb=0);
	// BkupNetwork
	// name, ip (network), netmask, thruput
	h_DM(const string&, const string&, const string&, int tpkb=0);
	// StorageVolume
	// name, bkupserver, thruput, bkupamt
	h_DM(const string&, const string&, int, int);
	
	h_DM(const h_DM&);
	~h_DM(); 
	h_DM& operator=(const h_DM&);
	DataMover* operator->() const; 
	operator bool() const { return mDataMover; }
	const string& getName() const;
	const string& mIsA() const;
	int count() const { return *mNumHandles; }
	const int DM_id() const; 
	void setIn(h_DM&);
	void setOut(h_DM&);
	static const h_DM& getActHand();
	static void setActHand(h_DM& h);
private:
	static h_DM mActHand;
	DataMover* mDataMover;
	std::size_t* mNumHandles;
	int* mEmpty;
};
//=========================== DataMover Abstract Class ================================
// The base DataMover class
class DataMover {
	friend class h_DM;
	friend class NetSuper;
	friend ostream& operator<<(ostream&, const h_DM&);
	friend istream& operator>>(istream&, const h_DM&);
public:
	DataMover(const string& n, const string& i) :
		mID(++mNumHandles), mName(n), mIsA(i) { }
	virtual ~DataMover() { }
	const string& getName() const { return mName; }
	const int getid() const { return mID; }
	const int getThruput() { return mThruput; }
	void setThruput(int tpkb); 
	virtual Net& getNet(int n=0) = 0; 
	virtual const string& getip_str(int n=0) = 0; 
	virtual void setip(const string&, int nn=0) = 0;
	virtual void setnm(const string&, int nn=0) = 0;
	virtual h_DM& addStgV(const string&, int, int) = 0;
	virtual h_DM& addNIC(const string&, const string&, int tpkb=0) = 0;
	virtual void handleMsg(Msg& m);
protected:
	virtual map<Net, h_DM>::iterator findNetNode(const Net&) = 0;
	virtual map<Net, h_DM>::iterator addNode(const Net&, const h_DM&) = 0;
	virtual map<Net, h_DM>::iterator node_list_end() = 0;
	virtual map<Net, h_DM>::iterator node_list_begin() = 0;
	virtual void print(ostream& o) const;
private:
	string mName;
	int mThruput;
	h_DM mIn;
	h_DM mOut;
	string mIsA;
	static int mNumHandles;
	int mID;
};

//=========================== h_DM function defines ======================================
h_DM::h_DM(const char& t, const string& n, const string& a, const string& nm,
	int tpkb, int nictpkb) : mEmpty(new int(0)), mNumHandles(new std::size_t(1)) {
	switch(t) {
		case 'S':
			mDataMover = new BkupServer(n, a, nm, tpkb, nictpkb);
			h_DM::setActHand(*this);
			break;
		case 'N':
			// tpkb is the parent id
			mDataMover = new BkupNIC(n, a, nm, tpkb, nictpkb);
			NetSuper::regNode(*this);
			break;
	}
}
h_DM::h_DM(const string& n, const string& a, const string& bs, const string& nm, 
	int tpkb) : mEmpty(new int(0)), mNumHandles(new std::size_t(1)) {
	mDataMover = new BkupClient(n, a, bs, nm, tpkb);
}
h_DM::h_DM(const string& n, const string& a, const string& nm,
	int tpkb) : mEmpty(new int(0)), mNumHandles(new std::size_t(1)) {
	mDataMover = new BkupNetwork(n, a, nm, tpkb);
	NetSuper::regNetwork(*this);
}

h_DM::h_DM(const string& n, const string& bkupsrv, int tpkb, int bkupamt) : 
	mEmpty(new int(0)), mNumHandles(new std::size_t(1)) {
	mDataMover = new StorageVolume(n, bkupsrv, tpkb, bkupamt);
}

h_DM::h_DM(const h_DM& copy) {
	if(*copy.mEmpty == 1)
		throw std::runtime_error("h_DM(h_DM&): Cannot copy empty handle.");
	mDataMover=copy.mDataMover; 
	mNumHandles=copy.mNumHandles;
	mEmpty=copy.mEmpty;
	 ++*mNumHandles; 
	 }
h_DM::~h_DM() { 
	
	if(--*mNumHandles == 0 && *mEmpty == 0) {
		delete mDataMover;
		delete mNumHandles;
		delete mEmpty;
	}
}
h_DM& h_DM::operator=(const h_DM& rhs) {
	if(*rhs.mEmpty == 1)
		throw std::runtime_error("h_DM::operator=(): Cannot assign empty handle.");	
		++*rhs.mNumHandles;
		*rhs.mEmpty=0;
	if(--*mNumHandles == 0) {
		delete mDataMover;
		delete mNumHandles;
		delete mEmpty;
	}
	else if(*mEmpty==1) {
		delete mNumHandles;
		delete mEmpty;
	}
	mDataMover = rhs.mDataMover;
	mNumHandles=rhs.mNumHandles;
	return *this;
}
inline DataMover* h_DM::operator->() const { 
		if(mDataMover)
			return mDataMover;
		throw std::runtime_error("h_DM(): Unbound handle.");
}
const string& h_DM::getName() const { return mDataMover->getName(); }
const int h_DM::DM_id() const { return mDataMover->getid(); }
const string& h_DM::mIsA() const { return mDataMover->mIsA; }
void h_DM::setIn(h_DM& i) { mDataMover->mIn=i; }
void h_DM::setOut(h_DM& o) { mDataMover->mOut=o; }
const h_DM& h_DM::getActHand() { return mActHand; }
void h_DM::setActHand(h_DM& h) { mActHand = h; }


Last edited on
boyakasha (4)
Yeah I was thinking about something like reference counting smart pointers, but it seems like overkill to me. I decided to try using the prototype pattern about 10 minutes ago and it looks like it will work. Here is the modified example:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
 
class DriverInterface {
	public:
		virtual void doSomething() = 0;
		virtual DriverInterface* clone() = 0;
};

class ConcreteDriverA : public DriverInterface {...};
class ConcreteDriverB : public DriverInterface {...};
class ConcreteDriverC : public DriverInterface {...};
	
class DriverManager {
	public:
		void registerDriver(DriverInterface* driver) { 
			_drivers.push_back(driver->clone());
		}
		
		~DriverManager(){
			DriverInterface* i;
			forall (i, _drivers){
				delete i;
			}
		}

	private:
		leda::list<DriverInterface*> _drivers;
};

int main(int argc, char** argv){
	DriverManager driverManager;
	//solution: declare drivers on the stack
	{
		ConcreteDriverA a;
		ConcreteDriverB b;
		ConcreteDriverC c;
		driverManager.registerDriver(&a);
		driverManager.registerDriver(&b);
		driverManager.registerDriver(&c);
	} //a,b, and c go out of scope, but driverManager has made clones so it doesn't matter
}


So in this example, DriverManager is responsible for deleting it's own copies of the drivers, and main() is responsible for deleting it's own copies of the drivers.

If someone has a better way, please share.
jsmith (5804)
On the contrary, I think smart pointers are the way to go. The boost libraries (http://www.boost.org) provide you with the shared_ptr<> template which does all the reference counting for you.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
 
#include <boost/smart_ptr.hpp>

// Definition is not needed by boost::shared_ptr<>, as the right virtual
// destructor is called, unlike std::auto_ptr<>...
class DriverInterface;

class DriverManager {
  public:
      void registerDriver( boost::shared_ptr<DriverInterface> const& driver ) {
             _drivers.push_back( driver );
      }

  private:
      std::deque< boost::shared_ptr<DriverInterface> > _drivers;
};      

int main() {
      DriverManager driverMgr;
  
      driverMgr.registerDriver( boost::shared_ptr<DriverInterface>(
          new ConcreteDriverA() ) );
}

boyakasha (4)
That code does look nice. Might have to grab boost.
ropez (310)
To be safe, and as a general rule, you should always use named variables for your smart pointers:
1
2
3
4
 
{
  boost::shared_ptr<DriverInterface> ifA(new ConcreteDriverA());
  driverMgr.registerDriver(ifA);
}


A much simpler way to do this, although it may not always be feasable, is to use stack allocation, but make sure you do it in the correct order.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
 
// DriverInterface and DriverManager like the original post

int main(int argc, char** argv){
  ConcreteDriverA a;
  ConcreteDriverB b;
  ConcreteDriverC c;

  DriverManager driverManager;
  driverManager.registerDriver(&a);
  driverManager.registerDriver(&b);
  driverManager.registerDriver(&c);

  // work

} // no problem

ropez (310)
Yet another solution, with explicit ownership:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
 
class DriverManager {
  public:
    void registerDriver(std::auto_ptr<DriverInterface>& driver) {
      _drivers.push_back(driver.release());
    }

    ~DriverManager() {
      DriverInterface* i;
      forall (i, _drivers) {
        delete i;
      }
    }

  private:
    std::list<DriverInterface*> _drivers;
};

int main() {
  DriverManager mgr;

  std::auto_ptr<DriverInterface> p;

  p.reset(new ConcreteDriverA);
  mgr.registerDriver(p);
  // p is null

  p.reset(new ConcreteDriverB);
  mgr.registerDriver(p);
  // p is null

  p.reset(new ConcreteDriverC);
  mgr.registerDriver(p);
  // p is null
}
jsmith (5804)
Going along the lines of ropez' solution, boost also provides the ptr_container library which contains most of the STL container classes except they only store pointers, and therefore the container takes care to delete the pointer when the element is removed.

ie, essentially it would be the same as ropez' solution except you wouldn't need the destructor for DriverManager.

Thanks ropez for pointing out my bug :) An important subtlety for exception safety.
Topic archived. No new replies allowed.