From c87eb0804bc5f40659b133cab6e2ade061959645 Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Fri, 8 Apr 2016 20:48:58 -0400 Subject: [PATCH] Unreported bugs: memory leaks, assertions dismissing Tags and Label editors... Symptoms were: Edit metadata; ESC; exit audacity -- memory leaks. Edit metadata; single-click "Genre" field twice; ESC -- assertion violaion in Windows debug build. Make a label; Track > Edit Labels; single-click time field twice; esc -- also caused assertions, then memory leak at exit. However, there are still two small memory leaks at exit after using Label editor, yet unexplained. --- src/LabelDialog.cpp | 14 ++++++++++---- src/Tags.cpp | 15 +++++++++++---- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/src/LabelDialog.cpp b/src/LabelDialog.cpp index d708e75da..be46c93c6 100644 --- a/src/LabelDialog.cpp +++ b/src/LabelDialog.cpp @@ -156,7 +156,8 @@ LabelDialog::LabelDialog(wxWindow *parent, mGrid->SetColLabelValue(3,_("End Time")); // Create and remember editors. No need to DELETE these as the wxGrid will - // do it for us. + // do it for us. (The DecRef() that is needed after GetDefaultEditorForType + // becomes the duty of the wxGridCellAttr objects after we set them in the grid.) mChoiceEditor = (ChoiceEditor *) mGrid->GetDefaultEditorForType(GRID_VALUE_CHOICE); mTimeEditor = (TimeEditor *) mGrid->GetDefaultEditorForType(GRID_VALUE_TIME); @@ -168,7 +169,10 @@ LabelDialog::LabelDialog(wxWindow *parent, // Initialize and set the time column attributes attr = new wxGridCellAttr(); + + // Don't need DecRef() after this GetDefaultRendererForType. attr->SetRenderer(mGrid->GetDefaultRendererForType(GRID_VALUE_TIME)); + attr->SetEditor(mTimeEditor); attr->SetAlignment(wxALIGN_CENTER, wxALIGN_CENTER); mGrid->SetColAttr(Col_Stime, attr); @@ -800,9 +804,11 @@ void LabelDialog::OnOK(wxCommandEvent & WXUNUSED(event)) void LabelDialog::OnCancel(wxCommandEvent & WXUNUSED(event)) { if (mGrid->IsCellEditControlShown()) { - mGrid->GetCellEditor(mGrid->GetGridCursorRow(), - mGrid->GetGridCursorCol()) - ->Reset(); + auto editor = mGrid->GetCellEditor(mGrid->GetGridCursorRow(), + mGrid->GetGridCursorCol()); + editor->Reset(); + // To avoid memory leak, don't forget DecRef()! + editor->DecRef(); mGrid->HideCellEditControl(); return; } diff --git a/src/Tags.cpp b/src/Tags.cpp index 84fbd0231..80ac6d641 100644 --- a/src/Tags.cpp +++ b/src/Tags.cpp @@ -1245,9 +1245,11 @@ void TagsEditor::OnOk(wxCommandEvent & WXUNUSED(event)) void TagsEditor::OnCancel(wxCommandEvent & WXUNUSED(event)) { if (mGrid->IsCellEditControlShown()) { - mGrid->GetCellEditor(mGrid->GetGridCursorRow(), - mGrid->GetGridCursorCol()) - ->Reset(); + auto editor = mGrid->GetCellEditor(mGrid->GetGridCursorRow(), + mGrid->GetGridCursorCol()); + editor->Reset(); + // To avoid memory leak, don't forget DecRef()! + editor->DecRef(); mGrid->HideCellEditControl(); return; } @@ -1262,6 +1264,7 @@ void TagsEditor::SetEditors() for (int i = 0; i < cnt; i++) { wxString label = mGrid->GetCellValue(i, 0); if (label.CmpNoCase(LABEL_GENRE) == 0) { + // This use of GetDefaultEditorForType does not require DecRef. mGrid->SetCellEditor(i, 1, mGrid->GetDefaultEditorForType(wxT("Combo"))); } else { @@ -1285,7 +1288,11 @@ void TagsEditor::PopulateGenres() parm = parm + (i == 0 ? wxT("") : wxT(",")) + g[i]; } - mGrid->GetDefaultEditorForType(wxT("Combo"))->SetParameters(parm); + // Here was a memory leak! wxWidgets docs for wxGrid::GetDefaultEditorForType() say: + // "The caller must call DecRef() on the returned pointer." + auto editor = mGrid->GetDefaultEditorForType(wxT("Combo")); + editor->SetParameters(parm); + editor->DecRef(); } bool TagsEditor::IsWindowRectValid(const wxRect *windowRect) const