c++c++buildervclwcsdup

_wcsdup memory is not being freed in C++Builder

I am having trouble freeing the memory allocated by _wcsdup() to create a TObject pointer for a TComboBox. I thought this memory was freed automatically when the TComboBox or the Parent TForm was freed?

I am using the TComboBox function AddItem() to add visible text and a pointer value to the TComboBox. When I exit my application, my memory leak detector says the memory allocated by _wcsdup() has not been freed.

My code is below.

int i=0;
UnicodeString Description;
UnicodeString Symbol;
wchar_t *MyPointerValue;

for(i=0; i<MyStringList1->Count; i++){
  Description = MyStringList1->Strings[i];
  Symbol      = MyStringList2->Strings[i];
  MyPointerValue = _wcsdup(Symbol.c_str());

  ComboBox1->AddItem(Description, (TObject *) MyPointerValue );
}

I have tried adding the code, if(MyPointerValue != NULL){ free(MyPointerValue); } inside my for loop seen below, but this frees the pointer memory so nothing is added to the TComboBox. This does resolve the issue of my memory leak software finding unfreed memory.

for(i=0; i<MyStringList1->Count; i++){
  Description = MyStringList1->Strings[i];
  Symbol      = MyStringList2->Strings[i];
  MyPointerValue = _wcsdup(Symbol.c_str());

  ComboBox1->AddItem(Description, (TObject *) MyPointerValue );

  if(MyPointerValue != NULL){
    free(MyPointerValue);
  }
}

I also tried to free the ComboBox TObject pointer memory directly by adding the code below to the FormClose event. The call to ->Free(); generates an error.

for(i=ComboBox1->Items->Count-1; i>-1; i--){
  if( ComboBox1->Items->Objects[i] ){
    ComboBox1->Items->Objects[i]->Free();
  }
}

How can I make sure the _wcsdup() memory is always freed?

As a note, I am using C++Builder 12.2 Update 2 with the Win64X VCL platform.

Solution

  • I am having trouble freeing the memory allocated by _wcsdup() to create a TObject pointer for a TComboBox.

    I would not recommend using _wcsdup() for this task. Use new instead to create and store UnicodeString objects, eg:

    for(int i = 0; i < MyStringList1->Count; ++i)
{
  UnicodeString Description     = MyStringList1->Strings[i];
  UnicodeString Symbol          = MyStringList2->Strings[i];
  UnicodeString *MyPointerValue = new UnicodeString(Symbol);

  ComboBox1->AddItem(Description, (TObject *) MyPointerValue );
}

    I thought this memory was freed automatically when the TComboBox or the Parent TForm was freed.

    No, the memory is not freed. Memory that you allocate manually with _wcsdup() (or new) must be freed manually with free() (or delete).

    Even if you stored actual TObject-based objects into the TComboBox::Items->Objects[] property, the VCL still would not free the memory automatically. TComboBox simply does not own the TObject pointers, they are arbitrary data that you own, the VCL makes no assumptions about what they point at.

    Contrast this with the TStringList::Objects[] property, which can take ownership and free the TObjects automatically, by setting the TStringList::OwnsObjects property to true. But, this only works for valid TObject-based objects, not memory allocated with _wcsdup/new.

    I am using the TComboBox function AddItem() to add visible text and a pointer value to the TComboBox. When I exit my application, my memory leak detector says the memory allocated by _wcsdup() has not been freed.

    That is correct, because you did not free the memory that you allocated manually.

    I have tried adding the code, if(MyPointerValue != NULL){ free(MyPointerValue); } inside my for loop seen below, but this frees the pointer memory so nothing is added to the TComboBox. This does resolve the issue of my memory leak software finding unfreed memory.

    That is exactly what you need to do. However, you are doing it in the wrong place. DON'T do in the for loop that populates the TComboBox. Do it after you are done using the TComboBox items, such as during program shutdown, or when you re-populate the TComboBox with new items.

    I also tried to free the ComboBox TObject pointer memory directly by adding the code below to the FormClose event. The call to ->Free(); generates an error.

    That is the correct way to go, however you must use free() instead of TObject::Free(), as you are not storing pointers to valid TObject-based objects, which is why calling TObject::Free() crashes.

    Try something more like this instead:

    void __fastcall TMyForm::FormClose(TObjct* Sender, TCloseAction &Action)
{
  ClearComboBox();
}

void __fastcall TMyForm::PopulateComboBox()
{
  ClearComboBox();

  for(int i = 0; i < MyStringList1->Count; ++i)
  {
    UnicodeString Description = MyStringList1->Strings[i];
    UnicodeString Symbol      = MyStringList2->Strings[i];

    wchar_t *MyPointerValue   = _wcsdup(Symbol.c_str());
    // or:
    UnicodeString *MyPointerValue = new UnicodeString(Symbol);

    ComboBox1->AddItem(Description, (TObject *) MyPointerValue );
  }
}

void __fastcall TMyForm::ClearComboBox()
{
  for(int i = ComboBox1->Items->Count-1; i > -1; --i)
  {
    free( (wchar_t*) ComboBox1->Items->Objects[i] );
    // or:
    delete (UnicodeString*) ComboBox1->Items->Objects[i];
  }
  ComboBox1->Clear();
}

    And then you can retrieve the Symbol string when needed:

    int index = ... ; // such as ComboBox1->ItemIndex...

wchar_t *Symbol = (wchar_t) ComboBox1->Items->Objects[index];
// or:
UnicodeString Symbol = * (UnicodeString) ComboBox1->Items->Objects[index];

// use Symbol as needed ...

    Now, that being said, if MyStringList2 remains valid, and its strings are not added/removed, while the TComboBox is populated, then a simpler solution would be to store indexes in the ComboBox's Objects[] property instead of storing the actual strings. That way, you don't have to worry about freeing memory in the ComboBox, eg:

    void __fastcall TMyForm::PopulateComboBox()
{
  ComboBox1->Clear();

  for(int i = 0; i < MyStringList1->Count; ++i)
  {
    UnicodeString Description = MyStringList1->Strings[i];
    UnicodeString Symbol      = MyStringList2->Strings[i];

    ComboBox1->AddItem(Description, (TObject *) (IntPtr) i );
  }
}

    And then you can retrieve the Symbol string when needed:

    int index = ... ; // such as ComboBox1->ItemIndex...

int symbolIdx = (int) (IntPtr) ComboBox1->Items->Objects[index];
UnicodeString Symbol = MyStringList2->Strings[symbolIdx];

// use Symbol as needed ...

    In which case, if the MyStringList2 strings always match with the TComboBox items, then you don't really need to store the indexes in the TComboBox at all, eg:

    void __fastcall TMyForm::PopulateComboBox()
{
  ComboBox1->Clear();

  for(int i = 0; i < MyStringList1->Count; ++i)
  {
    UnicodeString Description = MyStringList1->Strings[i];
    ComboBox1->Items->Add(Description);
  }
}

    And then you can retrieve the Symbol string when needed:

    int symbolIdx = ...; // such as ComboBox1->ItemIndex... 
UnicodeString Symbol = MyStringList2->Strings[symbolIdx];
...