View Issue Details

IDProjectCategoryView StatusLast Update
0003844FreeCADBugpublic2021-04-21 20:00
Reporterwmayer Assigned Towmayer  
PriorityhighSeveritymajorReproducibilityhave not tried
Status closedResolutionfixed 
Target Version0.20 
Summary0003844: PVS: The pointer was not released in destructor. A memory leak is possible.
DescriptionAt 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

Relationships

related to 0003993 closedwmayer Memory leak with Python3 

Activities

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.

Related Changesets

FreeCAD: master b7aee7bf

2021-04-21 19:46:14

wmayer

Details Diff
fixes 0003844: PVS: The pointer was not released in destructor. A memory leak is possible. Affected Issues
0003844
mod - src/Gui/Command.cpp Diff File

Issue History

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