micro-ROS / rmw_microxrcedds

RMW implementation using Micro XRCE-DDS middleware.
Apache License 2.0
32 stars 25 forks source link

Crash when rcl_timer_reset is called #208

Closed amfern closed 2 years ago

amfern commented 2 years ago

Describe the bug When I call rcl_timer_reset on a timer, microros will crash

Additional context I tracked the issue to the "Static operation" commit https://github.com/micro-ROS/rmw_microxrcedds/commit/cb4f5220b5666be270ac29820ccd3e7a19f267f1.

And I tracked it down the crash to rmw_trigger_guard_condition at line https://github.com/micro-ROS/rmw_microxrcedds/blob/5de3d7687fff033dffa62594c3ead55ced76c724/rmw_microxrcedds_c/src/rmw_trigger_guard_condition.c#L34

Before the "static operation" commit rmw_create_guard_condition() would allocate the data field dynamiclly and doing so again, does fix the crash. so the function looks, like this:

rmw_guard_condition_t *
rmw_create_guard_condition(
  rmw_context_t * context)
{
  (void)context;

  rmw_uxrce_mempool_item_t * memory_node = get_memory(&guard_condition_memory);
  if (!memory_node) {
      // POI - it fails here.. it retuns here NULL
    RMW_SET_ERROR_MSG("Not available memory node");
    return NULL;
  }
  rmw_uxrce_guard_condition_t * aux_guard_condition =
    (rmw_uxrce_guard_condition_t *)memory_node->data;
  aux_guard_condition->hasTriggered = false;
  aux_guard_condition->rmw_guard_condition.context = context;
  aux_guard_condition->rmw_guard_condition.implementation_identifier =
    rmw_get_implementation_identifier();

+  aux_guard_condition->rmw_guard_condition.data = (bool *)rmw_allocate(sizeof(bool));
+  bool * hasTriggered = (bool *)aux_guard_condition->rmw_guard_condition.data;
+  *hasTriggered = false;

  return &aux_guard_condition->rmw_guard_condition;
}

Although I understand it's not the appropriate fix, but I hope it will be enough information for you to confirm this bug.

pablogs9 commented 2 years ago

Hello @amfern, thanks for this. Let me know if using this patch this is solved: https://github.com/micro-ROS/rmw_microxrcedds/pull/209

amfern commented 2 years ago

Hi @pablogs9, i see #209 merged into main branch. so i tried to compile main branch but i get compilation error

/uros_ws/firmware/mcu_ws/uros/rmw_microxrcedds/rmw_microxrcedds_c/src/rmw_node.c:237:13: error: dereferencing pointer to incomplete type 'rmw_context_impl_t' {aka 'struct rmw_context_impl_t'}
  237 |     &context->graph_guard_condition;
      |             ^~
make[2]: *** [CMakeFiles/rmw_microxrcedds.dir/build.make:274: CMakeFiles/rmw_microxrcedds.dir/src/rmw_node.c.obj] Error 1
pablogs9 commented 2 years ago

It has been backported to galactic and foxy, please use the branch corresponding to your ROS 2 distro. I'm not sure this error is related to the PR but with a distro mismatch.

amfern commented 2 years ago

Sorry, for that.

I compiled the galactic branch, and it fixed the crash. thank you

pablogs9 commented 2 years ago

Cool, thanks a lot for the feedback!

Closing this