Package: release.debian.org Severity: normal User: release.debian....@packages.debian.org Usertags: pu
Hi. Paul Wise reported launchpadlib cache corruption in the Debian wiki in #736259. It looks like the concurrency improvements in lazr.restfulclient 0.13.1 helped with the issue, and he's now running a version with the patch attached, and reports an improvement. This seems a useful commit to backport for everyone. SR
diff -Nru lazr.restfulclient-0.12.0/debian/changelog lazr.restfulclient-0.12.0/debian/changelog --- lazr.restfulclient-0.12.0/debian/changelog 2012-01-23 16:59:18.000000000 +0200 +++ lazr.restfulclient-0.12.0/debian/changelog 2014-01-21 18:59:08.000000000 +0200 @@ -1,3 +1,11 @@ +lazr.restfulclient (0.12.0-2+deb7u1) stable; urgency=low + + * debian/patches/concurrency_fixes_rev_122.patch: backport rev 122: Creates + AtomicFileCache as a parent class for MultipleRepresentationCache to + handle concurrent use issues. (Closes: #736259) + + -- Stefano Rivera <stefa...@debian.org> Sun, 05 Jan 2014 09:02:52 +0200 + lazr.restfulclient (0.12.0-2) unstable; urgency=low * New maintainer. diff -Nru lazr.restfulclient-0.12.0/debian/patches/concurrency_fixes_rev_122.patch lazr.restfulclient-0.12.0/debian/patches/concurrency_fixes_rev_122.patch --- lazr.restfulclient-0.12.0/debian/patches/concurrency_fixes_rev_122.patch 1970-01-01 02:00:00.000000000 +0200 +++ lazr.restfulclient-0.12.0/debian/patches/concurrency_fixes_rev_122.patch 2014-01-21 18:59:46.000000000 +0200 @@ -0,0 +1,309 @@ +From: Jonathan Lange <jonathan.la...@gmail.com> +Subject: Creates AtomicFileCache as a parent class for MultipleRepresentationCache to handle concurrent use issues. +Bug-Ubuntu: http://bugs.launchpad.net/bugs/459418 +Bug-Debian: http://bugs.debian.org/736259 + +--- a/src/lazr/restfulclient/_browser.py ++++ b/src/lazr/restfulclient/_browser.py +@@ -1,4 +1,4 @@ +-# Copyright 2008 Canonical Ltd. ++# Copyright 2008,2012 Canonical Ltd. + + # This file is part of lazr.restfulclient. + # +@@ -29,19 +29,21 @@ + 'RestfulHttp', + ] + +- + import atexit +-import gzip ++import errno ++import os + import shutil ++import sys + import tempfile + # Import sleep directly into the module so we can monkey-patch it + # during a test. + from time import sleep + from httplib2 import ( +- FailedToDecompressContent, FileCache, Http, urlnorm) ++ Http, ++ urlnorm, ++ ) + import simplejson + from cStringIO import StringIO +-import zlib + + from urllib import urlencode + from wadllib.application import Application +@@ -49,6 +51,7 @@ + from errors import error_for, HTTPError + from _json import DatetimeJSONEncoder + ++ + # A drop-in replacement for httplib2's safename. + from httplib2 import _md5, re_url_scheme, re_slash + def safename(filename): +@@ -136,7 +139,99 @@ + return None + + +-class MultipleRepresentationCache(FileCache): ++class AtomicFileCache(object): ++ """A FileCache that can be shared by multiple processes. ++ ++ Based on a patch found at ++ <http://code.google.com/p/httplib2/issues/detail?id=125>. ++ """ ++ ++ TEMPFILE_PREFIX = ".temp" ++ ++ def __init__(self, cache, safe=safename): ++ """Construct an ``AtomicFileCache``. ++ ++ :param cache: The directory to use as a cache. ++ :param safe: A function that takes a key and returns a name that's ++ safe to use as a filename. The key must never return a string ++ that begins with ``TEMPFILE_PREFIX``. By default uses ++ ``safename``. ++ """ ++ self._cache_dir = os.path.normpath(cache) ++ self._get_safe_name = safe ++ try: ++ os.makedirs(self._cache_dir) ++ except OSError, e: ++ if e.errno != errno.EEXIST: ++ raise ++ ++ def _get_key_path(self, key): ++ """Return the path on disk where ``key`` is stored.""" ++ safe_key = self._get_safe_name(key) ++ if safe_key.startswith(self.TEMPFILE_PREFIX): ++ # If the cache key starts with the tempfile prefix, then it's ++ # possible that it will clash with a temporary file that we ++ # create. ++ raise ValueError( ++ "Cache key cannot start with '%s'" % self.TEMPFILE_PREFIX) ++ return os.path.join(self._cache_dir, safe_key) ++ ++ def get(self, key): ++ """Get the value of ``key`` if set. ++ ++ This behaves slightly differently to ``FileCache`` in that if ++ ``set()`` fails to store a key, this ``get()`` will behave as if that ++ key were never set whereas ``FileCache`` returns the empty string. ++ ++ :param key: The key to retrieve. Must be either bytes or unicode ++ text. ++ :return: The value of ``key`` if set, None otherwise. ++ """ ++ cache_full_path = self._get_key_path(key) ++ try: ++ f = open(cache_full_path, 'rb') ++ try: ++ return f.read() ++ finally: ++ f.close() ++ except (IOError, OSError), e: ++ if e.errno != errno.ENOENT: ++ raise ++ ++ def set(self, key, value): ++ """Set ``key`` to ``value``. ++ ++ :param key: The key to set. Must be either bytes or unicode text. ++ :param value: The value to set ``key`` to. Must be bytes. ++ """ ++ # Open a temporary file ++ handle, path_name = tempfile.mkstemp( ++ prefix=self.TEMPFILE_PREFIX, dir=self._cache_dir) ++ f = os.fdopen(handle, 'wb') ++ f.write(value) ++ f.close() ++ cache_full_path = self._get_key_path(key) ++ # And rename atomically (on POSIX at least) ++ if sys.platform == 'win32' and os.path.exists(cache_full_path): ++ os.unlink(cache_full_path) ++ os.rename(path_name, cache_full_path) ++ ++ def delete(self, key): ++ """Delete ``key`` from the cache. ++ ++ If ``key`` has not already been set then has no effect. ++ ++ :param key: The key to delete. Must be either bytes or unicode text. ++ """ ++ cache_full_path = self._get_key_path(key) ++ try: ++ os.remove(cache_full_path) ++ except OSError, e: ++ if e.errno != errno.ENOENT: ++ raise ++ ++ ++class MultipleRepresentationCache(AtomicFileCache): + """A cache that can hold different representations of the same resource. + + If a resource has two representations with two media types, +--- /dev/null ++++ b/src/lazr/restfulclient/tests/test_atomicfilecache.py +@@ -0,0 +1,158 @@ ++# Copyright 2012 Canonical Ltd. ++ ++# This file is part of lazr.restfulclient. ++# ++# lazr.restfulclient is free software: you can redistribute it and/or ++# modify it under the terms of the GNU Lesser General Public License ++# as published by the Free Software Foundation, version 3 of the ++# License. ++# ++# lazr.restfulclient is distributed in the hope that it will be ++# useful, but WITHOUT ANY WARRANTY; without even the implied warranty ++# of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU ++# Lesser General Public License for more details. ++# ++# You should have received a copy of the GNU Lesser General Public License ++# along with lazr.restfulclient. If not, see <http://www.gnu.org/licenses/>. ++ ++"""Tests for the atomic file cache.""" ++ ++__metaclass__ = type ++ ++import shutil ++import tempfile ++import unittest ++ ++import httplib2 ++ ++from lazr.restfulclient._browser import AtomicFileCache ++ ++ ++class TestFileCacheInterface(unittest.TestCase): ++ """Tests for ``AtomicFileCache``.""" ++ ++ file_cache_factory = httplib2.FileCache ++ ++ unicode_text = u'pa\u026a\u03b8\u0259n' ++ ++ def setUp(self): ++ super(TestFileCacheInterface, self).setUp() ++ self.cache_dir = tempfile.mkdtemp() ++ ++ def tearDown(self): ++ shutil.rmtree(self.cache_dir) ++ super(TestFileCacheInterface, self).tearDown() ++ ++ def make_file_cache(self): ++ """Make a FileCache-like object to be tested.""" ++ return self.file_cache_factory(self.cache_dir) ++ ++ def test_get_non_existent_key(self): ++ # get() returns None if the key does not exist. ++ cache = self.make_file_cache() ++ self.assertIs(None, cache.get('nonexistent')) ++ ++ def test_set_key(self): ++ # A key set with set() can be got by get(). ++ cache = self.make_file_cache() ++ cache.set('key', 'value') ++ self.assertEqual('value', cache.get('key')) ++ ++ def test_set_twice_overrides(self): ++ # Setting a key again overrides the value. ++ cache = self.make_file_cache() ++ cache.set('key', 'value') ++ cache.set('key', 'new-value') ++ self.assertEqual('new-value', cache.get('key')) ++ ++ def test_delete_absent_key(self): ++ # Deleting a key that's not there does nothing. ++ cache = self.make_file_cache() ++ cache.delete('nonexistent') ++ self.assertIs(None, cache.get('nonexistent')) ++ ++ def test_delete_key(self): ++ # A key once set can be deleted. Further attempts to get that key ++ # return None. ++ cache = self.make_file_cache() ++ cache.set('key', 'value') ++ cache.delete('key') ++ self.assertIs(None, cache.get('key')) ++ ++ def test_get_non_string_key(self): ++ # get() raises TypeError if asked to get a non-string key. ++ cache = self.make_file_cache() ++ self.assertRaises(TypeError, cache.get, 42) ++ ++ def test_delete_non_string_key(self): ++ # delete() raises TypeError if asked to delete a non-string key. ++ cache = self.make_file_cache() ++ self.assertRaises(TypeError, cache.delete, 42) ++ ++ def test_set_non_string_key(self): ++ # set() raises TypeError if asked to set a non-string key. ++ cache = self.make_file_cache() ++ self.assertRaises(TypeError, cache.set, 42, 'the answer') ++ ++ def test_set_non_string_value(self): ++ # set() raises TypeError if asked to set a key to a non-string value. ++ # Attempts to retrieve that value return the empty string. This is ++ # probably a bug in httplib2.FileCache. ++ cache = self.make_file_cache() ++ self.assertRaises(TypeError, cache.set, 'answer', 42) ++ self.assertEqual('', cache.get('answer')) ++ ++ def test_get_unicode(self): ++ # get() can retrieve unicode keys. ++ cache = self.make_file_cache() ++ self.assertIs(None, cache.get(self.unicode_text)) ++ ++ def test_set_unicode_keys(self): ++ cache = self.make_file_cache() ++ cache.set(self.unicode_text, 'value') ++ self.assertEqual('value', cache.get(self.unicode_text)) ++ ++ def test_set_unicode_value(self): ++ # set() cannot store unicode values. Values must be bytes. ++ cache = self.make_file_cache() ++ self.assertRaises( ++ UnicodeEncodeError, cache.set, 'key', self.unicode_text) ++ ++ def test_delete_unicode(self): ++ # delete() can remove unicode keys. ++ cache = self.make_file_cache() ++ cache.set(self.unicode_text, 'value') ++ cache.delete(self.unicode_text) ++ self.assertIs(None, cache.get(self.unicode_text)) ++ ++ ++class TestAtomicFileCache(TestFileCacheInterface): ++ """Tests for ``AtomicFileCache``.""" ++ ++ file_cache_factory = AtomicFileCache ++ ++ def test_set_non_string_value(self): ++ # set() raises TypeError if asked to set a key to a non-string value. ++ # Attempts to retrieve that value act is if it were never set. ++ # ++ # Note: This behaviour differs from httplib2.FileCache. ++ cache = self.make_file_cache() ++ self.assertRaises(TypeError, cache.set, 'answer', 42) ++ self.assertIs(None, cache.get('answer')) ++ ++ # Implementation-specific tests follow. ++ ++ def test_bad_safename_get(self): ++ safename = lambda x: AtomicFileCache.TEMPFILE_PREFIX + x ++ cache = AtomicFileCache(self.cache_dir, safename) ++ self.assertRaises(ValueError, cache.get, 'key') ++ ++ def test_bad_safename_set(self): ++ safename = lambda x: AtomicFileCache.TEMPFILE_PREFIX + x ++ cache = AtomicFileCache(self.cache_dir, safename) ++ self.assertRaises(ValueError, cache.set, 'key', 'value') ++ ++ def test_bad_safename_delete(self): ++ safename = lambda x: AtomicFileCache.TEMPFILE_PREFIX + x ++ cache = AtomicFileCache(self.cache_dir, safename) ++ self.assertRaises(ValueError, cache.delete, 'key') diff -Nru lazr.restfulclient-0.12.0/debian/patches/series lazr.restfulclient-0.12.0/debian/patches/series --- lazr.restfulclient-0.12.0/debian/patches/series 2012-01-19 15:20:12.000000000 +0200 +++ lazr.restfulclient-0.12.0/debian/patches/series 2014-01-05 08:57:02.000000000 +0200 @@ -1,2 +1,3 @@ remove_test_requires no_package_data.patch +concurrency_fixes_rev_122.patch
