Change the code according jinmei's review result in trac352: 1. change class ServeMixIn's class variables to instance variable. 2. change 'localhost' to '127.0.0.1' to avoid undesireable block. 3. add some documentation for class ServeMixIn.

git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac352@3149 e5f2f494-b856-4b98-b285-d166d9295462

src/bin/cmdctl/cmdctl.py.in

@@ -441,7 +441,9 @@ class CommandControl():
 
         return (keyfile, certfile, accountsfile)
 
-class SecureHTTPServer(serve_mixin.ServeMixIn, socketserver.ThreadingMixIn, http.server.HTTPServer):
+class SecureHTTPServer(serve_mixin.ServeMixIn, 
+                       socketserver.ThreadingMixIn, 
+                       http.server.HTTPServer):
     '''Make the server address can be reused.'''
     allow_reuse_address = True
 
@@ -449,6 +451,7 @@ class SecureHTTPServer(serve_mixin.ServeMixIn, socketserver.ThreadingMixIn, http
                  CommandControlClass,
                  idle_timeout = 1200, verbose = False):
         '''idle_timeout: the max idle time for login'''
+        serve_mixin.ServeMixIn.__init__(self)
         try:
             http.server.HTTPServer.__init__(self, server_address, RequestHandlerClass)
         except socket.error as err:

src/bin/xfrout/xfrout.py.in

@@ -297,6 +297,7 @@ class UnixSockServer(serve_mixin.ServeMixIn, ThreadingUnixStreamServer):
     def __init__(self, sock_file, handle_class, shutdown_event, config_data, cc, log):
         self._remove_unused_sock_file(sock_file)
         self._sock_file = sock_file
+        serve_mixin.ServeMixIn.__init__(self)
         ThreadingUnixStreamServer.__init__(self, sock_file, handle_class)
         self._lock = threading.Lock()
         self._transfers_counter = 0

src/lib/python/isc/utils/serve_mixin.py

@@ -17,44 +17,54 @@ import threading
 import socket
 import select
 
+SOCK_DATA = b'somedata'
 class ServeMixIn:
     '''Mix-In class to override the function serve_forever()
-    and shutdown() in class socketserver. 
+    and shutdown() in class socketserver.TCPServer. 
+      ServeMixIn should be used together with socketserver.TCPServer
+    or some derived classes of it, and ServeMixIn must be the first 
+    base class in multiple inheritance, eg. MyClass(ServeMixIn, 
+    socketserver.TCPServer). ServeMixIn.__init__() should be called
+    explicitely in derived class.
     '''
-    _serving = False
-    _is_shut_down = threading.Event()
-    _read_sock, _write_sock = socket.socketpair()
+    def __init__(self):
+        self.__serving = False
+        self.__is_shut_down = threading.Event()
+        self.__read_sock, self.__write_sock = socket.socketpair()
 
     def serve_forever(self, poll_interval=0.5):
-        ''' Override the serve_forever() in class BaseServer.
-        use one socket pair to wake up the select when shutdown()
-        is called in anther thread.
+        ''' Override the serve_forever([poll_interval]) in class 
+        socketserver.TCPServer. use one socket pair to wake up 
+        the select when shutdown() is called in anther thread.
+          Note, parameter 'poll_interval' is just used to keep the
+        interface, it's never used in this function.
         '''        
-        self._serving = True
-        self._is_shut_down.clear()
-        while self._serving:
-            # block until the self.socket or self._read_sock is readable 
+        self.__serving = True
+        self.__is_shut_down.clear()
+        while self.__serving:
+            # block until the self.socket or self.__read_sock is readable 
             try:
-                r, w, e = select.select([self, self._read_sock], [], [])
+                r, w, e = select.select([self, self.__read_sock], [], [])
             except select.error as err:
                 if err.args[0] != EINTR:
                     raise
                 else:
                     continue
             if r:
-                if self._read_sock in r:
+                if (self.__read_sock in r) and \
+                   (self.__read_sock.recv(len(SOCK_DATA)) == SOCK_DATA):
                     break
                 else:
                     self._handle_request_noblock()
 
-        self._is_shut_down.set()
+        self.__is_shut_down.set()
 
     def shutdown(self):
         '''Stops the serve_forever loop.
         Blocks until the loop has finished, the function should be called
         in another thread when serve_forever is running, or it will block.
         '''
-        self._serving = False
-        self._write_sock.send(b'anydata') # make self._read_sock readable.
-        self._is_shut_down.wait()
+        self.__serving = False
+        self.__write_sock.send(SOCK_DATA) # make self.__read_sock readable.
+        self.__is_shut_down.wait()
 

src/lib/python/isc/utils/tests/serve_mixin_test.py

@@ -28,8 +28,10 @@ class MyHandler(socketserver.BaseRequestHandler):
 class MyServer(ServeMixIn, 
                socketserver.ThreadingMixIn,
                socketserver.TCPServer):
-    pass
 
+    def __init__(self, server_addr, handler_class):
+        ServeMixIn.__init__(self)
+        socketserver.TCPServer.__init__(self, server_addr, handler_class)
 
 def send_and_get_reply(ip, port, msg):
     sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
@@ -42,7 +44,7 @@ def send_and_get_reply(ip, port, msg):
 class TestServeMixIn(unittest.TestCase):
     def test_serve_forever(self):
         # use port 0 to select an arbitrary unused port.
-        server = MyServer(('localhost', 0), MyHandler)
+        server = MyServer(('127.0.0.1', 0), MyHandler)
         ip, port = server.server_address
         server_thread = threading.Thread(target=server.serve_forever)
         server_thread.setDaemon(True)