Fix for memory allocation mismatches in TKOpenGl package

Here a solution for new/delete and malloc/free mismatches in TKOpenGl package.
It solves also (correctly) the delete(void *) warning.

Tested with Valgrind, no more allocation requests mismatches.

I tried to make the least invasive solution, but changes affect many files.

Ciao

Max

Attachments: 
JuryS's picture

Hi, Max. Great work!
I'm was late, but I'm create this patch for six month ago with more that 500 changes only in toolkit OpenGl. Also with valgrid I'm see many memory leaks, not only with OpenGL module. They are everywhere, in all libraries, It's like a paranoia and nobody can't do anything. And memory leaks is steel the first problem with using OCC Librarys.

Successful workings out and best regards !

Massimo Del Fedele's picture

Hi Yuriy,

I've looked just at TKOpenGl for the moment because the memory bugs made
my linked library (Ultimate++), which has an internal memory manager that
overrides operators new and delete to crash.
There are still memory leaks somewhere, I don't know if in opengl or elsewhere.
I'll take a look at them when I've a bit time.

Ciao

Max

Denis Barbier's picture

AFAICT your only problem reported on this forum with memory leaks had been http://www.opencascade.org/org/forum/thread_21696/ and the conclusion was that there is no memory leak in this example.
Once again, if you believe that there are memory leaks, you have to provide a simple self-contained test-case so that it can be investigated.

Massimo Del Fedele's picture

Hi Denis,

there are indeed memory leaks, they're spotted by my custom memory manager.
The problem is to finw WHAT do leak, because it just show me the unfreed pointer,
but to find what caused it is a bigger problem :-)
Maybe I'll take a look at it in future... with some help of my toolkit's coder, possibly.
I think it would be possible to add to it some memory tracking code that can show us where
the allocation is done.

BTW, I'm just in final phases of packaging OCC for this toolkit, and I'll post it on their site
with the Bottle demo app included, so if you want to test it it'll be quite easy.

Ciao

Max

Denis Barbier's picture

Max, an easy way to check if there are memory leaks is to run your example code within a loop. If unreleased memory grows then there are more iterations, there are indeed memory leaks. If not, this means that some static variables are initialized. They are indeed not freed, but do not cause problems since memory does not grow.

Massimo Del Fedele's picture

No, no.... my toolkit takes complete ownership of operator new and delete, and it checks them at program exit;
it can catch ALL what is allocated by him and not freed. I've still not tested if leaks grows with more usage
or not, but there's no doubt that they're there.
It will NO check allocations made by OCC custom allocator, of course, but just the one done by new/delete; so
the leaks are probably in TKOpenGl or some other packages that don't use custom allocators.

Max

JuryS's picture

May I ask you ?
Which type of memory allocation you are using ? INTEL TBB or default MMGT_OPT.
Also, what about MMGT_CLEAR ?

Massimo Del Fedele's picture

Default by now, but I'm using an external library that does custom allocation, and it does it well. It was because of it that I noticed the bugs in TKOpenGl, which leaded to app crash on startup.
IMHO for me it would be better to redirect MMGT_OPT allocator to my toolkit's one; I'll do it on my
app as it's quite well done and reentrant by default.

Ciao

Max

p-carret's picture

Hi Massimo,

Thank for your work, I've already talk many times on this forum about Memory leaks which are harmful in OCC.
The first answer is usually "check your code, occ is ok", OCC is so huge, I can understand but for commercial product it can be a disaster.

Could you please post your last patch again.
I don't know if the one in this Subject is the last one.

Best regards,

Philippe

Massimo Del Fedele's picture

Hi Philippe,

for me, this patch is the "last" one :-)
It solves the problem of memory allocation MISMATCHES, not the leaks.... for these some big work is needed.
I've no time by now to fix all leaks... most are harmless, because related to static variables unfreed at end, but I guess there are some that grows.
When I'll finish up my app, I'll check for latters.

Ciao

Max

p-carret's picture

Hi Massimo,

Thank you for your answer.

I've never used ".patch"
What is the right way ?

Ciao,

Philippe

Massimo Del Fedele's picture

inside OCC source folder, in terminal :

patch -p0 < TKOpenGl.patch

Anyways, if you don't use some toolkit that do custom allocation you won't (probably)
benefit of it. It doesn't solve any leaks, just pairs correctly new/delete and malloc/free.

Ciao

Max

Denis Barbier's picture

Hello, I am not an OCC developer, so I feel free to reply. There had been meaningful replies, even if there are not what you are expecting, for instance at
http://www.opencascade.org/org/forum/thread_9013/
http://www.opencascade.org/org/forum/thread_5253/

Massimo Del Fedele's picture

Hi Denis,

I've read the replies, but I don't agree with the explanation given.

IMHO, *any* memory allocated by an app should be freed explicitly at exit; telling that they're static blocks which don't grow or that the optimized allocator do the right job is not an excuse.
I'm using a toolkit that, overriding the main allocators, is able to detect what is freed and what not.
I didn't dig too deep in the subject, but I've seen some tenths (or hundreds...) of blocks that are *not*
freed at app exit; it's true that most are static variables (I looked at it just running a small test app and closing it immediately....) but they *should* indeed be freed anyways, for the main reason that they could hide some true harmful leaks.

I think that considerable effort should be put in resolving such leaks, harmful or not; there are some quite easy to handle (a dumb example is the allocation or optimized memory allocator....), some others quite hard, in particular in TKOpenGL part which is quite messy piece of code.

Just a small OT : the same is valid for warnings. Compiling occ with almost all warnings enabled gives tons of compilation warnings; 99% are harmless, but inside them there are some which are true bugs; they simply get lost inside the others.
Fixing warnings should be a trivial task, just a long one.... and would be meaningful only if supported by quick adoption inside OCC main tree; it makes no sense to have to rebase all such fixes on each OCC release.

Max

Denis Barbier's picture

Hi Max,

Of course I agree with everything you write here. One more point: a better handling of static variables is surely also needed to fix multithreading bugs.

JuryS's picture

Hi, Max. I make some test with original OCC librarys (6.5.1) and your patch. And here what I think about:

1. I think that is not good idea to use new operator, then memcpy and delete in your ___REALLOC____ mechanism; because std realloc is more fastly (for 100 times)
2. In some case you need to use delete[] operator when you are init massive.
3. In OpenGl_text.cxx you are need append TextDelete function
4. In OpenGl_transform_persistence.cxx you are need append TransformPersistenceDelete function.
5. Some memory is not released, because it's a static values, that are need before your application is running.

Also I have some question for you: It's a interesting, why you are using Ultimate++, there is easy license? I don't try this, but read annotation and I think that is great collection. Here is all I need, but what about certification with application that used Ultimate++?

Massimo Del Fedele's picture

1) Of course, *if* you can spot all new usages and replace with malloc, which is not easy at all.
I tell this because I tried and it didn't work. I guess that some memory is allocated in some hidden point by operator new, so you *have* to use it everywhere, so my solution.

2) delete and delete[] call both operator delete or operator delete[], which usually (but not always) point to the same de-allocator. The only difference is for non-POD types, where operator delete[] calls destructor for each allocated object. This is *not* the case of TKOpenGL.
3) ???
4) ???
5) the memory should be released anyways at end of application. If not, that's a bug.

About Ultimate++, I find it one of the best toolkits available, if not the best one.
You need a bit to be used to its mechanics, but then you'll never miss any other available toolkit.
Just to make an example, since using it I never had a single memory leak nor memory invalid access anymore. About apps that use it, there are a few; I wrote a commercial app using it too.

Ciao

Max

JuryS's picture

Thanks for your answer and good luck.