Repository: endpointcorp/lg_ros_nodes
Issue: ManagedApplication does not garbage collect

**Overview:**
When a `ManagedApplication` is created and then dereferenced from that context, it is not garbage collected.

**Development notes:**
I'll be pushing a test for this shortly.

Even with a cleansing pass and a thorough `close()` method, something is hanging on to this object.  I suspect it is the `ProcController` or something in the appctl chain.  We'll want to add tests for these classes too.

The gist of testing for garbage collection:

```
from lg_common import ManagedApplication
import gc
import weakref

app = ManagedApplication(cmd=['/bin/cat'])
app_ref = weakref.ref(app)
app.close()
app = None
gc.collect()
assert app_ref() is None
```


Changed files:
lg_common/test/offline/test_managed_application.py, lg_common/src/lg_common/managed_application.py, lg_common/test/online/test_adhoc_browser_director_bridge.py

diff --git a/lg_common/src/lg_common/managed_application.py b/lg_common/src/lg_common/managed_application.py
index c6b17179..eba7aaa1 100644
--- a/lg_common/src/lg_common/managed_application.py
+++ b/lg_common/src/lg_common/managed_application.py
@@ -24,7 +24,6 @@ class ManagedApplication(object):
                                    spawn_hooks=[self._handle_respawn],
                                    shell=shell,
                                    respawn=respawn)
-        self.sig_retry_timer = None
 
         self._respawn_handlers = []
         self._state_handlers = []
@@ -36,10 +35,22 @@ class ManagedApplication(object):
         else:
             self.state = ApplicationState.STOPPED
 
-        rospy.on_shutdown(self._cleanup)
-
         self.post_init()
 
+    def close(self):
+        """
+        End this ManagedApplication immediately.  After closing, the instance
+        can no longer be used.
+        """
+        self.set_state(ApplicationState.STOPPED)
+        del self.lock
+        del self.cmd
+        del self.state
+        del self.proc
+        del self.window
+        del self._respawn_handlers[:]
+        del self._state_handlers[:]
+
     def post_init(self):
         pass
 
@@ -57,40 +68,6 @@ class ManagedApplication(object):
         representation = "<ManagedApplication: state: %s, window: %s, cmd: %s, proc: %s" % (self.state, self.window, self.cmd, self.proc)
         return representation
 
-    def _cleanup(self):
-        # explicit SIGCONT needed to prevent undeath
-        self._signal_proc(signal.SIGCONT, retry=False)
-
-    # TODO(mv): better pid retrieval and/or signalling as a feature of
-    #           ProcController.
-    def _signal_proc(self, sig, retry=True):
-        # currently disabled (and for the foreseeable future)
-        return
-
-        if self.sig_retry_timer is not None:
-            self.sig_retry_timer.shutdown()
-
-        if self.proc.watcher is not None and \\
-           self.proc.watcher.proc is not None:
-
-            pid = self.proc.watcher.proc.pid
-            try:
-                os.kill(pid, sig)
-                rospy.logdebug("Sent signal {} to pid {}".format(sig, pid))
-            except OSError:
-                rospy.logerr(
-                    "OSError sending signal {} to pid {}".format(sig, pid))
-        else:
-            rospy.logwarn("Can't signal, no proc")
-            if not retry:
-                return
-
-            def retry_signal(ev):
-                self._signal_proc(sig)
-            self.sig_retry_timer = rospy.Timer(
-                rospy.Duration(SIG_RETRY_DELAY), retry_signal, oneshot=True
-            )
-
     def get_state(self):
         with self.lock:
             return self.state
@@ -107,7 +84,6 @@ class ManagedApplication(object):
 
             if state == ApplicationState.STOPPED:
                 rospy.logdebug("STOPPED")
-                self._signal_proc(signal.SIGCONT, retry=False)
                 self.proc.stop()
                 if self.window is not None:
                     self.window.set_visibility(False)
@@ -115,7 +91,6 @@ class ManagedApplication(object):
             elif state == ApplicationState.SUSPENDED:
                 rospy.logdebug("SUSPENDED")
                 self.proc.start()
-                self._signal_proc(signal.SIGSTOP)
                 if self.window is not None:
                     self.window.set_visibility(False)
                     self.window.converge()
@@ -123,7 +98,6 @@ class ManagedApplication(object):
             elif state == ApplicationState.HIDDEN:
                 rospy.logdebug("HIDDEN")
                 self.proc.start()
-                self._signal_proc(signal.SIGCONT)
                 if self.window is not None:
                     self.window.set_visibility(False)
                     self.window.converge()
@@ -136,7 +110,6 @@ class ManagedApplication(object):
             elif state == ApplicationState.VISIBLE:
                 rospy.logdebug("VISIBLE")
                 self.proc.start()
-                self._signal_proc(signal.SIGCONT)
                 if self.window is not None:
                     self.window.set_visibility(True)
                     self.window.converge()
diff --git a/lg_common/test/offline/test_managed_application.py b/lg_common/test/offline/test_managed_application.py
index c853f7b0..2a28e185 100755
--- a/lg_common/test/offline/test_managed_application.py
+++ b/lg_common/test/offline/test_managed_application.py
@@ -2,8 +2,10 @@
 PKG = 'lg_common'
 NAME = 'test_managed_application'
 
+import gc
 import os
 import unittest
+import weakref
 
 from lg_common.msg import ApplicationState
 from appctl_support import ProcController
@@ -89,8 +91,19 @@ class TestManagedApplication(unittest.TestCase):
         self.assertEqual(1, self.app.proc.stop_count)
 
 
+class TestCleanup(unittest.TestCase):
+    def test_cleanup(self):
+        app = ManagedApplication(cmd=TEST_CMD)
+        app_ref = weakref.ref(app)
+        app.close()
+        app = None
+        gc.collect()
+        self.assertIsNone(app_ref())
+
+
 if __name__ == '__main__':
     import rostest
     rostest.rosrun(PKG, NAME, TestManagedApplication)
+    rostest.rosrun(PKG, NAME, TestCleanup)
 
 # vim: tabstop=8 expandtab shiftwidth=4 softtabstop=4
diff --git a/lg_common/test/online/test_adhoc_browser_director_bridge.py b/lg_common/test/online/test_adhoc_browser_director_bridge.py
index 8287a84e..fa6d2cd8 100755
--- a/lg_common/test/online/test_adhoc_browser_director_bridge.py
+++ b/lg_common/test/online/test_adhoc_browser_director_bridge.py
@@ -308,6 +308,7 @@ class TestAdhocBrowserDirectorBridge(unittest.TestCase):
         self.assertEqual(AdhocBrowsers, type(self.mock_publisher_center.published_messages[0]))
         self.assertEqual(AdhocBrowsers, type(self.mock_publisher_right.published_messages[0]))
 
+
 def zero_id(d):
     for browser in d.browsers:
         browser.id = '0'