Rev 5361: Make LocationConfig use a lock too. in file:///home/vila/src/bzr/bugs/525571-lock-bazaar-conf-files/
Vincent Ladeuil
v.ladeuil+lp at free.fr
Mon Aug 23 13:45:50 BST 2010
At file:///home/vila/src/bzr/bugs/525571-lock-bazaar-conf-files/
------------------------------------------------------------
revno: 5361
revision-id: v.ladeuil+lp at free.fr-20100823124549-uoszb3y8ih24kncm
parent: v.ladeuil+lp at free.fr-20100823093203-gndeu5r71wft0pl6
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: lockable-config-files
timestamp: Mon 2010-08-23 14:45:49 +0200
message:
Make LocationConfig use a lock too.
* bzrlib/tests/test_config.py:
(lockable_config_scenarios, load_tests): What configs should be
tested as lockable ones.
(TestIniConfigBuilding.test_ini_config_ownership): Fixed.
(TestLockableConfig.get_existing_config): Some tests need to load
an existing config file rather than creating it.
(TestLockableConfig.test_read_while_writing): Ensure we get the
old config until the lock is released.
-------------- next part --------------
=== modified file 'bzrlib/config.py'
--- a/bzrlib/config.py 2010-08-23 09:32:03 +0000
+++ b/bzrlib/config.py 2010-08-23 12:45:49 +0000
@@ -594,7 +594,7 @@
self._write_config_file()
-class LocationConfig(IniBasedConfig):
+class LocationConfig(LockableConfig):
"""A configuration object that gives the policy for a location."""
def __init__(self, location, _content=None):
@@ -701,6 +701,7 @@
if policy_key in self._get_parser()[section]:
del self._get_parser()[section][policy_key]
+ @needs_write_lock
def set_user_option(self, option, value, store=STORE_LOCATION):
"""Save option and its value in the configuration."""
if store not in [STORE_LOCATION,
@@ -709,7 +710,6 @@
raise ValueError('bad storage policy %r for %r' %
(store, option))
self.reload()
- # FIXME: RBC 20051029 This should take a file lock on locations.conf.
location = self.location
if location.endswith('/'):
location = location[:-1]
=== modified file 'bzrlib/tests/test_config.py'
--- a/bzrlib/tests/test_config.py 2010-08-23 09:32:03 +0000
+++ b/bzrlib/tests/test_config.py 2010-08-23 12:45:49 +0000
@@ -40,6 +40,30 @@
from bzrlib.util.configobj import configobj
+def lockable_config_scenarios():
+ return [
+ ('global',
+ {'config_class': config.GlobalConfig,
+ 'config_args': [],
+ 'config_section': 'DEFAULT'}),
+ ('locations',
+ {'config_class': config.LocationConfig,
+ 'config_args': ['.'],
+ 'config_section': '.'}),]
+
+
+def load_tests(standard_tests, module, loader):
+ suite = loader.suiteClass()
+
+ lc_tests, remaining_tests = tests.split_suite_by_condition(
+ standard_tests, tests.condition_isinstance((
+ TestLockableConfig,
+ )))
+ tests.multiply_tests(lc_tests, lockable_config_scenarios(), suite)
+ suite.addTest(remaining_tests)
+ return suite
+
+
sample_long_alias="log -r-15..-1 --line"
sample_config_text = u"""
[DEFAULT]
@@ -398,9 +422,9 @@
self.requireFeature(features.chown_feature)
self.overrideAttr(os, 'chown', self._dummy_chown)
self.path = self.uid = self.gid = None
- conf = config.IniBasedConfig(file_name='foo.conf')
+ conf = config.IniBasedConfig(file_name='./foo.conf')
conf._write_config_file()
- self.assertEquals(self.path, 'foo.conf')
+ self.assertEquals(self.path, './foo.conf')
self.assertTrue(isinstance(self.uid, int))
self.assertTrue(isinstance(self.gid, int))
@@ -446,15 +470,21 @@
class TestLockableConfig(tests.TestCaseInTempDir):
- config_class = config.GlobalConfig
+ # Set by load_tests
+ config_class = None
+ config_args = None
+ config_section = None
def setUp(self):
super(TestLockableConfig, self).setUp()
- self._content = '[DEFAULT]\none=1\ntwo=2'
+ self._content = '[%s]\none=1\ntwo=2\n' % (self.config_section,)
self.config = self.create_config(self._content)
+ def get_existing_config(self):
+ return self.config_class(*self.config_args)
+
def create_config(self, content):
- c = self.config_class(_content=content)
+ c = self.config_class(*self.config_args, _content=content)
c._write_config_file()
return c
@@ -467,7 +497,7 @@
def test_listen_to_the_last_speaker(self):
c1 = self.config
- c2 = self.create_config(self._content)
+ c2 = self.get_existing_config()
c1.set_user_option('one', 'ONE')
c2.set_user_option('two', 'TWO')
self.assertEquals('ONE', c1.get_user_option('one'))
@@ -479,7 +509,7 @@
# If the same config is not shared, the same variable modified twice
# can only see a single result.
c1 = self.config
- c2 = self.create_config(self._content)
+ c2 = self.get_existing_config()
c1.set_user_option('one', 'c1')
c2.set_user_option('one', 'c2')
self.assertEquals('c2', c2._get_user_option('one'))
@@ -490,8 +520,8 @@
self.assertEquals('c2', c1._get_user_option('one'))
def test_writes_are_serialized(self):
- c1 = self.create_config(self._content)
- c2 = self.create_config(self._content)
+ c1 = self.config
+ c2 = self.get_existing_config()
# We spawn a thread that will pause *during* the write
before_writing = threading.Event()
@@ -525,6 +555,43 @@
c2.set_user_option('one', 'c2')
self.assertEquals('c2', c2.get_user_option('one'))
+ def test_read_while_writing(self):
+ c1 = self.config
+ # We spawn a thread that will pause *during* the write
+ ready_to_write = threading.Event()
+ do_writing = threading.Event()
+ writing_done = threading.Event()
+ c1_orig = c1._write_config_file
+ def c1_write_config_file():
+ ready_to_write.set()
+ # The lock is held we wait for the main thread to decide when to
+ # continue
+ do_writing.wait()
+ c1_orig()
+ writing_done.set()
+ c1._write_config_file = c1_write_config_file
+ def c1_set_option():
+ c1.set_user_option('one', 'c1')
+ t1 = threading.Thread(target=c1_set_option)
+ # Collect the thread after the test
+ self.addCleanup(t1.join)
+ # Be ready to unblock the thread if the test goes wrong
+ self.addCleanup(do_writing.set)
+ t1.start()
+ # Ensure the thread is ready to write
+ ready_to_write.wait()
+ self.assertTrue(c1._lock.is_held)
+ self.assertEquals('c1', c1.get_user_option('one'))
+ # If we read during the write, we get the old value
+ c2 = self.get_existing_config()
+ self.assertEquals('1', c2.get_user_option('one'))
+ # Let the writing occur and ensure it occurred
+ do_writing.set()
+ writing_done.wait()
+ # Now we get the updated value
+ c3 = self.get_existing_config()
+ self.assertEquals('c1', c3.get_user_option('one'))
+
class TestGetUserOptionAs(TestIniConfig):
More information about the bazaar-commits
mailing list