aerys / minko

3D framework for web, desktop and mobile devices.
http://minko.io
Other
904 stars 210 forks source link

Potential memory leaks #227

Closed chenxx08 closed 4 years ago

chenxx08 commented 8 years ago
std::weak_ptr<Canvas> wCanvas;
{
    auto canvas = Canvas::create("Minko Example - Cube");
    wCanvas = canvas;
    ...
    canvas->run();
}
auto useCount = wCanvas.use_count();

The useCount is not zero, the canvas and other objects(sceneManager, root, etc.) didn't disposed. If I create the Minko window several times and then close in my program, memory usage will be more and more.

Sorry for my poor English.

warrenseine commented 8 years ago

Indeed.

However, since re-creating multiple SDL windows is a very specific use case, we don't intend to fix this. A pull request is welcome!

JMLX42 commented 8 years ago

@warrenseine I think we've made lots of memory improvements for some of our latest customer projects and the 3D player on minko.io. Are those changes available on dev ?

chenxx08 commented 8 years ago

If Minko is not just for game, the case should be more common. I use it on the Android and iOS to view 3D model. User maybe repeatedly navigate to the 3D model view.

This problem should be caused by circular reference that is very common in this project. Do you have any suggestion to solve this problem. Thank you!

JMLX42 commented 8 years ago

This problem should be caused by circular reference that is very common in this project. Do you have any suggestion to solve this problem. Thank you!

This is exactly the kind of things we fixed using std::weak_ptr for example. @warrenseine ?

warrenseine commented 8 years ago

In that specific case, I think I know where the leak is. We keep the last allocated canvas in a static variable. See Canvas::defaultCanvas().

@promethe42 Changing this to a weak_ptr should safely fix the issue. @chenxx08 Can you try and tell us if the leak goes away?

chenxx08 commented 8 years ago

The problem still exsit. It's caused by circular reference. As shown in the following code.

//Canvas.hpp class Canvas: { ... std::shared_ptrinput::SDLMouse _mouse; ... }

//Canvas.cpp void Canvas::initializeInputs() { _mouse = input::SDLMouse::create(shared_from_this()); ... }

//Mouse.hpp class Mouse { ... std::shared_ptr _canvas; }

//Mouse.cpp Mouse::Mouse(std::shared_ptr canvas) : _canvas(canvas), ... { }

Touch, Joystick and some other class also have the same problem. Some class like Node fixed in Dev branch by use weak_ptr, some class still not use weak_ptr.

And some code like below may also cause circular reference.

_classField = someSignal.connect(std::bind( &Class::someMethod, std::dynamic_pointer_cast(shared_from_this()), ... ));

It's can be modified like below: _classField = someSignal.connect(std::bind( &Class::someMethod, this, ... ));

JMLX42 commented 8 years ago

Did you test the latest version on dev ?