Ticket #8483 (closed: fixed)
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: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: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