Memory Error---test_params2

[shaomeng]

Utility program test_params2 has definitely lost memory errors:

==151381== LEAK SUMMARY:
==151381==    definitely lost: 104 bytes in 1 blocks
==151381==    indirectly lost: 56 bytes in 1 blocks
==151381==      possibly lost: 0 bytes in 0 blocks
==151381==    still reachable: 4,096 bytes in 1 blocks
==151381==         suppressed: 0 bytes in 0 blocks
==151381== Rerun with --leak-check=full to see details of leaked memory

[clyne]

The leaks are fixed, but there is still a memory error:

perator()(VAPoR::ParamsContainer*) const (memory:2084)
==63237== by 0x1001E476B: std::__1::unique_ptr<VAPoR::ParamsContainer, std::__1::default_deleteVAPoR::ParamsContainer >::reset(VAPoR::ParamsContainer*) (memory:2345)
==63237== by 0x1001E7E48: std::__1::unique_ptr<VAPoR::ParamsContainer, std::__1::default_deleteVAPoR::ParamsContainer >::~unique_ptr() (memory:2299)
==63237== by 0x1001E47B4: std::__1::unique_ptr<VAPoR::ParamsContainer, std::__1::default_deleteVAPoR::ParamsContainer >::~unique_ptr() (memory:2299)
==63237== by 0x1001E4EB6: VAPoR::ViewpointParams::~ViewpointParams() (lib/params/ViewpointParams.cpp:127)

==63237== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

[shaomeng]

@clyne The current memory error is because of an XmlNode being free'd and then it's accessed again.

The memory management with regard to XmlNodes and how Params make use of them seem quite complicated though, and I cannot see an easy fix to this particular error without completely revamping the XmlNode/Params code with smart pointers. Then the question becomes, do we go down that path now?

[clyne]

@shaomeng this problem looks like a regression that was introduced by your commit to ViewpointParams.cpp, which introduced smart pointers (d1063b1). It's not clear to me that smart pointers and raw pointers can be mixed in this way, but I really don't know. Perhaps the above commit should be backed out?

[shaomeng]

@clyne Yeah, that commit used a unique pointer to wrap around an instance of ParamsContainer, so that instance of ParamsContainer would later be properly destroyed. As a result, Valgrind doesn't report a "Definitely Lost" memory block anymore, and the original bug was fixed. If that commit was backed out, the ParamsContainer instance would be lost.

It seems to me that when an instance of ParamsContainer was destroyed, it destroyed some XmlNodes it owns all together, but that XmlNode was accessed by some other code, causing the illegal access bug you reported. However, the complexity of XmlNode/ParamsBase code and very frequent use of pointer assignments prevented me from pinpointing where exactly that bug is.

I don't think there's mixed use of smart and raw pointer. There's one particular pointer, named _transforms, that's managed by a smart pointer. The smart pointer manages the lifespan of variable _transforms and that's it.