commit e96440eda20e1bbd89ed542654b4f080b8ecb064 Author: Damian Johnson atagar@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.