DRTVWR-418: Revamp testrunner to shutdown server Thread at end.

Instead of having testrunner.run()'s caller pass a Thread object on which to
run the caller's server instance's serve_forever() method, just pass the
server instance. testrunner.run() now constructs the Thread. This API change
allows run() to also call shutdown() on the server instance when done, and
then join() the Thread.

The hope is that this will avoid the Python runtime forcing the process
termination code to 1 due to forcibly killing the daemon thread still running
serve_forever().

While at it, eliminate calls to testrunner.freeport() -- just make the runtime
pick a suitable port instead.

4 files changed, 91 insertions, 59 deletions

diff --git a/indra/llcorehttp/tests/test_llcorehttp_peer.py b/indra/llcorehttp/tests/test_llcorehttp_peer.py
index caa204b519..b900ad73ff 100755
--- a/indra/llcorehttp/tests/test_llcorehttp_peer.py
+++ b/indra/llcorehttp/tests/test_llcorehttp_peer.py
@@ -48,7 +48,7 @@ from llbase import llsd
 sys.path.append(os.path.join(os.path.dirname(__file__), os.pardir, os.pardir,
                              "llmessage", "tests"))
 
-from testrunner import freeport, run, debug, VERBOSE
+from testrunner import run, debug, VERBOSE
 
 class TestHTTPRequestHandler(BaseHTTPRequestHandler):
     """This subclass of BaseHTTPRequestHandler is to receive and echo
@@ -297,22 +297,17 @@ if __name__ == "__main__":
         if option == "-V" or option == "--valgrind":
             do_valgrind = True
 
-    # Instantiate a Server(TestHTTPRequestHandler) on the first free port
-    # in the specified port range. Doing this inline is better than in a
-    # daemon thread: if it blows up here, we'll get a traceback. If it blew up
-    # in some other thread, the traceback would get eaten and we'd run the
-    # subject test program anyway.
-    httpd, port = freeport(xrange(8000, 8020),
-                           lambda port: Server(('127.0.0.1', port), TestHTTPRequestHandler))
+    # Instantiate a Server(TestHTTPRequestHandler) on a port chosen by the
+    # runtime.
+    httpd = Server(('127.0.0.1', 0), TestHTTPRequestHandler)
 
     # Pass the selected port number to the subject test program via the
     # environment. We don't want to impose requirements on the test program's
     # command-line parsing -- and anyway, for C++ integration tests, that's
     # performed in TUT code rather than our own.
-    os.environ["LL_TEST_PORT"] = str(port)
+    os.environ["LL_TEST_PORT"] = str(httpd.server_port)
     debug("$LL_TEST_PORT = %s", port)
     if do_valgrind:
         args = ["valgrind", "--log-file=./valgrind.log"] + args
         path_search = True
-    sys.exit(run(server=Thread(name="httpd", target=httpd.serve_forever), use_path=path_search, *args))
-
+    sys.exit(run(server_inst=httpd, use_path=path_search, *args))
diff --git a/indra/llmessage/tests/test_llsdmessage_peer.py b/indra/llmessage/tests/test_llsdmessage_peer.py
index bac18fa374..a0d5d1b354 100755
--- a/indra/llmessage/tests/test_llsdmessage_peer.py
+++ b/indra/llmessage/tests/test_llsdmessage_peer.py
@@ -36,7 +36,7 @@ from BaseHTTPServer import HTTPServer, BaseHTTPRequestHandler
 
 from llbase.fastest_elementtree import parse as xml_parse
 from llbase import llsd
-from testrunner import freeport, run, debug, VERBOSE
+from testrunner import run, debug, VERBOSE
 import time
 
 _storage=None
@@ -155,17 +155,13 @@ class Server(HTTPServer):
     allow_reuse_address = False
 
 if __name__ == "__main__":
-    # Instantiate a Server(TestHTTPRequestHandler) on the first free port
-    # in the specified port range. Doing this inline is better than in a
-    # daemon thread: if it blows up here, we'll get a traceback. If it blew up
-    # in some other thread, the traceback would get eaten and we'd run the
-    # subject test program anyway.
-    httpd, port = freeport(xrange(8000, 8020),
-                           lambda port: Server(('127.0.0.1', port), TestHTTPRequestHandler))
+    # Instantiate a Server(TestHTTPRequestHandler) on a port chosen by the
+    # runtime.
+    httpd = Server(('127.0.0.1', 0), TestHTTPRequestHandler)
     # Pass the selected port number to the subject test program via the
     # environment. We don't want to impose requirements on the test program's
     # command-line parsing -- and anyway, for C++ integration tests, that's
     # performed in TUT code rather than our own.
-    os.environ["PORT"] = str(port)
+    os.environ["PORT"] = str(httpd.server_port)
     debug("$PORT = %s", port)
-    sys.exit(run(server=Thread(name="httpd", target=httpd.serve_forever), *sys.argv[1:]))
+    sys.exit(run(server_inst=httpd, *sys.argv[1:]))
diff --git a/indra/llmessage/tests/testrunner.py b/indra/llmessage/tests/testrunner.py
index 9a2de71142..09f0f3c681 100755
--- a/indra/llmessage/tests/testrunner.py
+++ b/indra/llmessage/tests/testrunner.py
@@ -27,13 +27,12 @@ Linden Research, Inc., 945 Battery Street, San Francisco, CA  94111  USA
 $/LicenseInfo$
 """
 
-from __future__ import with_statement
-
 import os
 import sys
 import re
 import errno
 import socket
+from threading import Thread
 
 VERBOSE = os.environ.get("INTEGRATION_TEST_VERBOSE", "0") # default to quiet
 # Support usage such as INTEGRATION_TEST_VERBOSE=off -- distressing to user if
@@ -47,6 +46,9 @@ if VERBOSE:
 else:
     debug = lambda *args: None
 
+class Error(Exception):
+    pass
+
 def freeport(portlist, expr):
     """
     Find a free server port to use. Specifically, evaluate 'expr' (a
@@ -141,39 +143,73 @@ def freeport(portlist, expr):
         raise
 
 def run(*args, **kwds):
-    """All positional arguments collectively form a command line, executed as
-    a synchronous child process.
-    In addition, pass server=new_thread_instance as an explicit keyword (to
-    differentiate it from an additional command-line argument).
-    new_thread_instance should be an instantiated but not yet started Thread
-    subclass instance, e.g.:
-    run("python", "-c", 'print "Hello, world!"', server=TestHTTPServer(name="httpd"))
     """
-    # If there's no server= keyword arg, don't start a server thread: simply
-    # run a child process.
+    Run a specified command as a synchronous child process, optionally
+    launching a server Thread during the run.
+
+    All positional arguments collectively form a command line. The first
+    positional argument names the program file to execute.
+
+    Returns the termination code of the child process.
+
+    In addition, you may pass keyword-only arguments:
+
+    use_path=True: allow a simple filename as command and search PATH for that
+    filename. Otherwise the command must be a full pathname.
+
+    server_inst: an instance of a subclass of SocketServer.BaseServer.
+
+    When you pass server_inst, its serve_forever() method is called on a
+    separate Thread before the child process is run. It is shutdown() when the
+    child process terminates.
+    """
+    # server= keyword arg is discontinued
     try:
         thread = kwds.pop("server")
     except KeyError:
         pass
     else:
-        # Start server thread. Note that this and all other comm server
-        # threads should be daemon threads: we'll let them run "forever,"
-        # confident that the whole process will terminate when the main thread
-        # terminates, which will be when the child process terminates.
+        raise Error("Obsolete call to testrunner.run(): pass server_inst=, not server=")
+
+    try:
+        server_inst = kwds.pop("server_inst")
+    except KeyError:
+        # We're not starting a thread, so shutdown() is a no-op.
+        shutdown = lambda: None
+    else:
+        # Make a Thread on which to call server_inst.serve_forever().
+        thread = Thread(name="server", target=server_inst.serve_forever)
+
+        # Make this a "daemon" thread.
         thread.setDaemon(True)
         thread.start()
-    # choice of os.spawnv():
-    # - [v vs. l] pass a list of args vs. individual arguments,
-    # - [no p] don't use the PATH because we specifically want to invoke the
-    #   executable passed as our first arg,
-    # - [no e] child should inherit this process's environment.
-    debug("Running %s...", " ".join(args))
-    if kwds.get("use_path", False):
-        rc = os.spawnvp(os.P_WAIT, args[0], args)
-    else:
-        rc = os.spawnv(os.P_WAIT, args[0], args)
-    debug("%s returned %s", args[0], rc)
-    return rc
+
+        # We used to simply call sys.exit() with the daemon thread still
+        # running -- but in recent versions of Python 2, even when you call
+        # sys.exit(0), apparently killing the thread causes the Python runtime
+        # to force the process termination code to 1. So try to play nice.
+        def shutdown():
+            # evidently this call blocks until shutdown is complete
+            server_inst.shutdown()
+            # which should make it straightforward to join()
+            thread.join()
+
+    try:
+        # choice of os.spawnv():
+        # - [v vs. l] pass a list of args vs. individual arguments,
+        # - [no p] don't use the PATH because we specifically want to invoke the
+        #   executable passed as our first arg,
+        # - [no e] child should inherit this process's environment.
+        debug("Running %s...", " ".join(args))
+        if kwds.get("use_path", False):
+            rc = os.spawnvp(os.P_WAIT, args[0], args)
+        else:
+            rc = os.spawnv(os.P_WAIT, args[0], args)
+        debug("%s returned %s", args[0], rc)
+        return rc
+
+    finally:
+        shutdown()
 
 # ****************************************************************************
 #   test code -- manual at this point, see SWAT-564
diff --git a/indra/newview/tests/test_llxmlrpc_peer.py b/indra/newview/tests/test_llxmlrpc_peer.py
index 281b72a058..12394ad1d9 100755
--- a/indra/newview/tests/test_llxmlrpc_peer.py
+++ b/indra/newview/tests/test_llxmlrpc_peer.py
@@ -35,11 +35,20 @@ from threading import Thread
 from SimpleXMLRPCServer import SimpleXMLRPCServer
 
 mydir = os.path.dirname(__file__)       # expected to be .../indra/newview/tests/
-sys.path.insert(0, os.path.join(mydir, os.pardir, os.pardir, "lib", "python"))
-sys.path.insert(1, os.path.join(mydir, os.pardir, os.pardir, "llmessage", "tests"))
-from testrunner import freeport, run, debug
+sys.path.insert(0, os.path.join(mydir, os.pardir, os.pardir, "llmessage", "tests"))
+from testrunner import run, debug
 
 class TestServer(SimpleXMLRPCServer):
+    # This server_bind() override is borrowed and simplified from
+    # BaseHTTPServer.HTTPServer.server_bind(): we want to capture the actual
+    # server port. BaseHTTPServer.HTTPServer.server_bind() stores the actual
+    # port in a server_port attribute, but SimpleXMLRPCServer isn't derived
+    # from HTTPServer. So do it ourselves.
+    def server_bind(self):
+        """Override server_bind to store the server port."""
+        SimpleXMLRPCServer.server_bind(self)
+        self.server_port = self.socket.getsockname()[1]
+
     def _dispatch(self, method, params):
         try:
             func = getattr(self, method)
@@ -67,15 +76,11 @@ class TestServer(SimpleXMLRPCServer):
         pass
 
 if __name__ == "__main__":
-    # Instantiate a TestServer on the first free port in the specified port
-    # range. Doing this inline is better than in a daemon thread: if it blows
-    # up here, we'll get a traceback. If it blew up in some other thread, the
-    # traceback would get eaten and we'd run the subject test program anyway.
-    xmlrpcd, port = freeport(xrange(8000, 8020),
-                             lambda port: TestServer(('127.0.0.1', port)))
+    # Make the runtime choose an available port.
+    xmlrpcd = TestServer(('127.0.0.1', 0))
     # Pass the selected port number to the subject test program via the
     # environment. We don't want to impose requirements on the test program's
     # command-line parsing -- and anyway, for C++ integration tests, that's
     # performed in TUT code rather than our own.
-    os.environ["PORT"] = str(port)
-    sys.exit(run(server=Thread(name="xmlrpc", target=xmlrpcd.serve_forever), *sys.argv[1:]))
+    os.environ["PORT"] = str(xmlrpcd.server_port)
+    sys.exit(run(server_inst=xmlrpcd, *sys.argv[1:]))