serious bug that might cause memory leaks

[Eleni Maria Stea]

I found a serious bug in several mir classes which was that classes with virtual functions did not have a virtual destructor as they should.

Whenever such a class is used polymorphically with an instance of a subclass being used through a pointer of "base class" type:
if an attempt is made to delete it through that pointer, only the destructor of the base class will be called resulting in a memory leak since the decision of which destructor to call is taken at compile time for non-virtual functions.

[Alexandros Frantzis (afrantzis)]

We only use such classes through shared_ptrs, which, even when upcasted, "remember" the derived class type and call the proper destructor.

Of course, that's not to say that we shouldn't fix the missing virtual destructors (we should!), just that they are not a super critical issue in our case.

[Alan Griffiths (alan-griffiths)]

This is more an "accident waiting to happen" than a serious problem. I doubt that any of these classes are deleted through a pointer to base. (They are typically created using make_shared - and the resulting pointer deletes the correct type.)

[Eleni Maria Stea (hikiko)]

hmm... I added the virtual both in the base classes and the subclasses for clarity reasons (you don't have to follow the whole hierarchy to see what is virtual) but ok, I'll remove them.

About the shared pointer now, I think if the shared pointer points to the base class we will have problem (because the destructor that will be called is chosen at compile time for the virtual functions) but if it points to the subclass we are fine.

lol @tab :-D

[Alan Griffiths (alan-griffiths)]

"About the shared pointer now, I think if the shared pointer points to the base class we will have problem (because the destructor that will be called is chosen at compile time for the virtual functions) but if it points to the subclass we are fine."

Either you're saying something irrelevant, or you've misunderstood something. To clarify what do you expect the output of this to be?

#include <memory>
#include <iostream>

struct example { ~example() { std::cout << "No problem\n"; } };

int main()
{
    std::shared_ptr<void> pointer_to_void = std::make_shared<example>();
}

[Eleni Maria Stea (hikiko)]

Ok, then! I didn't realize that shared pointers keep knowledge of the assigned type, I thought it was all based on the template parameter and thus work like regular pointers! :)

[Eleni Maria Stea (hikiko)]

Well, from what I see, there are many classes and structs that have the issue and don't make use of shared pointers (eg. inside the 3rd_party directory). I have already updated my branch to fix them.

A random example of a potential memory leak in 3rd_party tests is this:
class MockANativeWindow that inherits from ANativeWindow and ANativeWindowInterface but it doesn't have a virtual destructor and it doesn't use shared pointers.

I guess that the memory leaks in tests are not quite important but I found such problems elsewhere as well, all fixed in the proposed branch. Maybe we should use the virtual destructors from now on.

[Eleni Maria Stea (hikiko)]

(I mean when we don't make use of shared pointers)

[Daniel van Vugt (vanvugt)]

Re-linked the old branches. It's good to have a paper trail.

[PS Jenkins bot (ps-jenkins)]

Fix committed into lp:mir at revision None, scheduled for release in mir, milestone 0.0.4