Refactor CNS cleanup method.

BUG=None
TEST=run cns tests.


Review URL: https://chromiumcodereview.appspot.com/10356033

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@136721 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 16 insertions, 27 deletions

diff --git a/media/tools/constrained_network_server/cns.py b/media/tools/constrained_network_server/cns.py
index 9b701e4..c87f20a 100755
--- a/media/tools/constrained_network_server/cns.py
+++ b/media/tools/constrained_network_server/cns.py
@@ -65,7 +65,7 @@ class PortAllocator(object):
 
     # Locks port creation and cleanup. TODO(dalecurtis): If performance becomes
     # an issue a per-port based lock system can be used instead.
-    self._port_lock = threading.Lock()
+    self._port_lock = threading.RLock()
 
   def Get(self, key, new_port=False, **kwargs):
     """Sets up a constrained port using the requested parameters.
@@ -95,7 +95,7 @@ class PortAllocator(object):
       # Cleanup ports on new port requests. Do it after the cache check though
       # so we don't erase and then setup the same port.
       if self._expiry_time_secs > 0:
-        self._CleanupLocked(all_ports=False)
+        self.Cleanup(all_ports=False)
 
       # Performance isn't really an issue here, so just iterate over the port
       # range to find an unused port. If no port is found, None is returned.
@@ -128,22 +128,24 @@ class PortAllocator(object):
       cherrypy.log('Error: %s\nOutput: %s' % (e.msg, e.error))
       return False
 
-  def _CleanupLocked(self, all_ports):
-    """Internal cleanup method, expects lock to have already been acquired.
+  def Cleanup(self, all_ports):
+    """Cleans up expired ports, or if all_ports=True, all allocated ports.
 
-    See Cleanup() for more information.
+    By default, ports which haven't been used for self._expiry_time_secs are
+    torn down. If all_ports=True then they are torn down regardless.
 
     Args:
       all_ports: Should all ports be torn down regardless of expiration?
     """
-    now = time.time()
-    # Use .items() instead of .iteritems() so we can delete keys w/o error.
-    for port, status in self._ports.items():
-      expired = now - status['last_update'] > self._expiry_time_secs
-      if all_ports or expired:
-        cherrypy.log('Cleaning up port %d' % port)
-        self._DeletePort(port)
-        del self._ports[port]
+    with self._port_lock:
+      now = time.time()
+      # Use .items() instead of .iteritems() so we can delete keys w/o error.
+      for port, status in self._ports.items():
+        expired = now - status['last_update'] > self._expiry_time_secs
+        if all_ports or expired:
+          cherrypy.log('Cleaning up port %d' % port)
+          self._DeletePort(port)
+          del self._ports[port]
 
   def _DeletePort(self, port):
     """Deletes network constraints on port.
@@ -156,19 +158,6 @@ class PortAllocator(object):
     except traffic_control.TrafficControlError as e:
       cherrypy.log('Error: %s\nOutput: %s' % (e.msg, e.error))
 
-  def Cleanup(self, interface, all_ports=False):
-    """Cleans up expired ports, or if all_ports=True, all allocated ports.
-
-    By default, ports which haven't been used for self._expiry_time_secs are
-    torn down. If all_ports=True then they are torn down regardless.
-
-    Args:
-      interface: Interface the constrained network is setup on.
-      all_ports: Should all ports be torn down regardless of expiration?
-    """
-    with self._port_lock:
-      self._CleanupLocked(all_ports)
-
 
 class ConstrainedNetworkServer(object):
   """A CherryPy-based HTTP server for serving files with network constraints."""
@@ -378,7 +367,7 @@ def Main():
   finally:
     # Disable Ctrl-C handler to prevent interruption of cleanup.
     signal.signal(signal.SIGINT, lambda signal, frame: None)
-    pa.Cleanup(options.interface, all_ports=True)
+    pa.Cleanup(all_ports=True)
 
 
 if __name__ == '__main__':