Author Topic: Is this a memory leak?  (Read 15507 times)

Offline Pecan

  • Plugin developer
  • Lives here!
  • ****
  • Posts: 2750
Is this a memory leak?
« on: January 24, 2006, 12:52:53 am »
I know this is probably a stupid question, but is wxString
freed in the following example? Or is there a leak.

Something like this happend to me during the recent
settings scheme revamp.

Code
class Aclass
{
   wxString* MyStringPtr;
   ~Aclass(){}
}
Aclass::Aclass()
{
    MyStringPtr = new wxString;
}

This is worrying me.

Thanks
pecan
« Last Edit: January 24, 2006, 01:00:59 am by Pecan »

Offline 280Z28

  • Regular
  • ***
  • Posts: 397
  • *insert unicode here*
Re: Is this a memory leak?
« Reply #1 on: January 24, 2006, 12:56:19 am »
It's not freed.
78 280Z, "a few bolt-ons" - 12.71@109.04
99 Trans Am, "Daily Driver" - 525rwhp/475rwtq
 Check out The Sam Zone :cool:

Offline rickg22

  • Lives here!
  • ****
  • Posts: 2283
Re: Is this a memory leak?
« Reply #2 on: January 24, 2006, 12:58:02 am »
Hint: Pointers to objects (wxString* m_pString) should always be allocated and deallocated manually on the constructor and destructor.
Direct objects (wxString m_String) are handled automatically.

Offline killerbot

  • Administrator
  • Lives here!
  • *****
  • Posts: 5490
Re: Is this a memory leak?
« Reply #3 on: January 24, 2006, 01:14:20 am »
a leak it is !


@Rick, not everything allocated by the user has to be freed by the user, especially with wxWidgets and menus, the ownership is tranfered in those cases.
So it's good to realize when ownership is transfered.
I for one, sureley doesn't know all the transfers within wx.  :(

Offline thomas

  • Administrator
  • Lives here!
  • *****
  • Posts: 3979
Re: Is this a memory leak?
« Reply #4 on: January 24, 2006, 01:38:23 am »
I for one, sureley doesn't know all the transfers within wx.  :(
If you give it away, you don't own it any more. That's a 99% good rule (with very very few exceptions).
"We should forget about small efficiencies, say about 97% of the time: Premature quotation is the root of public humiliation."

Offline Pecan

  • Plugin developer
  • Lives here!
  • ****
  • Posts: 2750
Re: Is this a memory leak?
« Reply #5 on: January 24, 2006, 02:13:37 am »
Whos responsibility is it to delete all the stuff used
to make dialogs, menus etc. Like:

EDIT: I screwed up the coloration, but you get the idea.


// ----------------------------------------------------------------------------
void wxKeyConfigPanel::BuildCtrls()
// ----------------------------------------------------------------------------
{
    if (m_nBuildMode & wxKEYBINDER_USE_TREECTRL) {

      // use a wxTreeCtrl to show the commands hierarchy
      m_pCommandsTree = new wxTreeCtrl(this, wxKEYBINDER_COMMANDS_BOX_ID, wxDefaultPosition,
                           wxDefaultSize, wxTR_HAS_BUTTONS | wxSUNKEN_BORDER);
   } else {

      // use a combobox + a listbox
      m_pCommandsList = newnew wxListBox(this, wxKEYBINDER_COMMANDS_BOX_ID, wxDefaultPosition,
                           wxDefaultSize, 0, NULL);
      m_pCategories = new wxComboBox(this, wxKEYBINDER_CATEGORIES_ID,
                        wxEmptyString, wxDefaultPosition, wxDefaultSize,
                        0, NULL, wxCB_READONLY);
   }

    m_pKeyField = new wxKeyMonitorTextCtrl(this, wxKEYBINDER_KEY_FIELD_ID);
    m_pBindings = new wxListBox(this, wxKEYBINDER_BINDINGS_BOX_ID);

    m_pAssignBtn = new wxButton(this, wxKEYBINDER_ASSIGN_KEY_ID, wxT("&Add"));
    m_pRemoveBtn = new wxButton(this, wxKEYBINDER_REMOVE_KEY_ID, wxT("&Remove"));
    m_pRemoveAllBtn =new new wxButton(this, wxKEYBINDER_REMOVEALL_KEY_ID, wxT("Remove all"));

   m_pCurrCmdField = newnew wxStaticText(this, -1, wxT(""), wxDefaultPosition,
      wxSize(-1, 20), wxSIMPLE_BORDER | wxST_NO_AUTORESIZE | wxALIGN_CENTRE);

   // we won't make it white because it must be clear to the user that this
   // is not a text control...
   m_pCurrCmdField->SetBackgroundColour(wxColour(200, 200, 200));

#ifdef __WXGTK__
    m_pDescLabel = newnew wxTextCtrl(this, -1, wxT(""), wxDefaultPosition,
                         wxDefaultSize, wxTE_READONLY | wxTE_MULTILINE);
#else
   m_pDescLabel =newnew wxStaticText(this, -1, wxT(""), wxDefaultPosition,
      wxSize(-1, 40), wxSIMPLE_BORDER | wxST_NO_AUTORESIZE);
   m_pDescLabel->SetBackgroundColour(wxColour(255, 255, 255));
#endif

   // KEY PROFILES
   // create the key profiles combobox & panel
   m_bEnableKeyProfiles = TRUE;

   // the style of a combobox must be set at the beginning:
   // you cannot change the wxCB_READONLY flag presence later...
   // VERY IMPORTANT: *NEVER* ADD THE CB_SORT STYLE:
   // IT WOULD GIVE US GREAT PROBLEMS WHEN EDITING THE KEYPROFILE...
   long style = (m_nBuildMode & wxKEYBINDER_ENABLE_PROFILE_EDITING) ? 0 : wxCB_READONLY;
   m_pKeyProfiles = newnew wxComboBox(this, wxKEYBINDER_KEYPROFILES_ID,
                        wxEmptyString, wxDefaultPosition, wxDefaultSize,
                        0, NULL, style);

   wxSizer *sizer = newnew wxBoxSizer(wxHORIZONTAL);
   sizer->Add(m_pKeyProfiles, 6, wxGROW);

   if (m_nBuildMode & wxKEYBINDER_SHOW_ADDREMOVE_PROFILE) {

      // create the Add & remove profile buttons
      sizer->Add(newnew wxButton(this, wxKEYBINDER_ADD_PROFILEBTN_ID, wxT("Add new")), 0,
                     wxGROW | wxLEFT | wxRIGHT | wxBOTTOM, 5);
      sizer->Add(newnew wxButton(this, wxKEYBINDER_REMOVE_PROFILEBTN_ID, wxT("Remove")), 0,
                     wxGROW | wxLEFT | wxRIGHT | wxBOTTOM, 5);
   }

   m_pKeyProfilesSizer = newnew wxBoxSizer(wxVERTICAL);
   m_pKeyProfilesSizer->Add(newnew wxStaticText(this, -1, wxT("Key profile:")), 0, wxGROW | wxALL, 5);
   m_pKeyProfilesSizer->Add(sizer, 0, wxGROW | wxLEFT | wxRIGHT, 5);
   m_pKeyProfilesSizer->Add(newnew wxStaticLine(this, -1), 0, wxGROW | wxALL, 5);
}
« Last Edit: January 24, 2006, 02:16:27 am by Pecan »

sethjackson

  • Guest

Offline thomas

  • Administrator
  • Lives here!
  • *****
  • Posts: 3979
Re: Is this a memory leak?
« Reply #7 on: January 24, 2006, 08:38:05 am »
Quote
Whos responsibility is it to delete all the stuff used
to make dialogs, menus etc. Like:
You give those objects away, so they aren't yours :)
"We should forget about small efficiencies, say about 97% of the time: Premature quotation is the root of public humiliation."

Offline killerbot

  • Administrator
  • Lives here!
  • *****
  • Posts: 5490
Re: Is this a memory leak?
« Reply #8 on: January 24, 2006, 08:59:55 am »
we just need to learn wx that well, that we know, when we give it away or not. Read the wx manual from cover to cover.  8)

Offline thomas

  • Administrator
  • Lives here!
  • *****
  • Posts: 3979
Re: Is this a memory leak?
« Reply #9 on: January 24, 2006, 09:48:03 am »
Oh, it is not that bad really:
http://www.google.com/search?q=%22responsible+for+deleting%22+site%3Ahttp%3A%2F%2Fwww.wxwidgets.org%2Fmanuals%2F2.6.2&btnG=Suche&meta=
http://www.google.com/search?hl=de&q=ownership+site%3Ahttp%3A%2F%2Fwww.wxwidgets.org%2Fmanuals%2F2.6.2&btnG=Suche&lr=

The only important one may be the one in wxProcess, but you will likely not need to use that anyway. The one in wxMenu only applies if you manually detach it (you never do that, so forget it), and the others are not important for different reasons (mostly because we either don't use them at all, or because we do use them globally, so you don't have to worry about freeing them).

It may of course be possible that this search missed one or two special cases, but that does not matter, either. The worst thing to happen is that you waste a few hundred bytes of memory... it might matter if it were about a thousand objects, but one leaked object really is no problem.
The only category of objects which we allocate in thousands is GUI components, and we know that we don't own these.
"We should forget about small efficiencies, say about 97% of the time: Premature quotation is the root of public humiliation."

takeshimiya

  • Guest
Re: Is this a memory leak?
« Reply #10 on: January 24, 2006, 01:02:14 pm »
but one leaked object really is no problem.

And at that time is where Valgrind comes in handy. Is there anything remotely simmilar for Windows?

Offline Ceniza

  • Developer
  • Lives here!
  • *****
  • Posts: 1441
    • CenizaSOFT
Re: Is this a memory leak?
« Reply #11 on: January 24, 2006, 02:40:45 pm »
Quote from: Takeshi Miya
And at that time is where Valgrind comes in handy. Is there anything remotely simmilar for Windows?

Rational Purify ($780)

Offline killerbot

  • Administrator
  • Lives here!
  • *****
  • Posts: 5490
Re: Is this a memory leak?
« Reply #12 on: January 24, 2006, 02:55:51 pm »
Compuware Numega DevPartner (for M$ DevStudio)

Offline Michael

  • Lives here!
  • ****
  • Posts: 1608
Re: Is this a memory leak?
« Reply #13 on: January 24, 2006, 03:05:01 pm »
Quote from: Takeshi Miya
And at that time is where Valgrind comes in handy. Is there anything remotely simmilar for Windows?

Rational Purify ($780)

Yes, but you can get a special license (I do not remember exactly which one :() and then get it for free (and other software from IBM too) :D.

Michael

[EDIT] Purify is available as trial (it is a bit annoying to go throught the process :(). Anyway, you can get it for free if you are accepted by the IBM Scholars Programm (which is not bad :)).
« Last Edit: January 27, 2006, 02:15:47 pm by Michael »

Offline Michael

  • Lives here!
  • ****
  • Posts: 1608
Re: Is this a memory leak?
« Reply #14 on: January 24, 2006, 03:08:58 pm »
May be this topic would be worth.

There are some posts in this forum about memory leaks. May be it could be interesting to have a look at them.

Michael

Offline 280Z28

  • Regular
  • ***
  • Posts: 397
  • *insert unicode here*
Re: Is this a memory leak?
« Reply #15 on: January 24, 2006, 03:42:12 pm »
When my copy of Code::Blocks is running in debug mode in Visual Studio, it dumps a list of all non-freed objects at exit including size of the allocation and the filename and line number where they were allocated.  :shock: Some feature of wxWidgets that came out in the VS debugger??
78 280Z, "a few bolt-ons" - 12.71@109.04
99 Trans Am, "Daily Driver" - 525rwhp/475rwtq
 Check out The Sam Zone :cool:

takeshimiya

  • Guest
Re: Is this a memory leak?
« Reply #16 on: January 24, 2006, 06:06:54 pm »
Seems I forgot to add the word "opensource" and "free" to it. :lol:

When my copy of Code::Blocks is running in debug mode in Visual Studio, it dumps a list of all non-freed objects at exit including size of the allocation and the filename and line number where they were allocated.  :shock: Some feature of wxWidgets that came out in the VS debugger??

Seems you have noticed it. :) The redefinition of operator new (which thomas likes to ditch very often :P) haves a real and very useful purpose.
And it's only enabled when you are in debug build. But anyways, you can disable it by settings wxUSE_DEBUG_NEW_ALWAYS to 0 in setup.h.

For wxDebugContext to do its work, the new and delete operators for wxObject have been redefined to store extra information about dynamically allocated objects (but not statically declared objects). This slows down a debugging version of an application, but can find difficult-to-detect memory leaks (objects are not deallocated), overwrites (writing past the end of your object) and underwrites (writing to memory in front of the object).


Here's something very useful about the class wxDebugContext:

Quote
wxDebugContext is a class for performing various debugging and memory tracing operations.

This class has only static data and function members, and there should be no instances. Probably the most useful members are SetFile (for directing output to a file, instead of the default standard error or debugger output); Dump (for dumping the dynamically allocated objects) and PrintStatistics (for dumping information about allocation of objects). You can also call Check to check memory blocks for integrity.

Here's an example of use. The SetCheckpoint ensures that only the allocations done after the checkpoint will be dumped.

Code: cpp
  wxDebugContext::SetCheckpoint();

  wxDebugContext::SetFile("c:\\temp\\debug.log");

  wxString *thing = new wxString;

  char *ordinaryNonObject = new char[1000];

  wxDebugContext::Dump();
  wxDebugContext::PrintStatistics();