Ensure host_forwarder is killed when setting up/tearing down sharding.

build/android/pylib/base_test_sharder.py

Index: build/android/pylib/base_test_sharder.py
diff --git a/build/android/pylib/base_test_sharder.py b/build/android/pylib/base_test_sharder.py
index fa05abec4e202b77d422fd2a29bcd5f20dfe996d..45dbc4ac7cd6caf90343344ce2631ef417d4051b 100644
--- a/build/android/pylib/base_test_sharder.py
+++ b/build/android/pylib/base_test_sharder.py
@@ -4,6 +4,7 @@
 
 
 import android_commands
+import atexit
 import logging
 import multiprocessing
 
@@ -66,10 +67,11 @@ class BaseTestSharder(object):
 
   def SetupSharding(self, tests):
     """Called before starting the shards."""
-    Forwarder.KillHost(self.build_type)
 
   def OnTestsCompleted(self, test_runners, test_results):
     """Notifies that we completed the tests."""

[bulach, 2012/11/29 15:50:30]

add a "pass" here..

[Philippe, 2012/11/29 17:53:32]

Oops :)

+
+  def _KillHostForwarder(self):
     Forwarder.KillHost(self.build_type)
 
   def RunShardedTests(self):
@@ -85,6 +87,8 @@ class BaseTestSharder(object):
     logging.warning('Look for the "Final result" banner in the end.')
     logging.warning('*' * 80)
     final_results = TestResults()
+    atexit.register(self._KillHostForwarder)

[bulach, 2012/11/29 15:50:30]

does this work? self._KillHostForwarder is a method, which requires "self" as a
first param... I've never used atexit before, but maybe this needs to pass
"self" as param..

registering atexits though, is pretty hairy... I'd much rather ensure that the
entry-point cleans up everything that may have been left behind, than trying to
cleanup via atexit... wdyt?

[Philippe, 2012/11/29 17:53:32]

This seems to work, it is one of the ways to pass a method and the instance it
is called on in Python (which I discovered recently to be honest).

Yeah I totally forgot that pylib was a library used outside the regular testing
infrastructure (e.g. telemetry).
We do kill the host_forwarder during setup so I guess I could just get rid of
the tear down step assuming that all the clients using the forwarder do kill
host_forwarder during setup as well (I will double check that).
But I would still be a bit more confident if the tear down happens. I added a
try/finally that should cover us for most of the cases (the others being when
python quits for a reason other than an exception).

+    self._KillHostForwarder()
     for retry in xrange(self.retries):
       logging.warning('Try %d of %d', retry + 1, self.retries)
       self.SetupSharding(self.tests)