Clear memory leaks in the APITest package

[Russell Taylor]

The SNS Jenkins has a ​valgrind job which uses the unit tests as a hook into exercising the Mantid code itself. This of course means that it picks up any leaks in the test code as well.

Using this job and/or valgrind runs of your own, eliminate all the leaks/errors that come out of the test code itself. I've created this as an isolated ticket because I'd like to get a clear view of the errors we have in the actual Mantid code. All changes to this ticket should only be to code in the Framework/API/test/ directory!

When working on this, don't just add delete calls - consider whether the leaking call really needs to be a heap allocation; create the object on the stack if not.

[Nick Draper]

Bulk move of tickets out of triage (new) to assigned at the introduction of the triage state

[Russell Taylor]

Re #8357. Eliminate valgrind invalid write warning.

By making sure to unregister the observer at the end of the test.

Changeset: 845efec319cf1cd719cee923ad07370e65bf1885

[Russell Taylor]

Re #8357. Fix leak in test by breaking cyclic reference.

The BoxController & BoxControllerDummyIO objects were each holding a shared_ptr back to the other.

Changeset: 72fed1a43f3622555347e63395766e3204c62082

[Russell Taylor]

Re #8357. Fix memory leaks in tests.

By creating object on the stack instead of unnecessarily on the heap.

Changeset: c02e1bdf04ba6eacc4b14bdfbca7da0d932b9570

[Russell Taylor]

Re #8357. Remove leak in test by adding components to instrument.

The fact that you have to call two methods in succession to get this right is probably not great design. Nor is the fact that you can 'mark' something that isn't in the instrument without any error or warning.

Changeset: 4b45a3284795ff43623134ad4fcfedfe1fb9da0e

[Russell Taylor]

Re #8357. Eliminate memory leaks in tests.

By calling delete where appropriate.

Changeset: a295915fdba69374fbc2a04490c41b80896b1e46

[Russell Taylor]

Re #8357. Eliminate memory leak in test.

By removing group from within group to eliminate cyclic shared_ptr reference. Possibly there's a real potential issue here, but would anyone really ever put a group within a group?

Changeset: 9533f817885addf622af52987b94f1908f386d1c

[Russell Taylor]

Re #8357. Eliminate valgrind invalid write warning.

By making sure to unregister the observer at the end of the test.

Changeset: 80f3695a539a3a67ace1becf8091636ec88ce73d

[Russell Taylor]

Re #8357. Fix leak in test by breaking cyclic reference.

The BoxController & BoxControllerDummyIO objects were each holding a shared_ptr back to the other.

Changeset: 7053b8961e800d64e54c51bc15c89341d0845e94

[Russell Taylor]

Re #8357. Fix memory leaks in tests.

By creating object on the stack instead of unnecessarily on the heap.

Changeset: d97c3c8c89110d496960c9af431d5f64847693d1

[Russell Taylor]

Re #8357. Remove leak in test by adding components to instrument.

The fact that you have to call two methods in succession to get this right is probably not great design. Nor is the fact that you can 'mark' something that isn't in the instrument without any error or warning.

Changeset: da65363bf7f7ab8d0b8789648b5b0ed5b8faa66d

[Russell Taylor]

Re #8357. Eliminate memory leaks in tests.

By calling delete where appropriate.

Changeset: 067e4473293d2462a4caa61273494c83ffbd8005

[Russell Taylor]

Re #8357. Eliminate memory leak in test.

By removing group from within group to eliminate cyclic shared_ptr reference. Possibly there's a real potential issue here, but would anyone really ever put a group within a group?

Changeset: ee89f1bb5030406f4c1859dddbe9b2b05391e74f

[Russell Taylor]

Re #8357. Update to account for change in TestHelpers class.

The BoxControllerDummyIO constructor was changed to accept (and hold internally) a bare pointer to avoid a cyclic reference.

Changeset: 981868bdda2c8ae0fdb3e9778f1ce9b0476bb297

[Russell Taylor]

Re #8357. Add a suppressions file for the API tests.

Valgrind is complaining about an OpenMP call in AlgorithmManagerTest. I don't know whether this is a real leak, but I suppose that there's not much we can do about it.

Changeset: 3058ccfae1cb69a4f53664df2cb91053504be900

[Russell Taylor]

Re #8357. Fix memory leaks in test.

Ensure Property objects returned by loadProperty are deleted after use.

Changeset: 0de7d7e83ee1e436cfffc5089a50829e345d00bf

[Russell Taylor]

To test: This mainly needs to be tested by inspection. The way I can tell that all leaks in API/test itself are gone is because I run valgrind in Eclipse and observe the absence of a little red cross next to that directory.

[Martyn Gigg]

I ran the APITest suite through valgrind in eclipse and I'm showing up an error in MemoryManagerTest.h:37, which uses an openmp loop. The stacktrace is:

==19901== 2,352 bytes in 7 blocks are possibly lost in loss record 36 of 92
==19901==    at 0x4C29DB4: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==19901==    by 0x4012074: _dl_allocate_tls (dl-tls.c:297)
==19901==    by 0x7B6BABC: pthread_create@@GLIBC_2.2.5 (allocatestack.c:571)
==19901==    by 0x774731B: ??? (in /usr/lib/x86_64-linux-gnu/libgomp.so.1.0.0)
==19901==    by 0x86299F: _ZN17MemoryManagerTest13test_parallelEv (MemoryManagerTest.h:37)
==19901==    by 0x862CD1: _ZN53TestDescription_suite_MemoryManagerTest_test_parallel7runTestEv (MemoryManagerTest.cpp:38)
==19901==    by 0x9B6AB4: _ZN7CxxTest19RealTestDescription3runEv (RealDescriptions.cpp:96)
==19901==    by 0x9B9FDE: _ZN7CxxTest10TestRunner7runTestERNS_15TestDescriptionE (TestRunner.h:94)
==19901==    by 0x9B9ED0: _ZN7CxxTest10TestRunner8runSuiteERNS_16SuiteDescriptionE (TestRunner.h:79)
==19901==    by 0x9B9D6B: _ZN7CxxTest10TestRunner8runWorldEv (TestRunner.h:63)
==19901==    by 0x9B9B2E: _ZN7CxxTest10TestRunner11runAllTestsERNS_12TestListenerE (TestRunner.h:24)
==19901==    by 0x9BFB6D: _ZN7CxxTest12XUnitPrinter3runEv (XUnitPrinter.h:30)

It looks like something that just needs a similar suppression to the one already present for AlgorithmManager?

[Russell Taylor]

Re #8357. Add suppression for another use of OpenMP in a test.

Changeset: 111f66ab17766b1392a4ac2f6739de9fd18fa325

[Russell Taylor]

That last one didn't show up for me (or in the Jenkins job) when running the entire API suite, though it did when I ran the MemoryManagerTest on its own. Anyhow, it's suppressed now.

[Martyn Gigg]

The API test folder is now clear for me too.

[Martyn Gigg]

Merge remote-tracking branch 'origin/bugfix/8357_api_test_memory_leaks'

Full changeset: ​4e89f08a33013f4b0628c312f176d500225ea2a4

[Stuart Campbell]

This ticket has been transferred to github issue ​9202