Ticket #8483 (closed: fixed)

Opened 7 years ago

Last modified 5 years ago

Fix unreleased use of POCO::XML document and node lists in API code

Reported by: Karl Palmen Owned by: Russell Taylor
Priority: major Milestone: Release 3.2
Component: Framework Keywords: maintenence
Cc: Blocked By:
Blocking: Tester: Jay Rainey

Description (last modified by Karl Palmen) (diff)

In the code of framework/API I found numerous cases or POCO::XML document of 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

  • type changed from enhancement to task

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 Nick Draper

  • Status changed from new to assigned

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

comment:6 Changed 7 years ago by Russell Taylor

  • Status changed from assigned to inprogress

Re #8483. Remove unnecessary include.

Not strictly related to this ticket, but I was surprised when I made a change in ImplicitFunctionParameter.h and everything recompiled. This turned out the be the reason.

Changeset: 2b2b0e5a234456a2388d406d965bd11348d62dd5

comment:7 Changed 7 years ago by Russell Taylor

Re #8483. Don't leak memory via Poco XML calls.

Changeset: 72b1dfc7aae2386782359f2544469dcf7d01668c

comment:8 Changed 7 years ago by Russell Taylor

Re #8483. Don't leak memory in Poco XML calls.

Changeset: 9a7e81d6f44f02a84a0fbfce61243f88814b733e

comment:9 Changed 7 years ago by Russell Taylor

Re #8483. Don't leak memory in Poco XML calls.

Changeset: 816ec33386fd3418180eb00be06144e16a5b8e05

comment:10 Changed 7 years ago by Russell Taylor

Re #8483. Add forward declaration after removal of include.

Changeset: ba9ccec848ae55c9d1edcfdae76e5730de9a031e

comment:11 Changed 7 years ago by Russell Taylor

Re #8483. While I'm there, thoroughly clean includes.

Changeset: 2fd8c8cac33d86bf725f949cce10726e8ca0bfb1

comment:12 Changed 7 years ago by Russell Taylor

Re #8483. Add now-required include.

After I removed its inclusion further up the chain.

Changeset: 549ecc634de0364224ba95fb424fe97cb8caaca3

comment:13 Changed 7 years ago by Russell Taylor

Re #8483. Add forward declarations.

Previously they were being picked up via some other header that I've now cleaned.

Changeset: 206acc64558f4c62d8aa1d715504bba650ef22f1

comment:14 Changed 7 years ago by Russell Taylor

Re #8483. Add now-required include.

Changeset: d1d5691ab8f13913ea676c131f88da59c8bcf325

comment:15 Changed 7 years ago by Russell Taylor

Re #8483. Add a now-required include.

Changeset: 13f8033f222c9d561dca042ce1ad1b44ba249360

comment:16 Changed 7 years ago by Russell Taylor

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

All uses of Poco XML in API should now be using AutoPtr, release is not used anywhere. I went off-topic a little when I removed an include in a header.

Test by inspection of code and CI builds.

comment:17 Changed 7 years ago by Jay Rainey

  • Status changed from verify to verifying
  • Tester set to Jay Rainey

comment:18 Changed 7 years ago by Jay Rainey

  • Status changed from verifying to reopened
  • Resolution fixed deleted

To test this I ran various grep commands in the API directory:

  • grep -nr "Document\\*" * - returns ​one result.
  • grep -nr "Document\\*" * - returns no results.
  • grep -nr "Element\\*" * - returns many results, though this may not be within the scope of this ticket. I mention this as you have improved several Element* ​here, but not elsewhere.

comment:19 Changed 7 years ago by Russell Taylor

  • Status changed from reopened to inprogress

Re #8483. Correct the spelling of the filename.

Changeset: 8b8dc60f8f3c00b9afa5eff1455a74eb5f39d96e

comment:20 Changed 7 years ago by Russell Taylor

Re #8483. Use Poco::AutoPtr to avoid leaking memory.

Changeset: 4b4d018128576685f25bd4bcedf7d5a84d7f25cd

comment:21 Changed 7 years ago by Russell Taylor

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

Thanks for checking thoroughly! I'm not sure how I missed that Document one. The results from the Element search are OK. Most are functions that take an Element*, which is fine. Not all methods that return an Element* allocate any memory that needs freeing. In particular, the ones seen here - documentElement() and getChildElement() - do not.

comment:22 Changed 7 years ago by Jay Rainey

  • Status changed from verify to verifying

comment:23 Changed 7 years ago by Jay Rainey

  • Status changed from verifying to closed

Merge remote-tracking branch 'origin/bugfix/8483_api_xml_memory_leaks'

Full changeset: ​08c62241303d102592a1479e0bb93d5f850bb37f

comment:24 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue ​9327

Note: See TracTickets for help on using tickets.