creator_to_cocos2dx icon indicating copy to clipboard operation
creator_to_cocos2dx copied to clipboard

memory leak

Open Seohanul opened this issue 8 years ago • 17 comments

when I put this code in button callback, It cause memory leak.

for (int i = 0; i < 10; ++i) { creator::CreatorReader * creatorReader = creator::CreatorReader::createWithFilename ("filename");

		creatorReader->setup ();

		/*creatorReader->autorelease ();*/

}

every time, momory goes up. how can I fix this problem?

Seohanul avatar Mar 16 '18 11:03 Seohanul

I enabled CC_REF_LEAK_DETECTION, and I did a compare with printleaks before and after loading the creatorReader, I find there are some SpriteFrameCache and SpriteFrame which are not released

Seohanul avatar Mar 19 '18 01:03 Seohanul

I did a Director::getInstance ()->purgeCachedData (); and the spriteframes are not being released, I see their ref count is 2 when purgecacheddata is called

Seohanul avatar Mar 19 '18 01:03 Seohanul

Reporting the first leak (and fix),

When creating a button, there is a child label which is created as a protectedchild node using in Ln400 of CreatorReader.cpp (button->setTitleLabel), the created Label is not set to autorelease so it will leak.

Seohanul avatar Mar 19 '18 10:03 Seohanul

Could you supply a test case to show the memory leak? for example to modify the default cpp-empty-test, add your code, and push it into GitHub, tell me the link.

And I didn't think Ln400 of CreatorReader.cpp (button->setTitleLabel) cause memory leak. the Label added into button is created by

case buffers::AnyNode_Label:
            node = createLabel(static_cast<const buffers::Label*>(buffer));
            break;

So it is setted into autorelease pool

drelaptop avatar Mar 20 '18 03:03 drelaptop

label memory leak

If label is child of button, CreatorReader.cpp (button->setTitleLabel) set a leabel to protected child. I'm not sure this can be released properly. I just set label->autorelease() after button->setTitleLabel, and no more leak.

Seohanul avatar Mar 23 '18 01:03 Seohanul

animation memory leak And there are other issue. AnimationManager not to be relased for looping animations. I play some animations at the same time as loading creator file. and looping. And after release all nodes, animation still remaining. ActionManager doesn't have fuction for stop 'all' animation or remove 'all' animation clip. And also I think looping animation is not setted into autorelease pool. How can I release looping animations properly?

Seohanul avatar Mar 23 '18 01:03 Seohanul

About the button memory leak, I think it's a bug of UIButton.cpp, to fix this, we can add CC_SAFE_RELEASE_NULL(_titleRenderer); into destructs function, like this:

Button::~Button()
{
    CC_SAFE_RELEASE_NULL(_titleRenderer);
}

another way, about the animation memory leak, I will research that. Indeed, when animation loop, exist memory leak

drelaptop avatar Mar 23 '18 09:03 drelaptop

Thank you for reply.

I have another question about animation manager.

virtual dtor

Animation Manager inherited by cocos2d::Node.

So, I think destructor of Animation Manager should be virtual. Am I right?

Seohanul avatar Mar 26 '18 02:03 Seohanul

I think so too, but it's not a matter thing for not any sub-class inherit AnimationManager.

This comment support Markdown, so you can add a code block by markdown, not need to paste a picture for a code block.

drelaptop avatar Mar 26 '18 05:03 drelaptop

Without the virtual dtor, it did not call the Node destructor properly so there was a leak.

imtrobin avatar Mar 26 '18 06:03 imtrobin

Node's destructor is virtual, it do call the Node destructor when AnimationManager destruct. I'm afraid that you are misunderstanding this.

drelaptop avatar Mar 26 '18 07:03 drelaptop

@Seohanul see commit fix loop animate memory leak ,fix PlayOnLoad loop animation leaks in PR #152 to fix the memory leak

if existing a loop animation, please stop it manually, using function:

    // if AnimationClip is stopped, can not run it again.
    void stopAnimationClip(cocos2d::Node *target, const std::string &animationClipName);
    // if a "Play On Load" animation is a loop animation, please stop it manually.
    void stopPlayOnLoad();

stopAnimationClip stop the animation started by playAnimationClip, stopPlayOnLoad stop the animation started by playOnLoad automatically.

Would you like to test this fix to make sure this patch works well? just merge that PR

drelaptop avatar Mar 26 '18 10:03 drelaptop

Why don't you call this stop on node dtor automatically? If its playOnLoad automatically, it is done so in creator studio, it should be able to unload automatically.

imtrobin avatar Mar 31 '18 08:03 imtrobin

Indeed, it's reasonable, but hard to do it in this plugin, I tried. Strictly playOnLoad isn't automatically, it's called by the CreatorReader::getSceneGraph function, stopPlayOnLoad should be put into what place, CreatorReader AnimationManager destruction function or somewhere else?

suggest to merge that PR, and have a try.

drelaptop avatar Mar 31 '18 12:03 drelaptop

How about stopPlayOnLoad called in AnimationManager destructor and stop automatically? If animations played on load has already stopped, stopPlayOnLoad do nothing because cashed animates would not be found. https://github.com/cocos2d/creator_to_cocos2dx/blob/master/creator_project/packages/creator-luacpp-support/reader/animation/AnimationManager.cpp#L65

nunu-e64 avatar Apr 03 '18 10:04 nunu-e64

Sorry, its a bad idea. Animation Manager is retained when running animation clip. So Animation Manager will not be destructed. https://github.com/cocos2d/creator_to_cocos2dx/blob/master/creator_project/packages/creator-luacpp-support/reader/animation/AnimationManager.cpp#L91

nunu-e64 avatar Apr 03 '18 11:04 nunu-e64

thanks for your review, I hold the same idea with you.

drelaptop avatar Apr 03 '18 12:04 drelaptop