Rev 6101: (jameinel) Handle bug #702914, in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

[Canonical.com Patch Queue Manager]

At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 6101 [merge]
revision-id: pqm at pqm.ubuntu.com-20110825140403-9xf3jcvxzxrazjuv
parent: pqm at pqm.ubuntu.com-20110825103314-h70ju19dco9937aa
parent: benji at benjiyork.com-20110819181931-ltgm1rp1n27uh9e2
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Thu 2011-08-25 14:04:03 +0000
message:
  (jameinel) Handle bug #702914,
   avoid race conditions removing attributes of ImportReplacer. (Benji York)
modified:
  bzrlib/lazy_import.py          lazy_import.py-20060910203832-f77c54gf3n232za0-1
  bzrlib/tests/test_lazy_import.py test_lazy_import.py-20060910203832-f77c54gf3n232za0-2
=== modified file 'bzrlib/lazy_import.py'
--- a/bzrlib/lazy_import.py	2011-05-18 16:42:48 +0000
+++ b/bzrlib/lazy_import.py	2011-08-19 18:02:37 +0000
@@ -98,8 +98,19 @@
 
     def _cleanup(self):
         """Stop holding on to all the extra stuff"""
-        del self._factory
-        del self._scope
+        try:
+            del self._factory
+        except AttributeError:
+            # Oops, we just lost a race with another caller of _cleanup.  Just
+            # ignore it.
+            pass
+
+        try:
+            del self._scope
+        except AttributeError:
+            # Another race loss.  See above.
+            pass
+
         # We keep _name, so that we can report errors
         # del self._name
 

=== modified file 'bzrlib/tests/test_lazy_import.py'
--- a/bzrlib/tests/test_lazy_import.py	2011-05-13 12:51:05 +0000
+++ b/bzrlib/tests/test_lazy_import.py	2011-08-19 18:19:31 +0000
@@ -16,7 +16,9 @@
 
 """Test the lazy_import functionality."""
 
+import linecache
 import os
+import re
 import sys
 
 from bzrlib import (
@@ -1167,3 +1169,79 @@
                           ('_import', 'root8'),
                           ('import', self.root_name, []),
                          ], self.actions)
+
+
+class TestScopeReplacerReentrance(TestCase):
+    """The ScopeReplacer should be reentrant.
+
+    Invoking a replacer while an invocation was already on-going leads to a
+    race to see which invocation will be the first to delete the _factory and
+    _scope attributes.  The loosing caller used to see AttributeErrors (bug
+    702914).
+
+    These tests set up a tracer that stops at the moment just before one of
+    the attributes is being deleted and starts another call to the
+    functionality in question (__call__, __getattribute__, __setattr_) in
+    order win the race, setting up the originall caller to loose.
+    """
+
+    def tracer(self, frame, event, arg):
+        # Grab the name of the file that contains the code being executed.
+        filename = frame.f_globals["__file__"]
+        # Convert ".pyc" and ".pyo" file names to their ".py" equivalent.
+        filename = re.sub(r'\.py[co]$', '.py', filename)
+        # If we're executing a line of code from the right module...
+        if event == 'line' and 'lazy_import.py' in filename:
+            line = linecache.getline(filename, frame.f_lineno)
+            # ...and the line of code is the one we're looking for...
+            if 'del self._factory' in line:
+                # We don't need to trace any more.
+                sys.settrace(None)
+                # Run another racer.  This one will "win" the race, deleting
+                # the attributes.  When the first racer resumes it will loose
+                # the race, generating an AttributeError.
+                self.racer()
+        return self.tracer
+
+    def run_race(self, racer):
+        self.racer = racer
+        sys.settrace(self.tracer)
+        self.racer() # Should not raise an AttributeError
+        # Make sure the tracer actually found the code it was looking for.  If
+        # not, maybe the code was refactored in such a way that these tests
+        # aren't needed any more.
+        self.assertEqual(None, sys.gettrace())
+
+    def test_call(self):
+        def factory(*args):
+            return factory
+        replacer = lazy_import.ScopeReplacer({}, factory, 'name')
+        self.run_race(replacer)
+
+    def test_setattr(self):
+        class Replaced:
+            pass
+
+        def factory(*args):
+            return Replaced()
+
+        replacer = lazy_import.ScopeReplacer({}, factory, 'name')
+
+        def racer():
+            replacer.foo = 42
+
+        self.run_race(racer)
+
+    def test_getattribute(self):
+        class Replaced:
+            foo = 'bar'
+
+        def factory(*args):
+            return Replaced()
+
+        replacer = lazy_import.ScopeReplacer({}, factory, 'name')
+
+        def racer():
+            replacer.foo
+
+        self.run_race(racer)