Rev 3318: Finish fix for bug #125784. need_read/write_lock decorators should attempt to raise an original exception. in http://bzr.arbash-meinel.com/branches/bzr/1.6-dev/safer-lock-decorator
John Arbash Meinel
john at arbash-meinel.com
Sun Jun 8 18:01:06 BST 2008
At http://bzr.arbash-meinel.com/branches/bzr/1.6-dev/safer-lock-decorator
------------------------------------------------------------
revno: 3318
revision-id: john at arbash-meinel.com-20080608170025-6jd29enxnk3d5sy9
parent: andrew.bennetts at canonical.com-20080331015438-1k5c8mqeag6ff9vk
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: safer-lock-decorator
timestamp: Sun 2008-06-08 12:00:25 -0500
message:
Finish fix for bug #125784. need_read/write_lock decorators should attempt to raise an original exception.
-------------- next part --------------
=== modified file 'NEWS'
--- a/NEWS 2008-03-28 07:24:55 +0000
+++ b/NEWS 2008-06-08 17:00:25 +0000
@@ -58,6 +58,12 @@
* Launchpad locations (lp: URLs) can be pulled. (Aaron Bentley, #181945)
+ * ``needs_read_lock`` and ``needs_write_lock`` now suppress an error during
+ ``unlock`` if there was an error in the original function. This helps
+ most when there is a failure with a smart server action, since often the
+ connection closes and we cannot unlock.
+ (Andrew Bennetts, John Arbash Meinel, #125784)
+
DOCUMENTATION:
* Improved documentation on send/merge relationship. (Peter Schuller)
=== modified file 'bzrlib/decorators.py'
--- a/bzrlib/decorators.py 2008-03-31 01:54:38 +0000
+++ b/bzrlib/decorators.py 2008-06-08 17:00:25 +0000
@@ -92,9 +92,17 @@
def %(name)s_read_locked(%(params)s):
self.lock_read()
try:
- return unbound(%(passed_params)s)
- finally:
+ result = unbound(%(passed_params)s)
+ except:
+ import sys
+ exc_info = sys.exc_info()
+ try:
+ self.unlock()
+ finally:
+ raise exc_info[0], exc_info[1], exc_info[2]
+ else:
self.unlock()
+ return result
read_locked = %(name)s_read_locked
"""
params, passed_params = _get_parameters(unbound)
@@ -127,9 +135,17 @@
def read_locked(self, *args, **kwargs):
self.lock_read()
try:
- return unbound(self, *args, **kwargs)
- finally:
+ result = unbound(self, *args, **kwargs)
+ except:
+ import sys
+ exc_info = sys.exc_info()
+ try:
+ self.unlock()
+ finally:
+ raise exc_info[0], exc_info[1], exc_info[2]
+ else:
self.unlock()
+ return result
read_locked.__doc__ = unbound.__doc__
read_locked.__name__ = unbound.__name__
return read_locked
@@ -141,8 +157,9 @@
def %(name)s_write_locked(%(params)s):
self.lock_write()
try:
- result = unbound(self, *args, **kwargs)
+ result = unbound(%(passed_params)s)
except:
+ import sys
exc_info = sys.exc_info()
try:
self.unlock()
=== modified file 'bzrlib/tests/test_decorators.py'
--- a/bzrlib/tests/test_decorators.py 2007-01-24 17:59:50 +0000
+++ b/bzrlib/tests/test_decorators.py 2008-06-08 17:00:25 +0000
@@ -20,37 +20,139 @@
import inspect
from bzrlib import decorators
-from bzrlib.decorators import needs_read_lock, needs_write_lock
from bzrlib.tests import TestCase
-class DecoratorSample(object):
- """Sample class that uses decorators.
+def create_decorator_sample(style, except_in_unlock=False):
+ """Create a DecoratorSample object, using specific lock operators.
- Log when requests go through lock_read()/unlock() or lock_write()/unlock.
+ :param style: The type of lock decorators to use (fast/pretty/None)
+ :param except_in_unlock: If True, raise an exception during unlock
+ :return: An instantiated DecoratorSample object.
"""
- def __init__(self):
- self.actions = []
-
- def lock_read(self):
- self.actions.append('lock_read')
-
- def lock_write(self):
- self.actions.append('lock_write')
-
- def unlock(self):
- self.actions.append('unlock')
-
- @needs_read_lock
- def frob(self):
- """Frob the sample object"""
- self.actions.append('frob')
-
- @needs_write_lock
- def bank(self, bar, biz=None):
- """Bank the sample, but using bar and biz."""
- self.actions.append(('bank', bar, biz))
+ if style is None:
+ # Default
+ needs_read_lock = decorators.needs_read_lock
+ needs_write_lock = decorators.needs_write_lock
+ elif style == 'pretty':
+ needs_read_lock = decorators._pretty_needs_read_lock
+ needs_write_lock = decorators._pretty_needs_write_lock
+ else:
+ needs_read_lock = decorators._fast_needs_read_lock
+ needs_write_lock = decorators._fast_needs_write_lock
+
+ class DecoratorSample(object):
+ """Sample class that uses decorators.
+
+ Log when requests go through lock_read()/unlock() or
+ lock_write()/unlock.
+ """
+
+ def __init__(self):
+ self.actions = []
+
+ def lock_read(self):
+ self.actions.append('lock_read')
+
+ def lock_write(self):
+ self.actions.append('lock_write')
+
+ def unlock(self):
+ if except_in_unlock:
+ self.actions.append('unlock_fail')
+ raise KeyError('during unlock')
+ else:
+ self.actions.append('unlock')
+
+ @needs_read_lock
+ def frob(self):
+ """Frob the sample object"""
+ self.actions.append('frob')
+ return 'newbie'
+
+ @needs_write_lock
+ def bank(self, bar, biz=None):
+ """Bank the sample, but using bar and biz."""
+ self.actions.append(('bank', bar, biz))
+ return (bar, biz)
+
+ @needs_read_lock
+ def fail_during_read(self):
+ self.actions.append('fail_during_read')
+ raise TypeError('during read')
+
+ @needs_write_lock
+ def fail_during_write(self):
+ self.actions.append('fail_during_write')
+ raise TypeError('during write')
+
+ return DecoratorSample()
+
+
+class TestDecoratorActions(TestCase):
+
+ _decorator_style = None # default
+
+ def test_read_lock_locks_and_unlocks(self):
+ sam = create_decorator_sample(self._decorator_style)
+ self.assertEqual('newbie', sam.frob())
+ self.assertEqual(['lock_read', 'frob', 'unlock'], sam.actions)
+
+ def test_write_lock_locks_and_unlocks(self):
+ sam = create_decorator_sample(self._decorator_style)
+ self.assertEqual(('bar', 'bing'), sam.bank('bar', biz='bing'))
+ self.assertEqual(['lock_write', ('bank', 'bar', 'bing'), 'unlock'],
+ sam.actions)
+
+ def test_read_lock_unlocks_during_failure(self):
+ sam = create_decorator_sample(self._decorator_style)
+ self.assertRaises(TypeError, sam.fail_during_read)
+ self.assertEqual(['lock_read', 'fail_during_read', 'unlock'],
+ sam.actions)
+
+ def test_write_lock_unlocks_during_failure(self):
+ sam = create_decorator_sample(self._decorator_style)
+ self.assertRaises(TypeError, sam.fail_during_write)
+ self.assertEqual(['lock_write', 'fail_during_write', 'unlock'],
+ sam.actions)
+
+ def test_read_lock_raises_original_error(self):
+ sam = create_decorator_sample(self._decorator_style,
+ except_in_unlock=True)
+ self.assertRaises(TypeError, sam.fail_during_read)
+ self.assertEqual(['lock_read', 'fail_during_read', 'unlock_fail'],
+ sam.actions)
+
+ def test_write_lock_raises_original_error(self):
+ sam = create_decorator_sample(self._decorator_style,
+ except_in_unlock=True)
+ self.assertRaises(TypeError, sam.fail_during_write)
+ self.assertEqual(['lock_write', 'fail_during_write', 'unlock_fail'],
+ sam.actions)
+
+ def test_read_lock_raises_unlock_error(self):
+ sam = create_decorator_sample(self._decorator_style,
+ except_in_unlock=True)
+ self.assertRaises(KeyError, sam.frob)
+ self.assertEqual(['lock_read', 'frob', 'unlock_fail'], sam.actions)
+
+ def test_write_lock_raises_unlock_error(self):
+ sam = create_decorator_sample(self._decorator_style,
+ except_in_unlock=True)
+ self.assertRaises(KeyError, sam.bank, 'bar', biz='bing')
+ self.assertEqual(['lock_write', ('bank', 'bar', 'bing'),
+ 'unlock_fail'], sam.actions)
+
+
+class TestFastDecoratorActions(TestDecoratorActions):
+
+ _decorator_style = 'fast'
+
+
+class TestPrettyDecoratorActions(TestDecoratorActions):
+
+ _decorator_style = 'pretty'
class TestDecoratorDocs(TestCase):
@@ -58,20 +160,20 @@
def test_read_lock_passthrough(self):
"""@needs_read_lock exposes underlying name and doc."""
- sam = DecoratorSample()
+ sam = create_decorator_sample(None)
self.assertEqual('frob', sam.frob.__name__)
self.assertEqual('Frob the sample object', sam.frob.__doc__)
def test_write_lock_passthrough(self):
"""@needs_write_lock exposes underlying name and doc."""
- sam = DecoratorSample()
+ sam = create_decorator_sample(None)
self.assertEqual('bank', sam.bank.__name__)
self.assertEqual('Bank the sample, but using bar and biz.',
sam.bank.__doc__)
def test_argument_passthrough(self):
"""Test that arguments get passed around properly."""
- sam = DecoratorSample()
+ sam = create_decorator_sample(None)
sam.bank('1', biz='2')
self.assertEqual(['lock_write',
('bank', '1', '2'),
More information about the bazaar-commits
mailing list