Memory Leak in BP5Deserializer

[caitlinross]

In adding a BP5 test in the Fides repo, address sanitizer found memory leaks in BP5Deserializer with the most recent version of ADIOS master branch. I see that memory leaks on the write side were brought up in #3381, but I guess the read side wasn't checked.

You can reproduce easily by modifying the ADIOS helloWriter/Reader examples. I modified helloBPWriter.cpp to use the BP5 engine, and helloBPReader.cpp to specify adios2::Mode::ReadRandomAccess in the Open call.
Build and run with valgrind or address sanitizer.

Valgrind output:

==1006506== 32 (24 direct, 8 indirect) bytes in 1 blocks are definitely lost in loss record 16 of 17
==1006506==    at 0x483BE63: operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==1006506==    by 0x611430F: adios2::format::BP5Deserializer::VarShape(adios2::core::VariableBase const&, unsigned long) const (BP5Deserializer.cpp:2089)
==1006506==    by 0x60C829F: adios2::core::engine::BP5Reader::VarShape(adios2::core::VariableBase const&, unsigned long) const (BP5Reader.cpp:689)
==1006506==    by 0x5D768E0: adios2::core::VariableBase::Shape(unsigned long) const (VariableBase.cpp:632)
==1006506==    by 0x5CDBDCD: std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > > adios2::core::IO::GetVariableInfo<int>(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::set<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&) (IO.tcc:303)
==1006506==    by 0x5CAE59C: adios2::core::IO::GetAvailableVariables(std::set<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&) (IO.cpp:400)
==1006506==    by 0x4B04928: adios2::IO::AvailableVariables[abi:cxx11](bool) (IO.cpp:131)
==1006506==    by 0x11074B: main (helloBPReader.cpp:45)
==1006506== 
==1006506== 32 (24 direct, 8 indirect) bytes in 1 blocks are definitely lost in loss record 17 of 17
==1006506==    at 0x483BE63: operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==1006506==    by 0x611430F: adios2::format::BP5Deserializer::VarShape(adios2::core::VariableBase const&, unsigned long) const (BP5Deserializer.cpp:2089)
==1006506==    by 0x60C829F: adios2::core::engine::BP5Reader::VarShape(adios2::core::VariableBase const&, unsigned long) const (BP5Reader.cpp:689)
==1006506==    by 0x5D768E0: adios2::core::VariableBase::Shape(unsigned long) const (VariableBase.cpp:632)
==1006506==    by 0x5CE1A0B: std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > > adios2::core::IO::GetVariableInfo<float>(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::set<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&) (IO.tcc:303)
==1006506==    by 0x5CAE812: adios2::core::IO::GetAvailableVariables(std::set<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&) (IO.cpp:400)
==1006506==    by 0x4B04928: adios2::IO::AvailableVariables[abi:cxx11](bool) (IO.cpp:131)
==1006506==    by 0x11074B: main (helloBPReader.cpp:45)

[eisenhauer]

Thanks! Just found that myself when working on the read struct stuff a couple of days ago, but decided not to complicate that PR with fixes for this. I need to sort some things with how VarShape works with minblocksinfo in order to fix this, and it's not immediately obvious how to do it, so I need to think a bit. (With the old BlocksInfo engines, we just returned an existing Shape object, but in minblocksinfo we don't keep data in that structure, so we have to create one. But it then doesn't get free'd the way it did before.)

[eisenhauer]

Closed by #3458