Properly implement action server/client handle cleanup. (#905)

* Properly implement action server/client handle cleanup.

In particular, it should never really be the case that the
underlying handle that we are using is None.  That only seemed
to be happening because we were double destroying; once in
the explicit call to destroy(), and once in __del__.  But
we don't actually need __del__; during garbage collection, we'll
drop the reference to the handle and then the underlying object
will get freed anyway.  Just remove all of that extraneous
infrastructure here.

Signed-off-by: Chris Lalancette <[email protected]>

Author: clalancette

rclpy/rclpy/action/client.py

@@ -353,15 +353,9 @@ def add_to_wait_set(self, wait_set):
         self._client_handle.add_to_waitset(wait_set)
 
     def __enter__(self):
-        if self._client_handle is None:
-            return None
-
         return self._client_handle.__enter__()
 
     def __exit__(self, t, v, tb):
-        if self._client_handle is None:
-            return
-
         self._client_handle.__exit__(t, v, tb)
 
     # End Waitable API
@@ -589,13 +583,5 @@ def wait_for_server(self, timeout_sec=None):
 
     def destroy(self):
         """Destroy the underlying action client handle."""
-        if self._client_handle is None:
-            return
-        with self._node.handle:
-            self._client_handle.destroy_when_not_in_use()
-            self._node.remove_waitable(self)
-        self._client_handle = None
-
-    def __del__(self):
-        """Destroy the underlying action client handle."""
-        self.destroy()
+        self._client_handle.destroy_when_not_in_use()
+        self._node.remove_waitable(self)

rclpy/rclpy/action/server.py

@@ -495,15 +495,9 @@ def add_to_wait_set(self, wait_set):
             self._handle.add_to_waitset(wait_set)
 
     def __enter__(self):
-        if self._handle is None:
-            return None
-
         return self._handle.__enter__()
 
     def __exit__(self, t, v, tb):
-        if self._handle is None:
-            return
-
         self._handle.__exit__(t, v, tb)
 
     # End Waitable API
@@ -602,16 +596,8 @@ def register_execute_callback(self, execute_callback):
 
     def destroy(self):
         """Destroy the underlying action server handle."""
-        if self._handle is None:
-            return
-
         for goal_handle in self._goal_handles.values():
             goal_handle.destroy()
 
         self._handle.destroy_when_not_in_use()
         self._node.remove_waitable(self)
-        self._handle = None
-
-    def __del__(self):
-        """Destroy the underlying action server handle."""
-        self.destroy()