search

RobotWebTools / rclnodejs

Node.js version of ROS 2.0 client
https://docs.ros.org/en/humble/Concepts/Basic/About-Client-Libraries.html?highlight=rclnodejs#community-maintained
Apache License 2.0
311 stars 70 forks source link

Memory Leak when subscribing to messages containing UInt8Array #842

Closed tytremblay closed 2 years ago

tytremblay commented 2 years ago

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.

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

minggangw commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

tytremblay commented 2 years ago

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 commented 2 years ago

@tytremblay good to know and thanks for your feedback!

xhy279 commented 2 years ago

Ty for the fix. Good job man.