Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

memory leak #150

Open
Seohanul opened this issue Mar 16, 2018 · 17 comments
Open

memory leak #150

Seohanul opened this issue Mar 16, 2018 · 17 comments

Comments

@Seohanul
Copy link

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
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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.

@drelaptop
Copy link
Contributor

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

@Seohanul
Copy link
Author

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
Copy link
Author

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?

@drelaptop
Copy link
Contributor

drelaptop commented Mar 23, 2018

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

@Seohanul
Copy link
Author

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?

@drelaptop
Copy link
Contributor

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.

@imtrobin
Copy link

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

@drelaptop
Copy link
Contributor

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

@drelaptop
Copy link
Contributor

drelaptop commented Mar 26, 2018

@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

@imtrobin
Copy link

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.

@drelaptop
Copy link
Contributor

drelaptop commented Mar 31, 2018

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.

@nunu-e64
Copy link
Contributor

nunu-e64 commented Apr 3, 2018

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
Copy link
Contributor

nunu-e64 commented Apr 3, 2018

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

@drelaptop
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants