[tor-bugs] #8607 [Stem]: Controller's cache isn't thread safe

[Tor Bug Tracker & Wiki]

#8607: Controller's cache isn't thread safe
-----------------------------+----------------------------------------------
 Reporter:  atagar           |          Owner:  atagar        
     Type:  defect           |         Status:  needs_revision
 Priority:  normal           |      Milestone:                
Component:  Stem             |        Version:                
 Keywords:  controller easy  |         Parent:                
   Points:                   |   Actualpoints:                
-----------------------------+----------------------------------------------
Changes (by atagar):

  * status:  needs_review => needs_revision


Comment:

 Great. Mostly just a few nitpicks.

 {{{
 +  def _get_cache(self, params, func=None):
 +    """
 +    Reads cached items
 }}}

 This, and the comment for _update_cache(), aren't terribly descriptive. It
 doesn't need to be much, but please at least make it a complete sentence
 with punctuation. ;)

 {{{
 +      for param in params:
 +        if func:
 +          cache_key = "%s.%s" % (func, param)
 +        else:
 +          cache_key = param
 +        if cache_key in self._request_cache:
 +          cached_values[param] = self._request_cache[cache_key]
 +      return cached_values
 }}}

 Please don't scrunch code like this. Have an empty line before the second
 'if' and 'return' clauses. This will help readability.

 {{{
 +  def _update_cache(self, params, strtolist=True):
 +    """
 +    Updates cache
 +
 +    :param dict params: **dict** of 'cache_key => value' pairs to be
 cached
 +    """
 }}}

 'strtolist' isn't mentioned in the pydocs. Maybe the caller should be
 doing the list wrapper?

 {{{
 +        if cache_key.lower().endswith("exitpolicy") and "exit_policy" in
 self._request_cache:
 +          del self._request_cache["exit_policy"]
 }}}

 This endswith() check might catch things that it isn't supposed to (ie, a
 cache key like 'something.new.exitpolicy').

 Stem has a style checker. Please always run 'python run_tests.py --all'
 prior to submitting patches. The style checker requires pep8...

 {{{
 atagar at morrigan:~/Desktop/stem$ ./run_tests.py --style
 ======================================================================
                              INITIALISING
 ======================================================================

 Performing startup activities...
   checking for orphaned .pyc files... done

 STYLE ISSUES
 * stem/control.py
   line 1326 - E128 continuation line under-indented for visual indent

 TESTING PASSED (39 seconds)
 }}}

 Cheers! -Damian

-- 
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/8607#comment:4>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online