View Issue Details

IDProjectCategoryView StatusLast Update
0003844FreeCADBugpublic2021-02-23 16:12
Reporterwmayer Assigned Towmayer  
PriorityhighSeveritymajorReproducibilityhave not tried
Status assignedResolutionopen 
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

developer   ~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

View 2 revisions

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

developer   ~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).

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 View Revisions
2021-02-23 16:12 chennes Note Added: 0015423