I have a problem with a code (legacy MMO) in VS2008, that I am currently maintaining as a hobby.
Disclaimer: I am not a programmer by trade, aside from C++ my only experience in coding is VBA.
I had problems with dynamically allocated objects (Database Actions using ODBC) in this code since it's being done so frequently, and it's causing heap fragmentation. And I had problems when using boost::shared_ptr
with boost::make_shared()
and it's causing Heap Corruption.
And I was thinking of removing many of the dynamic allocations and doing something along this lines.
Note: the code had Critical Sections when manipulating the queue, lets say LockOn()
enters the Critical Sections and LockOff()
leaves the Critical Sections.
class Base
{
public:
Base(int m_nClientNum):nClientNum(m_nClientNum){}
virtual ~Base(){}
virtual void Execute(){std::cout << " Base Executed " << std::endl;
protected:
int nClientNum;
};
class Derived: public Base
{
public:
Derived(int m_nClientNum,int m_nSomeValue, int m_nSomeOtherValue )
: nClientNum(m_nClientNum),
nSomeValue(m_nSomeValue),
nSomeOtherValue(m_nSomeOtherValue)
{
}
virtual ~Derived(){}
virtual void Execute()
{
std::cout << "Client Num: " << nClientNum << " , Value : " << nSomeValue << " ,Other Value: " << nSomeOtherValue << std::endl;
}
protected:
int nClientNum;
int nSomeValue;
int nSomeOtherValue;
};
class Consumer
{
public:
Consumer(){}
ConsumeBaseNow(Base* m_Base)
{
m_Base->Execute();
}
// The code originally only has the following two Functions
void AddBaseQueue(Base* m_Base)
{
LockOn();
m_vBase.push(m_Base);
LockOff();
}
void ConsumeBaseLater()
{
Base* pTemp = NULL; // NULL being defined as 0 in the standard header.
LockOn();
if ( m_vBase.IsEmpty() )
{
Sleep(1);
LockOff();
}
else
{
pTemp = m_vBase.front();
m_vBase.pop();
LockOff();
if ( pTemp )
{
pTemp->Execute();
delete pTemp;
pTemp = NULL;
}
}
}
protected:
std::queue<Base*> m_vBase;
};
// For simplicity's sake, I'm using the main function to show how the code is handling this class.
int main()
{
// Here is how the logic goes for the original Code
Consumer* pConsumer = new Consumer;
Derived* pDerived = new Derived(10,20,30);
pConsumer->AddBaseQueue( pDerived );
pConsumer->ConsumeBaseLater();
// Now my plan is to do this, since some of these Actions don't need
// to be queued since they are not being used that frequently and together at the same time.
Derived pDerived2(40,50,60);
Base* pBase = &pDerived2;
pConsumer->ConsumeBaseNow(pBase);
delete pConsumer;
pConsumer = NULL;
}
My questions are as follows:
Is creating a pointer that is pointing to an object on the stack undefined behavior? Is it a common thing to do, or is it something that I should put in a box and throw away in the lake?
Is there a problem with my boost library (1.43) that is causing heap corruption (memory keeps growing fast until I run out of it)? Note: I'm using it this way:
boost::shared_ptr<Base> pBase = boost::make_shared<Derived>(10,20,30);
Then I changed
ConsumeBaseLater
with this:{ LockOn(); if ( m_vBase.IsEmpty() ) { Sleep(1); LockOff(); } else { boost::shared_ptr<Base> pTemp ( m_vBase.front() ); pTemp->Execute(); m_vJob.pop(); LockOff(); } }
Or, am I using
shared_ptr
the wrong way?If I changed the queue to accept the objects themselves, how do I do it without slicing the objects? Since the queue is accepting the base class, and as far as my understanding goes, all derived classes are bases, but not all bases are derived.
EDIT: Thanks to Paul for pointing out that I didn't have virtual destructors in my original question's code (the code I'm maintaining does have virtual destructors, it's the first thing I looked at when I first encountered the heap problems), and I simply forgot to write it in the original question.