Memory issue when adding and removing time jump callbacks

[greimela-si]

Bug report

Required Info:

• Operating System: Ubuntu 16.04
• Installation type: from source
• Version or commit hash:
  https://github.com/ros2/ros2/blob/13ee60a815f170d689ffb323e4c3c2747a72407f/ros2.repos
• DDS implementation: Fast-RTPS
• Client library (if applicable): rclcpp

Steps to reproduce issue

1. Create an instance of the rcl clock struct
2. Add some time jump handler
3. Remove this time jump handler
4. Add a new time jump handler

Condensed example:

rcl_clock_add_jump_callback(&rcl_clock_, threshold, Clock::on_time_jump, some_handler);

rcl_clock_remove_jump_callback(&rcl_clock_, rclcpp::Clock::on_time_jump, some_handler)

rcl_clock_add_jump_callback(&rcl_clock_, threshold, Clock::on_time_jump, new_handler);

Expected behavior

The new time jump handler is registered in the clock.

Actual behavior

The new time jump handler is not registered in the clock, allocator.realloc() fails inside rcl_clock_add_jump_callback().
I suppose this originates in rcl_clock.jump_callbacks pointing to freed memory after rcl_clock_remove_jump_callback() has been called.
Reallocating a pointer that points to freed memory seems to be undefined behavior according to cppreference.

Additional information

One possible fix for this issue could be to use malloc() instead of realloc() in rcl_clock_add_jump_callback() if rcl_clock.num_jump_callbacks == 0.

@sloretz I think this functionality has been introduced in #284, any ideas?

[sloretz]

Oops. I think the bug could be fixed by setting rcl_clock.jump_callbacks to NULL when removing the last callback. allocator.reallocate(NULL, ...) is required to succeed which I think is meant to say it behaves like stdlib realloc and is the same as calling malloc().