Fix memory leak when adding the same key to the logger hash map multiple times

[jacobperron]

The first time a key is added the hash map will retain a copy of the pointer and later will deallocate the memory during shutdown. Subsequent times the same key is added to the hash map, it will NOT retain a copy of the new pointer, and hence not deallocate the memory during shutdown.

This fixes the issue by checking if we're adding an entry for an existing key and if so, deallocate the key.

Previously, ASAN was reporting memory leaks: https://ci.ros2.org/view/nightly/job/nightly_linux_address_sanitizer/1287/
With this change, we no longer see the memory leak reported: https://ci.ros2.org/job/ci_linux/17486/

[clalancette]

Yikes, good catch.

Actually, we can improve performance here if we do this a slightly different way. What I'm thinking here is that we do the following:

1. On entry to the function, check to see if the passed-in name is a key in the hashmap.
2. If it is, set copy_name to name, since we know that we aren't handing ownership of it to the hashmap.
3. If it isn't, strdup name into copy_name, since we are handing ownership to the hashmap.
4. Run the rest of the function as-is.

That way, for the case where we are updating the level, we never bother doing the strdup/deallocate, since it is unnecessary. What do you think?

[jacobperron]

Makes sense to me. I've updated the logic to avoid the extra allocation/deallocation: cf76443

[jacobperron]

ASAN: (unrelated warning)

CI (testing above rcutils):

• Linux
• Linux-aarch64
• Windows

[clalancette]

Looks reasonable to me with green CI and ASAN. Thanks for the comments as well; those are really helpful.

[fujitatomoya]

lgtm 👍