Fix memory leak in tracetools::get_symbol()

[christophebedard]

Part of ros2/ros2_tracing#34

Requires ros2/ros2_tracing#43

Requires ros2/ros2_tracing#46 to help avoid memory allocations when the tracepoint isn't enabled.

Signed-off-by: Christophe Bedard christophe.bedard@apex.ai

[christophebedard]

This is the simplest fix, but it could be improved. For example, we could first check if the tracepoint is actually enabled before calling tracetools::get_symbol() and allocating. This would require changes in tracetools, though. See ros2/ros2_tracing#43 (comment)

[alsora]

Isn't the macro sufficient to prevent using this code if tracetools is disabled?
Or do we support both "compile time disabled" and "runtime disabled" ?

[mjcarroll]

Or do we support both "compile time disabled" and "runtime disabled" ?

When building with tracepoints enabled, there will be some runtime associated here, even if the tracepoints are not actively being used. I think this is more important as we are discussing building with tracepoints by default in the binary packages.

[christophebedard]

In this case, this tracepoint is only triggered during initialization, so the overhead is fairly small. But, yes, being able to not do this kind of processing if a tracepoint is disabled at runtime would be much better.

Since that change (as described here ros2/ros2_tracing#43 (comment)) requires modifications to tracetools and to potential downstream packages (including rclcpp for this exact instance), I could first add TRACEPOINT_ENABLED(...) and DO_TRACEPOINT(...) to tracetools. Then we'd only need to modify rclcpp once.

[christophebedard]

Testing --packages-above tracetools test_tracetools rclcpp:

• Linux
• Linux-aarch64
• Windows

Includes:

• Fix memory leak in tracetools::get_symbol() ros2_tracing#43
• This PR

[mjcarroll]

Another approval for good measure

[christophebedard]

@mjcarroll thanks 😆

I can't merge this PR, so I'll let you merge it.