Thread: Tons of memory leaks

  1. #1
    Registered User VirtualAce's Avatar
    Join Date
    Aug 2001
    Posts
    9,607

    Tons of memory leaks

    I think this issue belongs on the C++ board and not the Windows board because the problem stems from the vector which is definitely a C++ issue. If the mods disagree feel free to move it.

    Problem:
    I've done a lot of work on my tile editor and I must have done something wrong because now I have a memory leak for every tile in the system. I did not have this before.

    Here is how the system works and I'll post the code that I believe is causing the problem.

    The tiles are managed by CTileManager. This is essentially a wrapper to a vector of CTile(s). All access is through CTileManager. All of CTile(s) members are private and CTileManager is a friend of CTile. This allows me to create a public interface to access and create private objects in the vector.
    I don't want CTile(s) being created by anyone but CTileManager.

    Here is the creation code. This code simply extracts tiles of desired width and height from a bitmap that acts as a palette of the tiles. This allows me to compile the tile bitmaps into one huge image that contains nearly all of the bitmaps for the game.
    And this allows me later to be able to access the bitmap quite easily in DX by simply altering the u and v texture coords instead of constantly switching textures. Eventually I will run out of texture memory in DX, but that solution is for later.

    Here is the code:
    Code:
    void CTileManager::AddFromImage(CString filename,COLORREF dwTrans)
    {
      //Bail if no init
      if (m_bInitFlag==false) return;
    
      HGDIOBJ image=::LoadImage(AfxGetInstanceHandle(),
                                filename,
                                IMAGE_BITMAP,
                                0,
                                0,
                                LR_LOADFROMFILE | LR_VGACOLOR | LR_DEFAULTSIZE);
    
      if (image==NULL)
      {
        ::MessageBox(0,filename,"FILE ERROR",0);
        return;
      }
    
      
      //BMP image on disk
      CDC ImageBitmapDC;
      CBitmap ImageBitmap;
    
      //Create compatible DC for image
      ImageBitmapDC.CreateCompatibleDC(NULL);
      //Create bitmap
      ImageBitmap.Attach(image);
    
      //Image Bitmap to DC
      ImageBitmapDC.SelectObject(ImageBitmap);
          
      BITMAP bmp;
      ImageBitmap.GetBitmap(&bmp);
          
      DWORD dwWidthCounter=0;
      DWORD dwHeightCounter=0;
    
      DWORD dwImageRight=bmp.bmWidth;
      DWORD dwImageBottom=bmp.bmHeight;
          
      //Resulting tile image
          
    
      CMainFrame *ptrFrame=(CMainFrame *)AfxGetMainWnd();
      CDC *debugDC=ptrFrame->GetActiveView()->GetDC();
    
      /*CDC TileDC;
      TileDC.CreateCompatibleDC(NULL);
      CBitmap TileBitmap;
    
      TileBitmap.CreateCompatibleBitmap(&ImageBitmapDC,m_dwWidth,m_dwHeight);
      TileDC.SelectObject(&TileBitmap);
      */
      DWORD dbugTile=0;
    
    
      do
      {
        CDC *TileDC=new CDC();
        TileDC->CreateCompatibleDC(NULL);
    
        CBitmap *TileBitmap=new CBitmap();
       
        TileBitmap->CreateCompatibleBitmap(&ImageBitmapDC,m_dwWidth,m_dwHeight);
        TileDC->SelectObject(TileBitmap);
      
        CTile *Tile=new CTile();
            
        //Create compatible DC for final tile image
        BOOL ok=TileDC->BitBlt(0,0,
                              m_dwWidth,m_dwHeight,
                              &ImageBitmapDC,
                              dwWidthCounter,
                              dwHeightCounter,
                              SRCCOPY);
            
        Tile->Create(TileDC,TileBitmap,Tiles.size());
        Tiles.push_back(Tile);
    
        BOOL dok=debugDC->BitBlt(dwWidthCounter,dwHeightCounter,
                                 m_dwWidth,m_dwHeight,
                                 GetDC(dbugTile),
                                 0,
                                 0,
                                 SRCCOPY);
        
    
        
        dwWidthCounter+=m_dwWidth;
        if (dwWidthCounter>=dwImageRight)
        {
          dwWidthCounter=0;
          dwHeightCounter+=m_dwHeight;
        }
        dbugTile++;
        
       } while (dwHeightCounter<dwImageBottom);
    
       ImageBitmapDC.SelectObject(ImageBitmap);
    }
    As you can see I'm creating a CBitmap and a CDC object each time I create a tile. I cannot delete the CDC object after I create it because the CDC that is in the vector is the exact same CDC local to this function. My thinking is if I push the CDC to the vector and then delete the CDC later in the destructor code for CTileManager, it should be ok. Deleting the CDC in the above code causes the tiles to all be blank. I can, however, delete the bitmap here but I choose to pass it on to CTileManager so it can also delete the bitmap when it shuts down.

    No matter which approach I take, I always have the same number of memory leaks as tiles. Even each tile attempts to destroy it's own CDC and CBitmap in it's destructor. However, I check to make sure they are valid. According to my code the end result is that the memory should be cleaned up correctly.

    My guess is somehow the bitmaps are never cleaned up? Dunno really. Need some expert advice on this one.

    I do the same thing for CMap and CMapManager and it creates 0 memory leaks. This must be something to do with CBitmap and CDC being Windows GDI objects.

    Perhaps when I SelectObject() to select the bitmap out of the device context, I should have a pointer to the previous bitmap, and then delete that pointer? In my setup SelectObject() should return NULL since there wasn't a bitmap object selected into the DC prior to this function.

    Also notice that ImageBitmapDC and ImageBitmap are created on the stack and so should go out of scope and be cleaned up at the end of the function. This would be done by the compiler.

    Perhaps the solution is to create the bitmap and the CDC inside of the Create() function for CTile. This way I would have more control over what's taking place. The idea here is that I'm prepping the CTile CDC so that it is ready to use. That is, the bitmap for the tile has already been blitted to the DC. This way I can blit the tile to the screen later extremely fast. No selection crapola.

    Help.

    EDIT: Added a text file that shows what is happening. According to the text file, the tiles I create have the same address as the tiles in the vector. Therefore they are being deleted correctly. Why is MSVC complaining about memory leaks when I create CTile inside of the loop?
    Last edited by VirtualAce; 12-04-2005 at 11:08 AM.

  2. #2
    Registered User
    Join Date
    Aug 2003
    Posts
    470
    It looks like inside your Tile Destructor the TileDC and TileBitbmap passed into the constructor aren't being deleted.
    (You're also leaking the GDI objects by either not selecting the old ones back in or using SaveDC/RestoreDC.)

  3. #3
    Registered User VirtualAce's Avatar
    Join Date
    Aug 2001
    Posts
    9,607
    In my code I select the bitmap out of the DC in the destructor for both the tile and the view.

    It's sort of complex but what I have is a renderer class that simply renders a tile map. This class can also be used to present a tile tool palette that allows you to select a tile to place in the map. Since both are essentially the same thing, I use the same class for it.

    CTileManager
    - vector of CTile(s)

    CMapManager
    - vector of CMap(s)

    CTileRenderer
    - CTileRendererInfo
    - struct describing all attributes of renderer
    - scroll information
    - color information
    - transparent color
    - background color
    - grid color (for when grid is turned on)
    - zoom information
    - size information
    - usage information (if this renderer is a selection tool or just a display)
    - simply renders tiles to an offscreen CDC
    - blits to on-screen CDC

    CTilePickerToolDlg
    - pointer to CTileManager
    - pointer to it's own CMapManager (multiple tile tool palettes can be created)
    - CTileRenderer

    CEditorView
    - CTileRenderer
    - CTileManager (same one as for CTilePickerToolDlg)
    - CMapManager (not the same as for CTilePickerToolDlg)


    Each class is responsible for destroying it's own objects whethey they be arrays or class objects. Prior to destroying the CDC and CBitmap for CTile, it's destructor selects the bitmap out of the DC, calls DeleteDC() since it was created using CreatCompatibleDC() and then deletes the CDC object. The CBitmap object is then also deleted.

    Each manager class calls vector::empty() which according to the docs will in turn call the destructor for each item in the vector. So, this should clean the memory up correctly.

    Please explain how you came to your conclusion. Perhaps I'm missing something vital.

  4. #4
    semi-colon generator ChaosEngine's Avatar
    Join Date
    Sep 2005
    Location
    Chch, NZ
    Posts
    597
    Quote Originally Posted by Bubba
    Each manager class calls vector::empty() which according to the docs will in turn call the destructor for each item in the vector. So, this should clean the memory up correctly.
    This is your problem. I think you need to re-read the docs on vector::empty()


    Besides even if empty did what you think it does, you'd still have memory leaks. You need to delete each object in the vector, not just call it's destructor.

    Your best bet for something like this is to use a boost::shared_ptr

    Trust me, I avoided this for years and when I eventually tried it I kicked myself for wasting so much time.
    "I saw a sign that said 'Drink Canada Dry', so I started"
    -- Brendan Behan

    Free Compiler: Visual C++ 2005 Express
    If you program in C++, you need Boost. You should also know how to use the Standard Library (STL). Want to make games? After reading this, I don't like WxWidgets anymore. Want to add some scripting to your App?

  5. #5
    Registered User VirtualAce's Avatar
    Join Date
    Aug 2001
    Posts
    9,607
    Oops. Should be vector::clear().

    Perhaps this is the issue. And just to be safe I am doing this:

    if (Tiles[i]) delete Tiles[i];

    in the destructor of the tile manager.

  6. #6
    & the hat of GPL slaying Thantos's Avatar
    Join Date
    Sep 2001
    Posts
    5,681
    of course the if() isn't needed as calling delete on a NULL pointer is perfectly allowed.

  7. #7
    Registered User VirtualAce's Avatar
    Join Date
    Aug 2001
    Posts
    9,607
    Except that Windows fires back with a nice dialog box stating an exception. I've tried this and without the if() Windows gripes.

  8. #8
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,895
    VC++6? I think it was broken that way.
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

  9. #9
    & the hat of GPL slaying Thantos's Avatar
    Join Date
    Sep 2001
    Posts
    5,681
    And that is why windows is evil!

    jk. Didn't someone post a fix for that though?

    I mean the standard is pretty clear on the behavior of deleting a NULL pointer so I don't see why it wouldn't be in there.

    Have you thought of using the std::auto_ptr?

  10. #10
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,895
    You must not use std::auto_ptr in containers. Proper standard libraries, such as STLport, actually reject such code.
    You can use boost::shared_ptr, though.
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

  11. #11
    & the hat of GPL slaying Thantos's Avatar
    Join Date
    Sep 2001
    Posts
    5,681
    yeah thats right, forgot that auto_ptr doesn't keep track if anyone else is pointing to that data.

  12. #12
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,895
    Actually it's about the transfer of ownership. Namely, if you, e.g., apply a sort() or for_each() to a container holding auto_ptrs, some or all of them will be empty afterwards, their pointees having been freed.
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Checking for memory leaks
    By Bladactania in forum C Programming
    Replies: 5
    Last Post: 02-10-2009, 12:58 PM
  2. memory leaks
    By TehOne in forum C Programming
    Replies: 4
    Last Post: 10-10-2008, 09:33 PM
  3. Memory leaks problem in C -- Help please
    By Amely in forum C Programming
    Replies: 14
    Last Post: 05-21-2008, 11:16 AM
  4. COM Memory Leaks
    By subdene in forum Windows Programming
    Replies: 0
    Last Post: 06-07-2004, 11:57 AM
  5. about memory leaks with this simple program
    By Unregistered in forum C++ Programming
    Replies: 5
    Last Post: 04-07-2002, 07:19 PM