[tor-commits] [stem/master] Revisions for cache thread safety

atagar at torproject.org atagar at torproject.org
Sun May 12 02:51:20 UTC 2013


commit e96440eda20e1bbd89ed542654b4f080b8ecb064
Author: Damian Johnson <atagar at torproject.org>
Date:   Sat May 11 18:36:44 2013 -0700

    Revisions for cache thread safety
    
    Handful of changes for the prior couple commits. Most are stylistic, but
    there's a couple funcitonal ones:
    
    * _set_cache() could accidently remove 'exit_policy'
    * _confchanged_listener didn't use the new thread safe operations
---
 stem/control.py |   83 ++++++++++++++++++++++++++----------------------------
 1 files changed, 40 insertions(+), 43 deletions(-)

diff --git a/stem/control.py b/stem/control.py
index 71f790e..08ce196 100644
--- a/stem/control.py
+++ b/stem/control.py
@@ -679,14 +679,10 @@ class Controller(BaseController):
 
     def _confchanged_listener(event):
       if self.is_caching_enabled():
-        for param, value in event.config.items():
-          cache_key = "getconf.%s" % param.lower()
+        self._set_cache(dict((k, None) for k in event.config), "getconf")
 
-          if cache_key in self._request_cache:
-            del self._request_cache[cache_key]
-
-          if param.lower() == "exitpolicy" and "exit_policy" in self._request_cache:
-            del self._request_cache["exit_policy"]
+        if "exitpolicy" in event.config.keys():
+          self._set_cache({"exitpolicy": None})
 
     self.add_event_listener(_confchanged_listener, EventType.CONF_CHANGED)
 
@@ -755,12 +751,12 @@ class Controller(BaseController):
       params = set(params)
 
     # check for cached results
+
     from_cache = [param.lower() for param in params]
     cached_results = self._get_cache_map(from_cache, "getinfo")
 
-    reply = {}
-    for key in cached_results.keys():
-      user_expected_key = _case_insensitive_lookup(params, key, key)
+    for key in cached_results:
+      user_expected_key = _case_insensitive_lookup(params, key)
       reply[user_expected_key] = cached_results[key]
       params.remove(user_expected_key)
 
@@ -789,6 +785,7 @@ class Controller(BaseController):
 
       if self.is_caching_enabled():
         to_cache = {}
+
         for key, value in response.entries.items():
           key = key.lower()  # make case insensitive
 
@@ -881,6 +878,7 @@ class Controller(BaseController):
     with self._msg_lock:
       try:
         config_policy = self._get_cache("exit_policy")
+
         if not config_policy:
           policy = []
 
@@ -896,6 +894,7 @@ class Controller(BaseController):
             policy += policy_line.split(",")
 
           policy += self.get_info("exit-policy/default").split(",")
+
           config_policy = stem.exit_policy.get_config_policy(policy)
           self._set_cache({"exit_policy": config_policy})
 
@@ -1328,12 +1327,12 @@ class Controller(BaseController):
     lookup_params = set([MAPPED_CONFIG_KEYS.get(entry, entry) for entry in params])
 
     # check for cached results
+
     from_cache = [param.lower() for param in lookup_params]
     cached_results = self._get_cache_map(from_cache, "getconf")
 
-    reply = {}
-    for key in cached_results.keys():
-      user_expected_key = _case_insensitive_lookup(lookup_params, key, key)
+    for key in cached_results:
+      user_expected_key = _case_insensitive_lookup(lookup_params, key)
       reply[user_expected_key] = cached_results[key]
       lookup_params.remove(user_expected_key)
 
@@ -1348,15 +1347,9 @@ class Controller(BaseController):
       reply.update(response.entries)
 
       if self.is_caching_enabled():
-        to_cache = {}
-        for param, value in response.entries.items():
-          param = param.lower()
-
-          if isinstance(value, (bytes, unicode)):
-            value = [value]
-          to_cache[param] = value
-
+        to_cache = dict((k.lower(), v) for k, v in response.entries.items())
         self._set_cache(to_cache, "getconf")
+
       # Maps the entries back to the parameters that the user requested so the
       # capitalization matches (ie, if they request "exitpolicy" then that
       # should be the key rather than "ExitPolicy"). When the same
@@ -1502,10 +1495,17 @@ class Controller(BaseController):
 
       if self.is_caching_enabled():
         to_cache = {}
+
         for param, value in params:
+          param = param.lower()
+
           if isinstance(value, (bytes, unicode)):
             value = [value]
-          to_cache[param.lower()] = value
+
+          to_cache[param] = value
+
+          if param == "exitpolicy":
+            self._set_cache({"exitpolicy": None})
 
         self._set_cache(to_cache, "getconf")
     else:
@@ -1596,25 +1596,24 @@ class Controller(BaseController):
         if not response.is_ok():
           raise stem.ProtocolError("SETEVENTS received unexpected response\n%s" % response)
 
-  def _get_cache(self, param, func=None):
+  def _get_cache(self, param, namespace = None):
     """
-    Queries cache for a configuration option.
+    Queries our request cache for the given key.
 
-    :param str param: key to be queried in cache
-    :param str func: function prefix to keys
+    :param str param: key to be queried
+    :param str namespace: namespace in which to check for the key
 
-    :returns: cached value corresponding to key or None if key wasn't found
+    :returns: cached value corresponding to key or **None** if the key wasn't found
     """
 
-    return self._get_cache_map([param], func).get(param, None)
+    return self._get_cache_map([param], namespace).get(param, None)
 
-  def _get_cache_map(self, params, func=None):
+  def _get_cache_map(self, params, namespace = None):
     """
-    Queries multiple configuration options in cache atomically, returning a
-    mapping of those options to their values.
+    Queries our request cache for multiple entries.
 
-    :param list params: keys to be queried in cache
-    :param str func: function prefix to keys
+    :param list params: keys to be queried
+    :param str namespace: namespace in which to check for the keys
 
     :returns: **dict** of 'param => cached value' pairs of keys present in cache
     """
@@ -1623,8 +1622,8 @@ class Controller(BaseController):
       cached_values = {}
 
       for param in params:
-        if func:
-          cache_key = "%s.%s" % (func, param)
+        if namespace:
+          cache_key = "%s.%s" % (namespace, param)
         else:
           cache_key = param
 
@@ -1633,18 +1632,19 @@ class Controller(BaseController):
 
       return cached_values
 
-  def _set_cache(self, params, func=None):
+  def _set_cache(self, params, namespace = None):
     """
-    Changes the value of tor configuration options in cache atomically.
+    Sets the given request cache entries. If the new cache value is **None**
+    then it is removed from our cache.
 
     :param dict params: **dict** of 'cache_key => value' pairs to be cached
-    :param str func: function prefix to keys
+    :param str namespace: namespace for the keys
     """
 
     with self._cache_lock:
       for key, value in params.items():
-        if func:
-          cache_key = "%s.%s" % (func, key)
+        if namespace:
+          cache_key = "%s.%s" % (namespace, key)
         else:
           cache_key = key
 
@@ -1654,9 +1654,6 @@ class Controller(BaseController):
         else:
           self._request_cache[cache_key] = value
 
-        if key.lower() == "exitpolicy" and "exit_policy" in self._request_cache:
-          del self._request_cache["exit_policy"]
-
   def is_caching_enabled(self):
     """
     **True** if caching has been enabled, **False** otherwise.





More information about the tor-commits mailing list