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.

Sign up for GitHub

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

Jump to bottom

Memory leak in Menu API #4860

Closed
dnstufff opened this issue May 17, 2016 · 3 comments
Closed

Memory leak in Menu API #4860

dnstufff opened this issue May 17, 2016 · 3 comments
Assignees
@ghostoy
Milestone
0.14.x

Comments

@dnstufff
Copy link

dnstufff commented May 17, 2016 •
edited
Loading

Steps to reproduce:

  1. Have a simple app that uses the Menu API. Just package.json and index.html:
<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <script type="text/javascript">
        var menu = new nw.Menu();
        menu.append(new nw.MenuItem({ label: 'Item A' }));
    </script>
</head>
<body >
</body>
</html>
  1. Start the app
  2. Take a heap snapshot from the dev tools (Profiling)
  3. Pick Containment from the drop down menu

Observation 1: 2 Window objects are present (Why not one but anyway)
image

  1. Make a location.reload() through the console multiple times
  2. Take another heap snapshot

Observation 2: Window objects have multiplied by as many times you've reloaded the app
image

Expected:
Since the menu variable is reinitialized on every load, the old one should be cleared by the GC

This bug is present in 0.12.3 and 0.14.5 and probably in all other versions in between
@dnstufff
Copy link
Author

dnstufff commented May 17, 2016 •
edited
Loading

@rogerwang, this is a follow up on #4856. reloadDev() used to do a good job clearing the leaks.

@rogerwang rogerwang added this to the 0.14.x milestone May 17, 2016
@ghostoy
Copy link
Member

ghostoy commented May 23, 2016

The issue is caused by a partial fix for #4313 (see aa3ed91), which referenced context of AppWindow in the background page in order to preserve callbacks after navigation. I will revert that commit to prevent memory leak.

ghostoy pushed a commit to ghostoy/nw.js that referenced this issue May 23, 2016
added manual test case and doc notes for nwjs#4860
a40d486
rogerwang added a commit that referenced this issue May 23, 2016
@rogerwang
Merge pull request #4879 from ghostoy/issue-4860
841b4a1 
Revert previous fix for #4313 and added notes and tests for #4860
@rogerwang
Copy link
Member

This is fixed in git and will be available in the next nightly build.

@ghostoy ghostoy mentioned this issue May 23, 2016
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ghostoy ghostoy

Labels
None yet
Projects
None yet
Milestone
0.14.x
Development

No branches or pull requests
3 participants
@rogerwang @ghostoy @dnstufff