Ticket #8360 (closed: fixed)

Opened 7 years ago

Last modified 5 years ago

Clear memory leaks in the Geometry package

Reported by: Russell Taylor Owned by: Russell Taylor
Priority: major Milestone: Release 3.2
Component: Framework Keywords: Maintenance
Cc: Blocked By: #8356, #8487
Blocking: Tester: Peter Peterson

Description

The SNS Jenkins has a valgrind job which checks the Geometry code in so far as it is exercised by the unit tests.

Using this job and/or valgrind runs of your own, eliminate all the leaks/errors that result from code in the Geometry package - i.e. under Framework/Geometry/inc/MantidGeometry or Framework/Geometry/src.

Note that this should be done after leaks coming out of the test code have been addressed (#8356).

Change History

comment:1 Changed 7 years ago by Russell Taylor

  • Blocked By 8487 added

comment:2 Changed 7 years ago by Russell Taylor

  • Owner set to Russell Taylor
  • Milestone changed from Backlog to Release 3.2

comment:3 Changed 7 years ago by Russell Taylor

  • Status changed from new to inprogress

comment:4 Changed 7 years ago by Russell Taylor

Re #8360. Add valgrind suppression file for Geometry unit tests.

OpenCascade uses some kind of custom memory allocator (see for example http://opencascade.blogspot.com/2011/06/is-my-memory-leaking-part2.html) that doesn't play nicely with valgrind. I don't think that there's a real leak here, so we want to squash the complaints.

Changeset: d02747f789932afe49bb77f68712818198d53af4

comment:5 Changed 7 years ago by Russell Taylor

Re #8360. Add valgrind suppression file for Geometry unit tests.

OpenCascade uses some kind of custom memory allocator (see for example http://opencascade.blogspot.com/2011/06/is-my-memory-leaking-part2.html) that doesn't play nicely with valgrind. I don't think that there's a real leak here, so we want to squash the complaints.

Changeset: a34b0be9c42456d6a8aaa0e6ae3418b4f0558ab7

comment:6 Changed 7 years ago by Russell Taylor

Re #8360. Make sure that the logging framework is shut down.

This line ensures that the logging framework is fully shut down and all objects cleaned up after the unit tests have run. Removes a lot of (somewhat spurious) reported memory leaks.

Changeset: 1c2b5f2362190e6efcd796ff72019284fdc24abf

comment:7 Changed 7 years ago by Russell Taylor

Re #8360. Use an std::vector rather than a C array.

Doesn't leak like the old way did as it wasn't being cleaned up at all. I'd be surprised if there's any significant impact on performance.

Changeset: 12f71525ee6bce43a81cf107eb25aa1a4b7f9178

comment:8 Changed 7 years ago by Russell Taylor

Re #8360. Be exception-safe.

Previously, the Plane objects would be leaked if setPlane threw an exception.

Changeset: fecb73ca846f83fef7bb3656f12adf707d58072d

comment:9 Changed 7 years ago by Russell Taylor

Re #8360. Suppress reported leaks from SurfaceFactory.

I don't think that these are really leaks, it's just that they are owned by the static SurfaceFactory instance.

Changeset: ac6912b95bb498003bf8e6e4431591b368bfca0d

comment:10 Changed 7 years ago by Martyn Gigg

Implement copy and assignment operators for Vertex2D

Refs #8360

Changeset: d07bd8449c86fd541e6aef6c486258b2cde93157

comment:11 Changed 7 years ago by Martyn Gigg

Implement copy and assignment operators for Quadrilateral.

Quadrilateral specialies ConvexPolygon to avoid heap allocations so the copy and assignment operators need to be aware of this. Refs #8360

Changeset: f29a36a27c3b6c93f72acc48077017664dee2aff

comment:12 Changed 7 years ago by Martyn Gigg

Fix memory leaks in LaszloIntersection code.

This only affected the cases where exceptions were thrown. Refs #8360

Changeset: 42ae264bb3f933794c84fac33733c3460dfecee2

comment:13 Changed 7 years ago by Russell Taylor

Re #8360. Add valgrind suppression file for Geometry unit tests.

OpenCascade uses some kind of custom memory allocator (see for example http://opencascade.blogspot.com/2011/06/is-my-memory-leaking-part2.html) that doesn't play nicely with valgrind. I don't think that there's a real leak here, so we want to squash the complaints.

Changeset: 1b7beb03343a86b81dc9a89644a9dd54755680ef

comment:14 Changed 7 years ago by Russell Taylor

Re #8360. Make sure that the logging framework is shut down.

This line ensures that the logging framework is fully shut down and all objects cleaned up after the unit tests have run. Removes a lot of (somewhat spurious) reported memory leaks.

Changeset: 3dc4dabb9e4bd868479000c8b3fd4f862c752358

comment:15 Changed 7 years ago by Russell Taylor

Re #8360. Use an std::vector rather than a C array.

Doesn't leak like the old way did as it wasn't being cleaned up at all. I'd be surprised if there's any significant impact on performance.

Changeset: c80c36ed16fab8dcfdabc47211955a7f9f5bf470

comment:16 Changed 7 years ago by Russell Taylor

Re #8360. Be exception-safe.

Previously, the Plane objects would be leaked if setPlane threw an exception.

Changeset: 73cb6a4e77ff5adb51d468c251aec0ec0aa0dd1e

comment:17 Changed 7 years ago by Russell Taylor

Re #8360. Suppress reported leaks from SurfaceFactory.

I don't think that these are really leaks, it's just that they are owned by the static SurfaceFactory instance.

Changeset: 602ca8662ae4a41fac51bb77e593f5911dcb2677

comment:18 Changed 7 years ago by Martyn Gigg

Implement copy and assignment operators for Vertex2D

Refs #8360

Changeset: 4757f08cba9be2f51428f657e0671e53945bd812

comment:19 Changed 7 years ago by Martyn Gigg

Implement copy and assignment operators for Quadrilateral.

Quadrilateral specialies ConvexPolygon to avoid heap allocations so the copy and assignment operators need to be aware of this. Refs #8360

Changeset: af7f8bd90ed029f48eb8fb1a40c9473eb8817c8c

comment:20 Changed 7 years ago by Martyn Gigg

Fix memory leaks in LaszloIntersection code.

This only affected the cases where exceptions were thrown. Refs #8360

Changeset: b12e386e0db17c4dc62c5ea9d74c914e536db129

comment:21 Changed 7 years ago by Russell Taylor

Merge branch 'bugfix/8360_geometry_memory_leaks' into develop

Conflicts:

Code/Tools/Valgrind/GeometryTest.supp

Re #8360.

Changeset: f8f823ad9e4ae47ad0e42e703e0528e2bc7ddf71

comment:22 Changed 7 years ago by Martyn Gigg

Add null ptr check to deleteChain

Refs #8360

Changeset: b59de9edce40438e91a3719703ce96577c255acd

comment:23 Changed 7 years ago by Martyn Gigg

Remove unnecessary deleteChain call.

The code is structured so that curIntersection will never have been created if no overlap occurs. Refs #8360

Changeset: 2b2341620c837e0062a525dd5c599443d16ca87b

comment:24 Changed 7 years ago by Russell Taylor

  • Status changed from inprogress to verify
  • Resolution set to fixed
Last edited 7 years ago by Russell Taylor (previous) (diff)

comment:25 Changed 7 years ago by Peter Peterson

  • Status changed from verify to verifying
  • Tester set to Peter Peterson

comment:26 Changed 7 years ago by Peter Peterson

  • Status changed from verifying to closed

Merge remote branch 'origin/bugfix/8360_geometry_memory_leaks'

Full changeset: 00846a22b09382d70b4191e8859e0b10712af158

comment:27 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 9205

Note: See TracTickets for help on using tickets.