Malloc failing only in release

Started by
11 comments, last by Namethatnobodyelsetook 19 years, 6 months ago
Alright, I'm having yet another weird problem, which I hadn't even noticed until now. I have a class that is little more than a list. It is defined as follows:

class KSEList {
public:
    KSEList();
    ~KSEList();
    KSE *current;
    KSEList *next;

    void operator delete (void *);
    void* operator new(size_t size);

    int getLength();

    float r;
    float s;
    int h;
};

New and delete are:

void* KSEList::operator new(size_t size) {
    KSEList *tmp = (KSEList *) malloc(size);
    tmp->current = 0;
    tmp->next = 0;
    return tmp;
}
void KSEList::operator delete(void *s) {
    if(s==0)
        return;
    if(((KSEList *)s)->next!=0) {
        delete ((KSEList *)s)->next;
    }
    if(((KSEList *)s)->current!=0)
        delete ((KSEList *)s)->current;
    free(s);
    s = 0;
}

Now, my problem is that when I run the code in Release mode (by going into the release directory and running the application) I eventually get the XP "GAT.exe has encountered a problem and needs to close. We are sorry for the inconvenience." I have narrowed this down to the 14th call to new, specifically, where tmp is being malloc'd. I don't get this problem in debug mode from the actual app or within the IDE, nor do I get it when running in release within the IDE. Is there a problem with my code?
If a plant cannot live according to its nature, it dies; so a man.
Advertisement
I don't think your operator new and delete are allowed to assume they're pointing to the class, and play with the data. For example, some compilers may store an array size when you do a new[], and this would be the flow of a new operation

x = new Blah[5];
which calls
void *Blah::operator new(sizeof(Blah)*5 + sizeof(size_t))
which does
return malloc (size)
then the compiler does:
*ptr = 5;
ptr += sizeof(size_t);
return ptr

so... x != the return from the malloc. Assuming the pointer is untouched may work, on some versions of some compilers, but relying on that is asking for trouble.

Any initilization of member data should be done in the constructor or initilization list. Any cleanup in the destructor. This will allow subclasses to work correctly. This will allow array allocations to work correctly. This won't get in the way of the compiler's housekeeping data. It's the way it was designed to work.

edit: Also, why does deleting an object, cause it to delete other objects? Can you not delete a single list element, without destroying the entire list (or remainder of the list)?
Ok, I'm not really familiar with new/delete to be honest.
Quote:Original post by Namethatnobodyelsetook
edit: Also, why does deleting an object, cause it to delete other objects? Can you not delete a single list element, without destroying the entire list (or remainder of the list)?

It's a list of sub-expressions, and if I delete one element, I'm always going to be deleting all of those it is connected to. As far as I could tell, it was working fine, but then with this release issue, maybe I'm wrong.
If a plant cannot live according to its nature, it dies; so a man.
Replacing everything in new with:
return malloc(size);

Ends with the same problem (in new)...


As a side question, what is the purpose of overloading new/delete if you can't do that? The only purpose I can think of is a counter to keep track of how many news/deletes have been called (memory leaks).
If a plant cannot live according to its nature, it dies; so a man.
first of all why don't you just use a standard library list? and if you mention anything about efficiency its going to be a redundant argument because you can always write/use a custom allocator, also if you only need a singlely linked-list then you can always use slist yes its not part of the c++ standard but its better than writting your own hand-written one.

Next don't mix C-style memory operations with c++, i mean stuff like calling new then using free, c-style memory operations deal with unintialized memory, its fine to use them with-in your version of operator new/delete but don't mix & match allocation with-out any thought.

Next your new/delete pair don't match, in your new operator don't allocate any memory for next & current but in your delete operator your deleting them and also probably destroying the list maybe even calling delete more than once.

Lastly if your going to write your own operator new & delete for your user-defined type aleast provide array versions of them aswell.


Quote:Original post by Namethatnobodyelsetook
x = new Blah[5];
which calls
void *Blah::operator new(sizeof(Blah)*5 + sizeof(size_t))


Well if he does that on his KSEList its going to call the global array new, and the calculation is more likely to be:

void* ::operator new[](sizeof(Blah) * 5 + delta bytes)

where delta bytes is some minimal implementation defined over-head.
Quote:Original post by Woodsman
As a side question, what is the purpose of overloading new/delete if you can't do that? The only purpose I can think of is a counter to keep track of how many news/deletes have been called (memory leaks).


The main purpose is to take-over memory management, stuff like using garabage collected heap memory, or object pool memory scheme instead of using the general heap memory basically to be more efficent. So when a clients call new/new[] & delete/delete[] on your user-defined type it will use your version of new/new[] & delete/delete[] and not the general puspose global new/new[] & delete/delete[].

[Edited by - snk_kid on September 22, 2004 10:33:50 AM]
Quote:Original post by snk_kid
Next don't mix C-style memory operations with c++, i mean stuff like calling new then using free, c-style memory operations deal with unintialized memory, its fine to use them with-in your version of operator new/delete but don't mix & match allocation with-out any thought.

Am I? Everything I'm allocating/deallocating is done in the above fashion.
Quote:Original post by snk_kid
Next your new/delete pair don't match, in your new operator don't allocate any memory for next & current but in your delete operator your deleting them and also probably destroying the list maybe even calling delete more than once.

But new is setting new and current to 0, and delete just returns if they are 0. Is that valid?
Quote:Original post by snk_kid
Lastly if your going to write your own operator new & delete for your user-defined type aleast provide array versions of them aswell.

I'll never need an array of them. Anyway, again, I'm new to this so your input is greatly appreciated.
If a plant cannot live according to its nature, it dies; so a man.
Quote:Original post by Woodsman
But new is setting new and current to 0, and delete just returns if they are 0. Is that valid?


yeah but if there are not null which they most likely will eventually be.

Actually i just remembered that that user-defined types of operator new & delete are implicitly static so when you call delete on KSEList it calls delete on next & current which is probably giving you the problems as your most likely messing up the list.

I would just ust STL's slist to be honest.

[Edited by - snk_kid on September 22, 2004 11:10:13 AM]
The last line of operator delete where you set s=0 doesn't null the pointer passed in, it just sets the (local) parameter to 0, so the original is unchanged. I bet that's part of the problem.
"Most people think, great God will come from the sky, take away everything, and make everybody feel high" - Bob Marley
Quote:Original post by Woodsman
As a side question, what is the purpose of overloading new/delete if you can't do that? The only purpose I can think of is a counter to keep track of how many news/deletes have been called (memory leaks).


Yes, but new and delete shouldn't attempt to write to class member variables. What I do is to wrap the allocations. I allocate extra space, put in my tracking info, and return the pointer following my extra space. On delete, I subtract from the pointer to get back to my tracking structure and the actual malloc'd pointer.

// User Information APIunsigned long MemSys_BeginTrackingSection(void);void MemSys_DumpAllocationsSince(unsigned long nTrackingID, bool bSingleAlloc, bool bLine, bool bFile, bool bTotal);unsigned long MemSys_MemoryAllocatedSince(unsigned long nTrackingID);// Internal Memory API, advanced user operations APIvoid *MemSys_TrackedAllocate(size_t s, char *pFile, unsigned long nLine, unsigned long nType);void *MemSys_TrackedReAllocate(void *pMem, size_t s);void MemSys_TrackedFree(void *pMem, unsigned long nType);void MemSys_PatchOwnerInfo(void *pMem, char *pFile, unsigned long nLine);// Types for TrackedAllocate and TrackedFree.// Enforces using correct free for each allocation type#define MemSys_AllocStyleC 0x4D454D01#define MemSys_AllocStyleCPP 0x4D454D02#define MemSys_AllocStyleCPPArray 0x4D454D03// Global newvoid *operator new(size_t);void *operator new[](size_t);void operator delete(void *pMem);void operator delete[](void *pMem);void *operator new(size_t, char *pFile, unsigned long nLine);void *operator new[](size_t, char *pFile, unsigned long nLine);void operator delete(void *pMem, char *pFile, unsigned long nLine);void operator delete[](void *pMem, char *pFile, unsigned long nLine);#define GNew new(__FILE__, __LINE__)// Placement newvoid *operator new(size_t, void *pAddr);void *operator new[](size_t, void *pAddr);void operator delete(void *pMem, void *pAddr);void operator delete[](void *pMem, void *pAddr);#define PNew(a) new(a)// Take over C allocationsvoid *MemSys_Malloc(size_t n, char *pFile, unsigned long nLine);void *MemSys_Calloc(size_t n, size_t m, char *pFile, unsigned long nLine);void *MemSys_ReAlloc(void *ptr, size_t n, char *pFile, unsigned long nLine);void MemSys_Free(void *ptr);#define malloc(n) MemSys_Malloc(n, __FILE__, __LINE__)#define calloc(n,m) MemSys_Calloc(n,m, __FILE__, __LINE__)#define realloc(ptr,n) MemSys_ReAlloc(ptr, n, __FILE__, __LINE__)#define free(ptr) MemSys_Free(ptr)


#include "stdafx.h"#undef malloc#undef calloc#undef free#undef realloc#include "stdio.h"unsigned long g_MemSys_DebugID = 0;class MemSys_Node{public:  MemSys_Node *m_pNext;  MemSys_Node *m_pPrev;  unsigned long m_nType;  unsigned long m_nSize;  char *        m_pFile;  unsigned long m_nLine;  unsigned long m_nDebugID;};class MemSys_List{public:  MemSys_Node   m_oHead;  MemSys_Node   m_oTail;  EMutex        m_oMutex;  MemSys_List();  ~MemSys_List();  void Add(MemSys_Node *);  void Rem(MemSys_Node *);  void Sort();};void MemSys_List::Sort(void){  // Return on empty list;  if (m_oHead.m_pNext == &m_oTail)    return;  // based on http://www.chiark.greenend.org.uk/~sgtatham/algorithms/listsort.html  // Apparently O(n log n), which is good.  // Merge sort has issues on arrays (extra storage requirements) which are  //  not an issue with list based implementations.  This is supposed to be  //  a really good list sort algorithm.  // This could be made faster by only maintaining the next pointers, and  //  going back and filling in the prev pointers when the sort is complete.  MemSys_List sortedList;  long k = 1;  long numMerges;  MemSys_Node *p, *q, *tmp;  long psize, qsize;  do  {    p = m_oHead.m_pNext;    numMerges = 0;    while (p->m_pNext)    {      numMerges++;      q = p;      psize = 0;      while (psize < k)      {        psize++;        q = q->m_pNext;        if (!q->m_pNext)          break;      }      qsize = k;      // Merge phase      if (q->m_pNext)  // Since q doesn't change every loop iteration, I've broken this out of the main loop condition      {        while (qsize && psize)        {          int strres;          // test case          if (!p->m_pFile)          {            if (!q->m_pFile)              strres = 0;            else              strres = -1;          }          else          {            if (!q->m_pFile)              strres = 1;            else              strres = strcmp(p->m_pFile, q->m_pFile);          }          if ( (strres < 0) || (strres == 0 && p->m_nLine <= q->m_nLine) )//          if ( *((long *)(&((char *)p)[offset])) <= *((long *)(&((char *)q)[offset])) )          {            tmp = p->m_pNext;            sortedList.Add(p);            psize--;            p = tmp;          }          else          {            tmp = q->m_pNext;            sortedList.Add(q);            qsize--;            q = tmp;            if (!q->m_pNext) // Since q doesn't change every loop iteration, I've broken this out of the main loop condition              break;          }        }      }      while (psize--)      {        tmp = p->m_pNext;        sortedList.Add(p);        p = tmp;      }      while (qsize-- && q->m_pNext)      {        tmp = q->m_pNext;        sortedList.Add(q);        q = tmp;      }      p = q;    }    k <<= 1;    m_oHead.m_pNext = sortedList.m_oHead.m_pNext;    m_oTail.m_pPrev = sortedList.m_oTail.m_pPrev;    m_oHead.m_pNext->m_pPrev = &m_oHead;    m_oTail.m_pPrev->m_pNext = &m_oTail;    sortedList.m_oHead.m_pNext = &sortedList.m_oTail;    sortedList.m_oTail.m_pPrev = &sortedList.m_oHead;  } while (numMerges > 1);}MemSys_List::MemSys_List(){  m_oHead.m_pNext = &m_oTail;  m_oHead.m_pPrev = 0;  m_oTail.m_pPrev = &m_oHead;  m_oTail.m_pNext = 0;}MemSys_List::~MemSys_List(){}void MemSys_List::Add(MemSys_Node *pNode){  pNode->m_pNext = &m_oTail;  pNode->m_pPrev = m_oTail.m_pPrev;  m_oTail.m_pPrev->m_pNext = pNode;  m_oTail.m_pPrev = pNode;}void MemSys_List::Rem(MemSys_Node *pNode){  pNode->m_pPrev->m_pNext = pNode->m_pNext;  pNode->m_pNext->m_pPrev = pNode->m_pPrev;}MemSys_List g_MemSys_MemList;#define OBTAINMUT   g_MemSys_MemList.m_oMutex.Obtain()#define RELEASEMUT  g_MemSys_MemList.m_oMutex.Release()void *MemSys_TrackedAllocate(size_t s, char *pFile, unsigned long nLine, unsigned long nType){  MemSys_Node *pMem = (MemSys_Node *)malloc(s + sizeof(MemSys_Node));  if (pMem)  {    pMem->m_nDebugID = g_MemSys_DebugID;    pMem->m_pFile = pFile;    pMem->m_nLine = nLine;    pMem->m_nSize = s;    pMem->m_nType = nType;    OBTAINMUT;    g_MemSys_MemList.Add(pMem);    RELEASEMUT;    pMem++;  }#if MEMSYSTEM_THROW  else  {    if (nType != MemSys_AllocStyleC)    {      throw EException_NoMemory();    }  }#endif  return pMem;}void *MemSys_TrackedReAllocate(void *pMem, size_t s){  MemSys_Node *pMemory = (MemSys_Node *)pMem;  MemSys_Node *pNewMemory;  char *pFile;  unsigned long nLine;  unsigned long nID;  pMemory--;  EASSERT(pMemory->m_nType == MemSys_AllocStyleC);  EASSERT(pMemory->m_pNext);  EASSERT(pMemory->m_pNext->m_pPrev == pMemory);  EASSERT(pMemory->m_pPrev);  EASSERT(pMemory->m_pPrev->m_pNext == pMemory);  pFile = pMemory->m_pFile;  nLine = pMemory->m_nLine;  nID = pMemory->m_nDebugID;  OBTAINMUT;  g_MemSys_MemList.Rem(pMemory);  pNewMemory = (MemSys_Node *)realloc(pMemory, s + sizeof(MemSys_Node));  if (!pNewMemory)  {    g_MemSys_MemList.Add(pMemory);    RELEASEMUT;    return 0;  }  pNewMemory->m_nDebugID = nID;  pNewMemory->m_nLine = nLine;  pNewMemory->m_pFile = pFile;  pNewMemory->m_nSize = s;  pNewMemory->m_nType = MemSys_AllocStyleC;  g_MemSys_MemList.Add(pNewMemory);  RELEASEMUT;  pNewMemory++;  return pNewMemory;}void MemSys_TrackedFree(void *pMem, unsigned long nType){  // delete 0, or free(0) are acceptable.  if (pMem == 0)    return;  MemSys_Node *pMemory = (MemSys_Node *)pMem;  pMemory--;  EASSERT(pMemory->m_nType == nType);  EASSERT(pMemory->m_pNext);  EASSERT(pMemory->m_pNext->m_pPrev == pMemory);  EASSERT(pMemory->m_pPrev);  EASSERT(pMemory->m_pPrev->m_pNext == pMemory);  OBTAINMUT;  g_MemSys_MemList.Rem(pMemory);  RELEASEMUT;  free (pMemory);}void MemSys_PatchOwnerInfo(void *pMem, char *pFile, unsigned long nLine){  MemSys_Node *pMemory = (MemSys_Node *)pMem;  pMemory--;  EASSERT(pMemory->m_pNext);  EASSERT(pMemory->m_pPrev);  EASSERT(pMemory->m_pNext->m_pPrev == pMemory);  EASSERT(pMemory->m_pPrev->m_pNext == pMemory);  pMemory->m_pFile = pFile;  pMemory->m_nLine = nLine;}#if MEMSYSTEM_ENABLE_GLOBAL == 1void *operator new(size_t s){  return MemSys_TrackedAllocate(s, 0, 0, MemSys_AllocStyleCPP);}void *operator new[](size_t s){  return MemSys_TrackedAllocate(s, 0, 0, MemSys_AllocStyleCPPArray);}void operator delete(void *pMem){  MemSys_TrackedFree(pMem, MemSys_AllocStyleCPP);}void operator delete[](void *pMem){  MemSys_TrackedFree(pMem, MemSys_AllocStyleCPPArray);}void *operator new(size_t s, char *pFile, unsigned long nLine){  return MemSys_TrackedAllocate(s, pFile, nLine, MemSys_AllocStyleCPP);}void *operator new[](size_t s, char *pFile, unsigned long nLine){  return MemSys_TrackedAllocate(s, pFile, nLine, MemSys_AllocStyleCPPArray);}void operator delete(void *pMem, char *pFile, unsigned long nLine){  MemSys_TrackedFree(pMem, MemSys_AllocStyleCPP);}void operator delete[](void *pMem, char *pFile, unsigned long nLine){  MemSys_TrackedFree(pMem, MemSys_AllocStyleCPPArray);}#endif#if MEMSYSTEM_ENABLE_PLACEMENT == 1void *operator new(size_t s, void *pAddr){  return pAddr;}void *operator new[](size_t s, void *pAddr){  return pAddr;}void operator delete(void *pMem, void *pAddr){}void operator delete[](void *pMem, void *pAddr){}#endif#if MEMSYSTEM_ENABLE_CFUNC == 1void *MemSys_Malloc(size_t n, char *pFile, unsigned long nLine){  return MemSys_TrackedAllocate(n, pFile, nLine, MemSys_AllocStyleC);}void *MemSys_Calloc(size_t n, size_t m, char *pFile, unsigned long nLine){  void *pMem;  pMem = MemSys_TrackedAllocate(n*m, pFile, nLine, MemSys_AllocStyleC);  memset(pMem, 0, n*m);  return pMem;}void *MemSys_ReAlloc(void *ptr, size_t n, char *pFile, unsigned long nLine){  if (n == 0)  {    if (ptr)      MemSys_Free(ptr);    return 0;  }  if (ptr)    return MemSys_TrackedReAllocate(ptr, n);  else    return MemSys_TrackedAllocate(n, pFile, nLine, MemSys_AllocStyleC);}void MemSys_Free(void *ptr){  MemSys_TrackedFree(ptr, MemSys_AllocStyleC);}#endifvoid MemSys_NumberToString(char *pBuffer, unsigned long nLen, unsigned long nValue){  char *pOut = &pBuffer[nLen-1];  long n = 0;  memset(pBuffer, ' ', nLen);  pBuffer[nLen] = 0;  do  {    *pOut = (char) ((nValue % 10) + '0');    nValue = nValue / 10;    pOut--;    n++;    if (n==3 && nValue)    {      n = 0;      *pOut = ',';      pOut--;    }  }  while (nValue != 0);}unsigned long MemSys_BeginTrackingSection(void){  g_MemSys_DebugID++;  return g_MemSys_DebugID;}// NOTE THIS VERSION OF MY MEMORY DUMP FUNC ISN'T COMPLETEvoid MemSys_DumpAllocationsSince(unsigned long nTrackingID, bool bSingleAlloc, bool bLine, bool bFile, bool bTotal){  char tempBuffer[512];  unsigned long nPrevLine = 0xFFFFFFFF;  char *pPrevFile = (char *)1;  unsigned long nAllocsPerLine=0, nMemPerLine=0;  unsigned long nAllocsPerFile=0, nMemPerFile=0;  unsigned long nAllocsTotal=0, nMemTotal=0;  MemSys_Node *pNode;  OBTAINMUT;  g_MemSys_MemList.Sort();  for (pNode = g_MemSys_MemList.m_oHead.m_pNext; pNode->m_pNext; pNode = pNode->m_pNext)  {    if (pNode->m_nDebugID <= nTrackingID)    {      EASSERT(false);      // WRITEME;      // File change?  Dump last line and file      if (pPrevFile != pNode->m_pFile)      {        if (pPrevFile != (char *)1)        {          if (!bSingleAlloc)          {            // Line dump          }          if (nAllocsPerLine != nAllocsPerFile)          {            // File dump          }        }      }      else if (nPrevLine != pNode->m_nLine)      {        if (!bSingleAlloc)        {          // Line change?  Dump last line        }      }      // Dump single allocs      // Update line values      // Update file values      // Update total values.      sprintf (tempBuffer, "%s(%d): %d bytes\n", pNode->m_pFile, pNode->m_nLine, pNode->m_nSize);      EOutputDebugString(tempBuffer);    }  }  RELEASEMUT;}unsigned long MemSys_MemoryAllocatedSince(unsigned long nTrackingID){  unsigned long nMem = 0;  MemSys_Node *pNode;  OBTAINMUT;  for (pNode = g_MemSys_MemList.m_oHead.m_pNext; pNode->m_pNext; pNode = pNode->m_pNext)  {    if (pNode->m_nDebugID <= nTrackingID)      nMem += pNode->m_nSize;  }  RELEASEMUT;  return nMem;}

This topic is closed to new replies.

Advertisement