Implementing feedback from @hoffmann-stefan

ros2-dotnet · Oct 12, 2023 · 7d3c0a3 · 7d3c0a3
1 parent 232a4e0
commit 7d3c0a3
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 33 deletions.
diff --git a/rcldotnet/Node.cs b/rcldotnet/Node.cs
@@ -96,13 +96,11 @@ internal delegate RCLRet NativeRCLActionDestroyServerHandleType(
 
         internal static NativeRCLActionDestroyServerHandleType native_rcl_action_destroy_server_handle = null;
 
-        [UnmanagedFunctionPointer(CallingConvention.Cdecl)]
-        internal delegate string NativeRCLGetStringType(
-            SafeNodeHandle nodeHandle);
+        internal static RCLdotnetDelegates.NativeRCLGetStringType native_rcl_node_get_name_handle = null;
 
-        internal static NativeRCLGetStringType native_rcl_get_name_handle = null;
+        internal static RCLdotnetDelegates.NativeRCLGetStringType native_rcl_node_get_namespace_handle = null;
 
-        internal static NativeRCLGetStringType native_rcl_get_namespace_handle = null;
+        internal static RCLdotnetDelegates.NativeRCLGetStringType native_rcl_node_get_fully_qualified_name_handle = null;
 
         static NodeDelegates()
         {
@@ -191,17 +189,23 @@ static NodeDelegates()
                 (NativeRCLActionDestroyServerHandleType)Marshal.GetDelegateForFunctionPointer(
                     native_rcl_action_destroy_server_handle_ptr, typeof(NativeRCLActionDestroyServerHandleType));
 
-            IntPtr native_rcl_get_name_handle_ptr = _dllLoadUtils.GetProcAddress(
-                nativeLibrary, "native_rcl_get_name_handle");
-            NodeDelegates.native_rcl_get_name_handle =
-                (NativeRCLGetStringType)Marshal.GetDelegateForFunctionPointer(
-                    native_rcl_get_name_handle_ptr, typeof(NativeRCLGetStringType));
-
-            IntPtr native_rcl_get_namespace_handle_ptr = _dllLoadUtils.GetProcAddress(
-                nativeLibrary, "native_rcl_get_namespace_handle");
-            NodeDelegates.native_rcl_get_namespace_handle =
-                (NativeRCLGetStringType)Marshal.GetDelegateForFunctionPointer(
-                    native_rcl_get_namespace_handle_ptr, typeof(NativeRCLGetStringType));
+            IntPtr native_rcl_node_get_name_handle_ptr = _dllLoadUtils.GetProcAddress(
+                nativeLibrary, "native_rcl_node_get_name_handle");
+            NodeDelegates.native_rcl_node_get_name_handle =
+                (RCLdotnetDelegates.NativeRCLGetStringType)Marshal.GetDelegateForFunctionPointer(
+                    native_rcl_node_get_name_handle_ptr, typeof(RCLdotnetDelegates.NativeRCLGetStringType));
+
+            IntPtr native_rcl_node_get_namespace_handle_ptr = _dllLoadUtils.GetProcAddress(
+                nativeLibrary, "native_rcl_node_get_namespace_handle");
+            NodeDelegates.native_rcl_node_get_namespace_handle =
+                (RCLdotnetDelegates.NativeRCLGetStringType)Marshal.GetDelegateForFunctionPointer(
+                    native_rcl_node_get_namespace_handle_ptr, typeof(RCLdotnetDelegates.NativeRCLGetStringType));
+
+            IntPtr native_rcl_node_get_fully_qualified_name_handle_ptr = _dllLoadUtils.GetProcAddress(
+                nativeLibrary, "native_rcl_node_get_fully_qualified_name_handle");
+            NodeDelegates.native_rcl_node_get_fully_qualified_name_handle =
+                (RCLdotnetDelegates.NativeRCLGetStringType)Marshal.GetDelegateForFunctionPointer(
+                    native_rcl_node_get_fully_qualified_name_handle_ptr, typeof(RCLdotnetDelegates.NativeRCLGetStringType));
         }
     }
 
@@ -230,6 +234,12 @@ internal Node(SafeNodeHandle handle)
             _actionServers = new List<ActionServer>();
         }
 
+        public string Name => RCLdotnet.GetStringFromNativeDelegate(NodeDelegates.native_rcl_node_get_name_handle, Handle);
+
+        public string Namespace => RCLdotnet.GetStringFromNativeDelegate(NodeDelegates.native_rcl_node_get_namespace_handle, Handle);
+
+        public string FullyQualifiedName => RCLdotnet.GetStringFromNativeDelegate(NodeDelegates.native_rcl_node_get_fully_qualified_name_handle, Handle);
+
         public IList<Subscription> Subscriptions => _subscriptions;
 
         // TODO: (sh) wrap in readonly collection
@@ -460,9 +470,5 @@ private static CancelResponse DefaultCancelCallback(ActionServerGoalHandle goalH
         {
             return CancelResponse.Reject;
         }
-
-        public string GetName() => NodeDelegates.native_rcl_get_name_handle(Handle);
-
-        public string GetNamespace() => NodeDelegates.native_rcl_get_namespace_handle(Handle);
     }
 }
diff --git a/rcldotnet/RCLdotnet.cs b/rcldotnet/RCLdotnet.cs
@@ -352,6 +352,9 @@ internal delegate RCLRet NativeRCLWriteToQosProfileHandleType(
 
         internal static NativeRCLWriteToQosProfileHandleType native_rcl_write_to_qos_profile_handle = null;
 
+        [UnmanagedFunctionPointer(CallingConvention.Cdecl)]
+        internal delegate IntPtr NativeRCLGetStringType(SafeHandle handle);
+
         static RCLdotnetDelegates()
         {
             _dllLoadUtils = DllLoadUtilsFactory.GetDllLoadUtils();
@@ -1459,5 +1462,24 @@ internal static void WriteToMessageHandle(IRosMessage message, SafeHandle messag
                 }
             }
         }
+
+        internal static string GetStringFromNativeDelegate<T>(RCLdotnetDelegates.NativeRCLGetStringType nativeDelegate, T safeHandle) where T : SafeHandle
+        {
+            bool mustRelease = false;
+            try
+            {
+                // This avoids accessing a invalid/freed pointer if some other thread disposes the SafeNodeHandle.
+                safeHandle.DangerousAddRef(ref mustRelease);
+                IntPtr namePtr = nativeDelegate(safeHandle);
+                return Marshal.PtrToStringAnsi(namePtr);
+            }
+            finally
+            {
+                if (mustRelease)
+                {
+                    safeHandle.DangerousRelease();
+                }
+            }
+        }
     }
 }
diff --git a/rcldotnet/rcldotnet_node.c b/rcldotnet/rcldotnet_node.c
@@ -241,24 +241,20 @@ int32_t native_rcl_action_destroy_server_handle(void *action_server_handle, void
   return ret;
 }
 
-// Avoid problems caused by automatic free of the original string.
-const char * get_str_cpy(const char * src) {
-  size_t size = strlen(src) + 1;
-
-  char *copy = (char *)malloc(size);
-  memcpy(copy, src, size);
+const char * native_rcl_node_get_name_handle(void *node_handle) {
+  rcl_node_t *node = (rcl_node_t *)node_handle;
 
-  return copy;
+  return rcl_node_get_name(node);
 }
 
-const char * native_rcl_get_name_handle(void *node_handle) {
+const char * native_rcl_node_get_namespace_handle(void *node_handle) {
   rcl_node_t *node = (rcl_node_t *)node_handle;
 
-  return get_str_cpy(rcl_node_get_name(node));
+  return rcl_node_get_namespace(node);
 }
 
-const char * native_rcl_get_namespace_handle(void *node_handle) {
+const char * native_rcl_node_get_fully_qualified_name_handle(void *node_handle) {
   rcl_node_t *node = (rcl_node_t *)node_handle;
 
-  return get_str_cpy(rcl_node_get_namespace(node));
+  return rcl_node_get_fully_qualified_name(node);
 }
diff --git a/rcldotnet/rcldotnet_node.h b/rcldotnet/rcldotnet_node.h
@@ -72,9 +72,12 @@ RCLDOTNET_EXPORT
 int32_t RCLDOTNET_CDECL native_rcl_action_destroy_server_handle(void *action_server_handle, void *node_handle);
 
 RCLDOTNET_EXPORT
-const char * RCLDOTNET_CDECL native_rcl_get_name_handle(void *node_handle);
+const char * RCLDOTNET_CDECL native_rcl_node_get_name_handle(void *node_handle);
 
 RCLDOTNET_EXPORT
-const char * RCLDOTNET_CDECL native_rcl_get_namespace_handle(void *node_handle);
+const char * RCLDOTNET_CDECL native_rcl_node_get_namespace_handle(void *node_handle);
+
+RCLDOTNET_EXPORT
+const char * RCLDOTNET_CDECL native_rcl_node_get_fully_qualified_name_handle(void *node_handle);
 
 #endif // RCLDOTNET_NODE_H