0003844: PVS: The pointer was not released in destructor. A memory leak is possible. - FreeCAD Tracker

NOTICE: Migration to GitHub Issues

On Feb 7, 2022, the FreeCAD project migrated all issues from this site at tracker.freecad.org to our main GitHub repository. All new bugs must be submitted there.
This Mantis repository is in read-only mode and will be retained for reference as long as it is useful.

For details please see the announcement at the Forums: https://forum.freecadweb.org/viewtopic.php?p=555883#p555883

ID	Project	Category	View Status	Date Submitted	Last Update

0003844	FreeCAD	Bug	public	2019-02-17 18:11	2021-04-21 20:00

Reporter	wmayer	Assigned To	wmayer
Priority	high	Severity	major	Reproducibility	have not tried
Status	closed	Resolution	fixed
Target Version	0.20

Summary	0003844: PVS: The pointer was not released in destructor. A memory leak is possible.
Description	At the moment the CommandBase class has a couple of member variables of type const char*. Now the strings passed to the constructor are compiled into the binaries so that in the destructors the memory mustn't be freed. However, there are also setter methods to change the strings and usually the passed arguments have only a tmp. lifetime. So, the method strdup() must be used. However, on destruction time we don't know whether the memory was allocated on the heap or not. To be on the safe side nothing is freed but in case the setter methods have been used we have a major memory leak. So, to solve all the hassles it's best to use std::string as members. This may cause a certain overhead but at least makes sure that all memory is freed.
Tags	#pending

FreeCAD Information

wmayer 2019-02-17 18:13 administrator ~0012672	General Analysis Command.cpp:742 Medium V773 The 'sScriptName' pointer was not released in destructor. A memory leak is possible. General Analysis Command.cpp:141 Medium V773 The 'sMenuText' pointer was not released in destructor. A memory leak is possible. General Analysis Command.cpp:141 Medium V773 The 'sToolTipText' pointer was not released in destructor. A memory leak is possible. General Analysis Command.cpp:141 Medium V773 The 'sWhatsThis' pointer was not released in destructor. A memory leak is possible. General Analysis Command.cpp:141 Medium V773 The 'sStatusTip' pointer was not released in destructor. A memory leak is possible. General Analysis Command.cpp:141 Medium V773 The 'sPixmap' pointer was not released in destructor. A memory leak is possible. General Analysis Command.cpp:141 Medium V773 The 'sAccel' pointer was not released in destructor. A memory leak is possible. General Analysis Command.cpp:225 Medium V773 The 'sAppModule' pointer was not released in destructor. A memory leak is possible. General Analysis Command.cpp:225 Medium V773 The 'sGroup' pointer was not released in destructor. A memory leak is possible.

Kunda1 2021-02-21 03:51 administrator ~0015405	@chennes are these still valid?

chennes 2021-02-22 15:49 administrator ~0015418	Yes, this is still valid. I believe the lines in question are Command.h 291-296, which @wmayer is suggesting should be std::string, rather than char *.

wmayer 2021-02-23 12:01 administrator ~0015422 Last edited: 2021-02-23 13:59	The only problem with std::string is that it's rather fat. While a plain C string only requires 8 bytes (as it's a pointer) std::string already has 32 bytes. But it seems QByteArray is a good alternative as this also only requires 8 bytes, too. `int s; s = sizeof(char*); // s=8 s = sizeof(QByteArray); // s=8 s = sizeof(std::string); // s=32 QByteArray str("Hello, World!"); s = str.capacity(); // s=13, i.e. it doesn't allocate more than needed` EDIT: QByteArray uses a pointer to QTypedArrayData<char> so, that's why sizeof said 8 bytes. But for QTypedArrayData<char> sizeof says it's 24 so in the end QByteArray is not better than std::string.

chennes 2021-02-23 16:12 administrator ~0015423	I did not benchmark it for memory consumption, but I have a local branch where I've done this migration (to std::string), so I can take a look later this week and see if it's a big enough memory hit to be worth worrying about (or maybe you already have a sense for that).

wmayer 2021-04-21 20:00 administrator ~0015640	Fix committed to master branch.

Date Modified	Username	Field	Change
2019-02-17 18:11	wmayer	New Issue
2019-02-17 18:11	wmayer	Status	new => assigned
2019-02-17 18:11	wmayer	Assigned To	=> wmayer
2019-02-17 18:13	wmayer	Note Added: 0012672
2019-06-20 13:17	Kunda1	Relationship added	related to 0003993
2021-02-06 06:50	abdullah	Target Version	=> 0.20
2021-02-21 03:51	Kunda1	Note Added: 0015405
2021-02-21 03:52	Kunda1	Target Version	0.20 => 0.19
2021-02-21 03:52	Kunda1	Tag Attached: #pending
2021-02-22 15:49	chennes	Note Added: 0015418
2021-02-23 12:01	wmayer	Note Added: 0015422
2021-02-23 12:02	wmayer	Target Version	0.19 => 0.20
2021-02-23 13:59	wmayer	Note Edited: 0015422
2021-02-23 16:12	chennes	Note Added: 0015423
2021-04-21 20:00	wmayer	Changeset attached	=> FreeCAD master b7aee7bf
2021-04-21 20:00	wmayer	Note Added: 0015640
2021-04-21 20:00	wmayer	Status	assigned => closed
2021-04-21 20:00	wmayer	Resolution	open => fixed