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 when subscribing to messages containing UInt8Array #842

Closed
tytremblay opened this issue Mar 23, 2022 · 7 comments
Closed

Memory Leak when subscribing to messages containing UInt8Array #842

tytremblay opened this issue Mar 23, 2022 · 7 comments
Labels

Comments

@tytremblay
Copy link

Description
It seems memory is not getting released for subscribers subscribing to messages containing a UInt8Array. If a sufficiently large array is published, the memory of the Node.js process can grow very quickly.

  • Library Version: ^0.21.0
  • ROS Version: foxy
  • Platform / OS: Ubuntu 20.04.3 LTS (64-bit)

Steps To Reproduce
Attached is a small nodejs example of the issue I'm seeing. In the example, I have set up a publisher to publish a large image every 100ms. I've set up a subscriber to subscribe to that topic, and print process.memoryUsage().rss each time it receives a message.

The example can be run by running npx ts-node server.ts in a terminal window and npx ts-node client.ts in another window.

When both are running, you can see that the memory footprint of the node process running the client grows whenever a message is received.

Expected Behavior
Memory should not grow with each message

Actual Behavior
Memory grows with each message
rosleak.zip

@tytremblay tytremblay added the bug label Mar 23, 2022
@minggangw
Copy link
Member

Thanks for your feedback, rclnodejs replaced one of its dependencies, ref-napi, which may cause this memory leak. Would you please help to test with v0.20.1 & nodejs12 to see if you can still reproduce it? Thanks!

@tytremblay
Copy link
Author

tytremblay commented Mar 24, 2022

I am unable to reproduce on v0.20.1 and node 12. I performed the test using the project attached to the original message.

After 750 messages, the RSS size was ~200MB. I repeated the same test with v0.21.0 and node 16 and, after 750 messages, the RSS size was ~950MB

@minggangw
Copy link
Member

Thanks for testing it! So I am pretty sure it is the change of ref-napi that leads to the memory leak, thanks for finding this critical issue! will fix it asap.

minggangw added a commit that referenced this issue Apr 2, 2022
This patch fixed the memory leak when using typed arrays on nodejs > 10

Fix: #842
minggangw added a commit that referenced this issue Apr 2, 2022
This patch fixed the memory leak when using typed arrays on nodejs > 10

Fix: #842
@minggangw
Copy link
Member

@tytremblay please try with the latest 0.21.1 to check if the issue is gone, thanks!

@tytremblay
Copy link
Author

Thank you! The issue seems to be resolved. I updated to 0.21.1 and ran the above project. After 2000 messages, the RSS Size was ~200MB as expected.

@minggangw
Copy link
Member

@tytremblay good to know and thanks for your feedback!

@xhy279
Copy link
Contributor

xhy279 commented Apr 25, 2022

Ty for the fix. Good job man.

minggangw added a commit that referenced this issue Jun 16, 2022
This patch fixed the memory leak when using typed arrays on nodejs > 10

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

Successfully merging a pull request may close this issue.

3 participants