Memory Leaks in Igloo?

[grizzie17]

Has anyone else noticed that there are a ton of memory leaks generated
by Igloo?

I was trying to add memory leak testing for our tests "Specs" so we
could see not only did the tests pass but they also exited clean with
regards to memory, but Igloo is leaking all over the place. Does
anyone have a fix for this?

Thanks in advance.

[Joakim Karlsson]

I'll have a look at it. Thanks for reporting this.

Regards,

Joakim

[Joakim Karlsson]

I've been running the internal Igloo tests with valgrind, and all it reports are a couple of "still reachable" memory blocks. From what I can see these are not proper memory leaks.

If anyone has more input on this, I'd love to hear from you.

/Joakim

[Mike Charlton]

On 13 May 2012 18:22, Joakim Karlsson <joakim.h...@gmail.com> wrote:
> If anyone has more input on this, I'd love to hear from you.

It's been a while since I looked at the code, but I don't really remember a lot
of places where it allocates memory. And when it does, I think it uses
auto-pointers. I'll spend a couple of hours doing a code inspection today
to see if I see anything obvious.

MikeC

[Mike Charlton]

Well... I had a look through the code. There is a potential memory
leak in the Test Runner and I'm not entirely sure what the best way to
fix it is. Basically, if you don't call Run() on the TestRunner, you
will leak the ContextRunners.

The way I look at it, there's actually a problem with the way memory
is deleted here. The ContextRunners are created at static
initialization time, when the structs for the Contexts are created by
the macros. But they are deleted when we call Run(). This has a
number of problems. The first is that you can only call Run() once.
After that your ContextRunners are gone.

On a more conceptual level, these objects shouldn't actually be
deleted at run time. They should be deleted when the TestRunner class
is torn down. It's been too long since I did funky C++, but I don't
think there is any way to do that... Can you make a static
destructor? ;-)

In any case, possibly it should be explicitly called when you are sure
you don't want to run the tests any more. Or maybe it should be
called in the destructor for the TestRunner.

MikeC

P.S. The other place where you could potentially leak memory is in the
TestListeners. Adding a listener to the TestRunner, does not make the
TestRunner own the memory. You have to delete them yourself when you
are done. It might be worth making that explicit in the
documentation... Another small (but inconsequential) software error
is that you can't clear the listenerAggregator. So if you delete your
listeners, your listenerAggregator will be pointing to deleted memory
for a while.

[Joakim Karlsson]

Thanks Mike,

Good point about the inconsistency between allocating and deallocating
ContextRunners. But that wouldn't show up as a memory leak in a normal
run, right?

I'll look at the ListenerAggregator and TestListeners later.

/Joakim

[Mike Charlton]

On 15 May 2012 14:29, Joakim Karlsson <joa...@jkarlsson.com> wrote:
> Good point about the inconsistency between allocating and deallocating
> ContextRunners. But that wouldn't show up as a memory leak in a normal
> run, right?

It shouldn't. I only really bring it up because it would probably be a good
idea to fix it and there is a small chance that the person did a memory leak
check but somehow forgot to run the tests (Well... pretty unlikely...)

> I'll look at the ListenerAggregator and TestListeners later.

In my opinion, it should just be a matter of adding a method to clear
the listeners. In virtually every case it's not going to be a problem.
But again, if for some reason you want to run the tests twice with
different listeners (not possible at the moment), it would be necessary.

Again, the reason I brought it up is that the original poster may have
not realized that the memory was not owned by ContextRunner.
If you new() a listener and send the pointer to AddListener(),
you will leak memory if you don't delete it later. Personally, I think
that is correct behavior in this case. Possibly the listeners could
be passed by reference instead of by address to underline the
fact that you need to dispose of the memory yourself.

I didn't notice anything else that looked like it could leak memory,
but it's possible I overlooked something...

MikeC

[grizzie17]

It turns out the leaks are caused by the use of static data.  Using the memory leak detection that is provided with Visual C++ flagged the problem because the data is not destroyed until after the 'main()' function exits.  I figured a way around the problem so I can include memory leak detection into the "Specs".

 

Thanks...

[Joakim Karlsson]

Glad you solved your problem.

Thanks for getting back with a follow up.

Regards,
Joakim