DDMAL / libmei

C++ library and Python bindings for the Music Encoding Initiative format
http://music-encoding.org
Other
63 stars 23 forks source link

memory leak from setRootElement #127

Open andrewacashner opened 2 years ago

andrewacashner commented 2 years ago

I am seeing a memory leak with the simplest example document:

test.cpp:

#include <mei/mei.h>
#include <mei/shared.h>

int main(){
    mei::MeiDocument *doc = new mei::MeiDocument();
    mei::Mei *mei = new mei::Mei();
    doc->setRootElement(mei);
    delete doc;
}

I compile with this command:

clang++ -Wall -Werror -Wextra -Wpedantic -lmei -O1 -g -fsanitize=address -fno-omit-frame-pointer -o test test.cpp

This is the output from clang about the memory leak:


=================================================================
==16660==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 80 byte(s) in 1 object(s) allocated from:
    #0 0x4fd927 in operator new(unsigned long) (/[dir]/test+0x4fd927)
    #1 0x7fd379584aa8 in mei::MeiDocument::setRootElement(mei::MeiElement*) (/usr/local/lib/libmei.so+0xf3aa8)
    #2 0x50065e in main /[dir]/test.cpp:7:10
    #3 0x7fd378f9e55f in __libc_start_call_main (/lib64/libc.so.6+0x2d55f)

Direct leak of 80 byte(s) in 1 object(s) allocated from:
    #0 0x4fd927 in operator new(unsigned long) (/[dir[/test+0x4fd927)
    #1 0x7fd379584b4e in mei::MeiDocument::setRootElement(mei::MeiElement*) (/usr/local/lib/libmei.so+0xf3b4e)
    #2 0x50065e in main /[dir]/test.cpp:7:10
    #3 0x7fd378f9e55f in __libc_start_call_main (/lib64/libc.so.6+0x2d55f)

Indirect leak of 37 byte(s) in 1 object(s) allocated from:
    #0 0x4fd927 in operator new(unsigned long) (/[dir]/test+0x4fd927)
    #1 0x7fd3793b7181 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_assign(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (/lib64/libstdc++.so.6+0x145181)

SUMMARY: AddressSanitizer: 197 byte(s) leaked in 3 allocation(s).
ahankinson commented 2 years ago

If you add a delete statement in the document destructor does it fix it?

https://github.com/rism-digital/libmei/blob/develop/src/meidocument.cpp#L56

Something like delete this->root?

andrewacashner commented 2 years ago

I think I found the solution. The problem is with the destructor for MeiElement. Instead of calling attributes.clear() I looped through the vector and delete each element, as you already do in ~MeiDocument(). (I don't know why it's not necessary to do the same for the children vector.) I changed the destructor to the following and I was able to compile the sample program above with no complaints from clang:

In src/meielement.cpp at line 32:

mei::MeiElement::~MeiElement() {
    vector<MeiAttribute*>::iterator attr;
    for(attr = attributes.begin(); attr != attributes.end(); ++attr) {
        delete *attr;
    }
}