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
316 stars 70 forks source link

Improve RclHandle ptr free() strategy #728

Closed wayneparrott closed 3 years ago

wayneparrott commented 3 years ago

@koonpeng recently proposed redesigning the RclHandle to make the deleter lambda function responsible for free()ing the RclHandle memory pointer rather than free()ing its ptr directly as it does now.

tldr; The RclHandle Reset() method divides clean up duties between its self and a custom deleter lambda function provided during instance initialization. The deleter function performs custom delete activities on the RclHandle's ptr. While the RclHandle directly free()s its memory ptr. Recently this split in responsibilities resulted in a double free() error where the RclHandle ptr was free()'ed by a parent RclHandle and then again freed by its RclHandle wrapper.. The resolution required a creative hack using a custom deleter function that tricked the RclHandler to bypass calling free() on its ptr. This scenario revealed the need to externalize all of the clean up of the RclHandler memory ptr. The RclHandle deleter function is a natural candidate for the memory ptr free() responsibility.

minggangw commented 3 years ago

Ping, anybody are looking at this issue, if not I am going to take it, thanks!

wayneparrott commented 3 years ago

@minggangw pls proceed.