You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by dr...@apache.org on 2008/06/11 00:55:26 UTC

svn commit: r666363 - in /incubator/thrift/trunk: lib/py/src/server/TServer.py test/py/RunClientServer.py test/py/TestClient.py test/py/TestServer.py

Author: dreiss
Date: Tue Jun 10 15:55:26 2008
New Revision: 666363

URL: http://svn.apache.org/viewvc?rev=666363&view=rev
Log:
Python forking server should close connection in parent.

When a function called by the forking python thrift server throws an
exception the client will hang.  This happens because the parent of the
forked process does not properly close the socket fd.

Under normal operation the server operation completes and returns a value to
the client.  However, when an exception occurs the 'end' message is never
sent to the client so the client relies on a connection close to abort the
call (this is how the threading server works I believe).

This fails with the forking server because the parent process never closes
the socket fd.  The child has closed the fd at this point, but the
connection will not actually be closed until all open instances of the fd
are closed. Since the client is waiting for a message and the server never
closes it the client is forced to wait until a read timeout occurs many
minutes later.

This diff closes the parent's copy of the socket fd immediately after the
fork occurs.

I modified my load test server to throw an exception.  The client did not hang.

Modified:
    incubator/thrift/trunk/lib/py/src/server/TServer.py
    incubator/thrift/trunk/test/py/RunClientServer.py
    incubator/thrift/trunk/test/py/TestClient.py
    incubator/thrift/trunk/test/py/TestServer.py

Modified: incubator/thrift/trunk/lib/py/src/server/TServer.py
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/lib/py/src/server/TServer.py?rev=666363&r1=666362&r2=666363&view=diff
==============================================================================
--- incubator/thrift/trunk/lib/py/src/server/TServer.py (original)
+++ incubator/thrift/trunk/lib/py/src/server/TServer.py Tue Jun 10 15:55:26 2008
@@ -170,7 +170,6 @@
         print '%s, %s, %s' % (type(x), x, traceback.format_exc())
 
 
-
 class TForkingServer(TServer):
 
   """A Thrift server that forks a new process for each request"""
@@ -191,6 +190,13 @@
     self.children = []
 
   def serve(self):
+    def try_close(file):
+      try:
+        file.close()
+      except IOError, e:
+        print '%s, %s, %s' % (type(e), e, traceback.format_exc())
+
+
     self.serverTransport.listen()
     while True:
       client = self.serverTransport.accept()
@@ -202,6 +208,12 @@
           self.children.append(pid)
           self.collect_children()
 
+          # Parent must close socket or the connection may not get
+          # closed promptly
+          itrans = self.inputTransportFactory.getTransport(client)
+          otrans = self.outputTransportFactory.getTransport(client)
+          try_close(itrans)
+          try_close(otrans)
         else:
           itrans = self.inputTransportFactory.getTransport(client)
           otrans = self.outputTransportFactory.getTransport(client)
@@ -209,24 +221,20 @@
           iprot = self.inputProtocolFactory.getProtocol(itrans)
           oprot = self.outputProtocolFactory.getProtocol(otrans)
 
+          ecode = 0
           try:
             while True:
               self.processor.process(iprot, oprot)
           except TTransport.TTransportException, tx:
             pass
           except Exception, e:
-            print '%s, %s, %s' % (type(x), x, traceback.format_exc())
-            os._exit(1)
-
-          def try_close(file):
-            try:
-              file.close()
-            except IOError, e:
-              print '%s, %s, %s' % (type(x), x, traceback.format_exc())
+            print '%s, %s, %s' % (type(e), e, traceback.format_exc())
+            ecode = 1
+          finally:
+            try_close(itrans)
+            try_close(otrans)
 
-          try_close(itrans)
-          try_close(otrans)
-          os._exit(0)
+          os._exit(ecode)
 
       except TTransport.TTransportException, tx:
         pass
@@ -234,7 +242,6 @@
         print '%s, %s, %s' % (type(x), x, traceback.format_exc())
 
 
-
   def collect_children(self):
     while self.children:
       try:

Modified: incubator/thrift/trunk/test/py/RunClientServer.py
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/test/py/RunClientServer.py?rev=666363&r1=666362&r2=666363&view=diff
==============================================================================
--- incubator/thrift/trunk/test/py/RunClientServer.py (original)
+++ incubator/thrift/trunk/test/py/RunClientServer.py Tue Jun 10 15:55:26 2008
@@ -1,5 +1,6 @@
 #!/usr/bin/env python
 
+import time
 import subprocess
 import sys
 import os
@@ -8,12 +9,20 @@
 def relfile(fname):
     return os.path.join(os.path.dirname(__file__), fname)
 
-serverproc = subprocess.Popen([sys.executable, relfile("TestServer.py")])
-try:
+def runTest(server_class):
+    print "Testing ", server_class
+    serverproc = subprocess.Popen([sys.executable, relfile("TestServer.py"), server_class])
+    try:
 
-    ret = subprocess.call([sys.executable, relfile("TestClient.py")])
-    if ret != 0:
-        raise Exception("subprocess failed")
-finally:
-    # fixme: should check that server didn't die
-    os.kill(serverproc.pid, signal.SIGKILL)
+        ret = subprocess.call([sys.executable, relfile("TestClient.py")])
+        if ret != 0:
+            raise Exception("subprocess failed")
+    finally:
+        # fixme: should check that server didn't die
+        os.kill(serverproc.pid, signal.SIGKILL)
+
+    # wait for shutdown
+    time.sleep(5)
+
+map(runTest, ["TForkingServer", "TThreadPoolServer",
+              "TThreadedServer", "TSimpleServer"])

Modified: incubator/thrift/trunk/test/py/TestClient.py
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/test/py/TestClient.py?rev=666363&r1=666362&r2=666363&view=diff
==============================================================================
--- incubator/thrift/trunk/test/py/TestClient.py (original)
+++ incubator/thrift/trunk/test/py/TestClient.py Tue Jun 10 15:55:26 2008
@@ -86,6 +86,12 @@
       self.assertEqual(x.errorCode, 1001)
       self.assertEqual(x.message, 'Xception')
 
+    try:
+      self.client.testException("throw_undeclared")
+      self.fail("should have thrown exception")
+    except Exception: # type is undefined
+      pass
+
   def testAsync(self):
     start = time.time()
     self.client.testAsync(2)

Modified: incubator/thrift/trunk/test/py/TestServer.py
URL: http://svn.apache.org/viewvc/incubator/thrift/trunk/test/py/TestServer.py?rev=666363&r1=666362&r2=666363&view=diff
==============================================================================
--- incubator/thrift/trunk/test/py/TestServer.py (original)
+++ incubator/thrift/trunk/test/py/TestServer.py Tue Jun 10 15:55:26 2008
@@ -51,6 +51,8 @@
       x.errorCode = 1001
       x.message = str
       raise x
+    elif str == "throw_undeclared":
+      raise ValueError("foo")
 
   def testAsync(self, seconds):
     print 'testAsync(%d) => sleeping...' % seconds
@@ -62,5 +64,8 @@
 transport = TSocket.TServerSocket(9090)
 tfactory = TTransport.TBufferedTransportFactory()
 pfactory = TBinaryProtocol.TBinaryProtocolFactory()
-server = TServer.TThreadedServer(processor, transport, tfactory, pfactory)
+
+ServerClass = getattr(TServer, sys.argv[1])
+
+server = ServerClass(processor, transport, tfactory, pfactory)
 server.serve()