Ticket #8487 (closed: fixed)

Opened 7 years ago

Last modified 5 years ago

Fix unreleased use of POCO::XML document and nodelists in Geometry code

Reported by: Karl Palmen Owned by: Russell Taylor
Priority: major Milestone: Release 3.2
Component: Framework Keywords: maintenance
Cc: Blocked By: #9118
Blocking: #8360, #10637 Tester: Nick Draper

Description (last modified by Karl Palmen) (diff)

In the code of framework/Geometry I found numerous cases of POCO::XML document or nodelist objects being created, but not released. This could cause memory leakage. Fix by using AutPtr for the object.

Change History

comment:1 Changed 7 years ago by Karl Palmen

AutoPtr is preferred to release().

comment:2 Changed 7 years ago by Karl Palmen

  • Description modified (diff)

comment:3 Changed 7 years ago by Karl Palmen

  • Description modified (diff)

comment:4 Changed 7 years ago by Russell Taylor

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

comment:5 Changed 7 years ago by Russell Taylor

  • Blocking 8360 added

comment:6 Changed 7 years ago by Karl Palmen

  • Owner changed from Russell Taylor to Karl Palmen
  • Status changed from new to inprogress

comment:7 Changed 7 years ago by Russell Taylor

  • Owner changed from Karl Palmen to Russell Taylor

Karl, I've already made a start on this as it links in with memory leaks I'm fixing.

comment:8 Changed 7 years ago by Karl Palmen

OK

comment:9 Changed 7 years ago by Russell Taylor

Re #8487. Use Poco::AutoPtr in place of manual calls to release().

Changeset: 312716582989ea547e1f5304551bea254e8fcada

comment:10 Changed 7 years ago by Russell Taylor

  • Blocked By 9118 added

comment:11 Changed 7 years ago by Russell Taylor

Re #8487. Use Poco::AutoPtr in place of manual calls to release().

Changeset: fcb35d8d19a9117cc678942d0c8e0da4fd9aeae2

comment:12 Changed 7 years ago by Russell Taylor

Re #8487. Remove trial print added in #9118 & left in by mistake.

Changeset: b3d6ff84c1dc0a89c5d7c645198d59b247e93a7e

comment:13 Changed 7 years ago by Russell Taylor

Re #8487. Don't leak memory due to Poco calls.

Changeset: 3298f7808bece5b2f18c9d5f15378ea0ed16a9f8

comment:14 Changed 7 years ago by Russell Taylor

Re #8487. Ensure Poco XML call doesn't leak memory.

Changeset: 668b03e2cfcd31426b368ee98a61c78e265c6189

comment:15 Changed 7 years ago by Russell Taylor

Re #8487. Eliminate Poco-related leaks in test.

Changeset: ce7af193503a0fe50227e16c896a1f6ab78003b2

comment:16 Changed 7 years ago by Russell Taylor

Re #8487. Replace manual calls to release with AutoPtr use.

Changeset: a0f15138715ba280239583942ee33a38b402e2e0

comment:17 Changed 7 years ago by Russell Taylor

Re #8487. Replace tabs with spaces.

No code changes.

Changeset: bfc2d4d0066d1ffc6c71dc85aac95ba0edf83f1a

comment:18 Changed 7 years ago by Russell Taylor

Re #8487. Replace manual call to release with AutoPtr.

Changeset: ec7417f0e3e2d9e1a0adac60a3a601d57aa89608

comment:19 Changed 7 years ago by Russell Taylor

Re #8487. Replace tabs with spaces.

No code changed.

Changeset: 07e28e51211fc20ac99c3df66ec2c6ab4bfa1d1c

comment:20 Changed 7 years ago by Russell Taylor

Re #8487. Hold document object in AutoPtr instead of raw pointer.

Changeset: a1b1c3a526ff7bde42cf3bde5d0b25c38b5e595e

comment:21 Changed 7 years ago by Russell Taylor

Re #8487. Minimize the number of headers included.

Changeset: 6befd24c8e5a503f0877a3cbdbdab728422eb72a

comment:22 Changed 7 years ago by Russell Taylor

Re #8487. Replace manual calls to release with AutoPtr.

Changeset: f18a6fe3da2aca7a52fac41e9bbf84b200c4f770

comment:23 Changed 7 years ago by Russell Taylor

Re #8487. It seems that Windows wants this include.

Changeset: 38955ae37bc0253685b8a3997613bc24d761f37d

comment:24 Changed 7 years ago by Russell Taylor

Re #8487. Put include in right place to make Windows happy.

Also clear up a few unused headers.

Changeset: 168e8c4b4d4b6c9caf2ddb49408ecf7c7fda5666

comment:25 Changed 7 years ago by Russell Taylor

Re #8487. Give up and put the include back in the header.

The strange thing is that my local Windows build was just fine...

Changeset: 8f5741a251943fb3a84c758e2856f39bc0611359

comment:26 Changed 7 years ago by Russell Taylor

  • Status changed from inprogress to verify
  • Resolution set to fixed

That should be all the Poco::XML code using AutoPtr now instead of release (or leaking), with the exception of vtkGeometryCacheWriter/Reader - these classes do not appear to be unit tested so I don't want to mess with them. The release calls are in the destructor, so it should be exception-safe.

Check the there are no more calls to release() anywhere, and that any calls to things like parseString(), getElementsByTagName() & create*Node() are assigning into AutoPtr.

comment:27 Changed 7 years ago by Nick Draper

  • Status changed from verify to verifying
  • Tester set to Nick Draper

comment:28 Changed 7 years ago by Nick Draper

The only remaining calls to release are in the vtkcache writer/reader, and I confirm they have the appropriate release calls in their destructors.

All the other specified searches did not come up with any that have been missed.

comment:29 Changed 7 years ago by Nick Draper

  • Status changed from verifying to closed

Merge remote-tracking branch 'origin/bugfix/8487_geometry_poco_xml_leaks'

Full changeset: ​7f15bfac171b3bee4aafd76fd7af229f633c442e

comment:30 Changed 6 years ago by Federico M Pouzols

  • Blocking 10637 added

comment:31 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue ​9331

Note: See TracTickets for help on using tickets.