Merge lp:~alecu/ubuntuone-client/stop-proxy-tunnel into lp:ubuntuone-client

[Alejandro J. Cura]

- Stop the proxy tunnel when stopping the reactor (LP: #963485).

- Stop the proxy tunnel when stopping the reactor (LP: #963485).

[Natalia Bidart (nataliabidart)]

Looks good.

review: Approve

[Diego Sarmentero (diegosarmentero)]

+1

review: Approve

=== modified file 'tests/syncdaemon/test_tunnel_runner.py'
--- tests/syncdaemon/test_tunnel_runner.py	2012-03-19 03:31:10 +0000
+++ tests/syncdaemon/test_tunnel_runner.py	2012-03-23 22:49:18 +0000
@@ -48,6 +48,20 @@
         self.assertEqual(client, reactor)
 
 
+class FakeProcessTransport(object):
+    """A fake ProcessTransport."""
+
+    pid = 0
+
+    def __init__(self):
+        """Initialize this fake."""
+        self._signals_sent = []
+
+    def signalProcess(self, signalID):
+        """Send a signal to the process."""
+        self._signals_sent.append(signalID)
+
+
 class TunnelRunnerTestCase(TestCase):
     """Tests for the TunnelRunner."""
 
@@ -58,8 +72,22 @@
         """Initialize this testcase."""
         yield super(TunnelRunnerTestCase, self).setUp()
         self.spawned = []
-        self.patch(tunnel_client.reactor, "spawnProcess",
-                   lambda *args, **kwargs: self.spawned.append((args, kwargs)))
+        self.triggers = []
+        self.fake_process_transport = FakeProcessTransport()
+
+        def fake_spawn_process(*args, **kwargs):
+            """A fake spawnProcess."""
+            self.spawned.append((args, kwargs))
+            return self.fake_process_transport
+
+        self.patch(tunnel_client.reactor, "spawnProcess", fake_spawn_process)
+
+        def fake_add_system_event_trigger(*args, **kwargs):
+            """A fake addSystemEventTrigger."""
+            self.triggers.append((args, kwargs))
+
+        self.patch(tunnel_client.reactor, "addSystemEventTrigger",
+                   fake_add_system_event_trigger)
         self.process_protocol = None
         self.process_protocol_class = tunnel_client.TunnelProcessProtocol
         self.patch(tunnel_client, "TunnelProcessProtocol",
@@ -76,6 +104,23 @@
         self.assertEqual(len(self.spawned), 1,
                         "The tunnel process is started.")
 
+    def test_system_event_finished(self):
+        """An event is added to stop the process with the reactor."""
+        expected = [(("before", "shutdown", self.tr.stop), {})]
+        self.assertEqual(self.triggers, expected)
+
+    def test_stop_process(self):
+        """The process is stopped if still running."""
+        self.tr.process_transport.pid = 1234
+        self.tr.stop()
+        self.assertEqual(self.fake_process_transport._signals_sent, ["KILL"])
+
+    def test_not_stopped_if_already_finished(self):
+        """Do not stop the tunnel process if it's already finished."""
+        self.tr.process_transport.pid = None
+        self.tr.stop()
+        self.assertEqual(self.fake_process_transport._signals_sent, [])
+
     @defer.inlineCallbacks
     def test_tunnel_process_get_client_yielded_twice(self):
         """The get_client method can be yielded twice."""
 
=== modified file 'ubuntuone/syncdaemon/tunnel_runner.py'
--- ubuntuone/syncdaemon/tunnel_runner.py	2012-03-19 13:20:36 +0000
+++ ubuntuone/syncdaemon/tunnel_runner.py	2012-03-23 22:49:18 +0000
@@ -33,6 +33,7 @@
     def __init__(self, host, port):
         """Start this runner instance."""
         self.client_d = defer.Deferred()
+        self.process_transport = None
         try:
             self.start_process(host, port)
         except ImportError:
@@ -45,7 +46,15 @@
         protocol = TunnelProcessProtocol(self.client_d)
         process_path = self.get_process_path()
         args = [TUNNEL_EXECUTABLE, host, str(port)]
-        reactor.spawnProcess(protocol, process_path, env=None, args=args)
+        self.process_transport = reactor.spawnProcess(protocol, process_path,
+                                                      env=None, args=args)
+        reactor.addSystemEventTrigger("before", "shutdown", self.stop)
+
+    def stop(self):
+        """Stop the tunnel process if still running."""
+        logger.info("Stopping process %r", self.process_transport.pid)
+        if self.process_transport.pid is not None:
+            self.process_transport.signalProcess("KILL")
 
     def get_process_path(self):
         """Get the path to the tunnel process."""