Set disable loan to on by default. (#1110)

It is currently not safe to use; see the executor code in rclcpp for more information. Signed-off-by: Chris Lalancette <clalancette@gmail.com>
ros2 · Nov 2, 2023 · 1b01127 · 1b01127
1 parent 1b79535
commit 1b01127
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 11 deletions.
diff --git a/rcl/include/rcl/subscription.h b/rcl/include/rcl/subscription.h
@@ -209,7 +209,7 @@ rcl_subscription_fini(rcl_subscription_t * subscription, rcl_node_t * node);
  * - qos = rmw_qos_profile_default
  * - allocator = rcl_get_default_allocator()
  * - rmw_subscription_options = rmw_get_default_subscription_options();
- * - disable_loaned_message = false, true only if ROS_DISABLE_LOANED_MESSAGES=1
+ * - disable_loaned_message = true, false only if ROS_DISABLE_LOANED_MESSAGES=0
  *
  * \return A structure containing the default options for a subscription.
  */

diff --git a/rcl/src/rcl/subscription.c b/rcl/src/rcl/subscription.c
@@ -24,6 +24,7 @@ extern "C"
 #include "rcl/error_handling.h"
 #include "rcl/node.h"
 #include "rcl/node_type_cache.h"
+#include "rcutils/env.h"
 #include "rcutils/logging_macros.h"
 #include "rcutils/strdup.h"
 #include "rcutils/types/string_array.h"
@@ -233,15 +234,19 @@ rcl_subscription_get_default_options()
   default_options.rmw_subscription_options = rmw_get_default_subscription_options();
 
   // Load disable flag to LoanedMessage via environmental variable.
-  bool disable_loaned_message = false;
-  rcl_ret_t ret = rcl_get_disable_loaned_message(&disable_loaned_message);
-  if (ret == RCL_RET_OK) {
-    default_options.disable_loaned_message = disable_loaned_message;
-  } else {
+  // TODO(clalancette): This is kind of a copy of rcl_get_disable_loaned_message(), but we need
+  // more information than that function provides.
+  default_options.disable_loaned_message = true;
+
+  const char * env_val = NULL;
+  const char * env_error_str = rcutils_get_env(RCL_DISABLE_LOANED_MESSAGES_ENV_VAR, &env_val);
+  if (NULL != env_error_str) {
     RCUTILS_SAFE_FWRITE_TO_STDERR("Failed to get disable_loaned_message: ");
-    RCUTILS_SAFE_FWRITE_TO_STDERR(rcl_get_error_string().str);
-    rcl_reset_error();
-    default_options.disable_loaned_message = false;
+    RCUTILS_SAFE_FWRITE_TO_STDERR_WITH_FORMAT_STRING(
+      "Error getting env var: '" RCUTILS_STRINGIFY(RCL_DISABLE_LOANED_MESSAGES_ENV_VAR) "': %s\n",
+      env_error_str);
+  } else {
+    default_options.disable_loaned_message = !(strcmp(env_val, "0") == 0);
   }
 
   return default_options;

diff --git a/rcl/test/rcl/test_subscription.cpp b/rcl/test/rcl/test_subscription.cpp
@@ -736,7 +736,7 @@ TEST_F(CLASSNAME(TestSubscriptionFixture, RMW_IMPLEMENTATION), test_subscription
 TEST_F(CLASSNAME(TestSubscriptionFixture, RMW_IMPLEMENTATION), test_subscription_option) {
   {
     rcl_subscription_options_t subscription_options = rcl_subscription_get_default_options();
-    EXPECT_FALSE(subscription_options.disable_loaned_message);
+    EXPECT_TRUE(subscription_options.disable_loaned_message);
   }
   {
     ASSERT_TRUE(rcutils_set_env("ROS_DISABLE_LOANED_MESSAGES", "1"));
@@ -746,11 +746,16 @@ TEST_F(CLASSNAME(TestSubscriptionFixture, RMW_IMPLEMENTATION), test_subscription
   {
     ASSERT_TRUE(rcutils_set_env("ROS_DISABLE_LOANED_MESSAGES", "2"));
     rcl_subscription_options_t subscription_options = rcl_subscription_get_default_options();
-    EXPECT_FALSE(subscription_options.disable_loaned_message);
+    EXPECT_TRUE(subscription_options.disable_loaned_message);
   }
   {
     ASSERT_TRUE(rcutils_set_env("ROS_DISABLE_LOANED_MESSAGES", "Unexpected"));
     rcl_subscription_options_t subscription_options = rcl_subscription_get_default_options();
+    EXPECT_TRUE(subscription_options.disable_loaned_message);
+  }
+  {
+    ASSERT_TRUE(rcutils_set_env("ROS_DISABLE_LOANED_MESSAGES", "0"));
+    rcl_subscription_options_t subscription_options = rcl_subscription_get_default_options();
     EXPECT_FALSE(subscription_options.disable_loaned_message);
   }
 }