diff --git a/djblets/extensions/manager.py b/djblets/extensions/manager.py index f595e8248bc4f6e133bf3b1118c87ecbd87d9e88..2efdf1bcf3430c75a44f2eab46eb7afa559e9642 100644 --- a/djblets/extensions/manager.py +++ b/djblets/extensions/manager.py @@ -41,7 +41,6 @@ from django.conf import settings from django.conf.urls import patterns, include from django.contrib.admin.sites import AdminSite from django.core.cache import cache -from django.core.files import locks from django.core.management import call_command from django.core.management.base import CommandError from django.core.management.color import no_style @@ -66,6 +65,7 @@ from djblets.extensions.models import RegisteredExtension from djblets.extensions.signals import (extension_initialized, extension_uninitialized) from djblets.urls.resolvers import DynamicURLResolver +from djblets.util.compat.django.core.files import locks class SettingListWrapper(object): @@ -703,9 +703,9 @@ class ExtensionManager(object): while old_version != cur_version: with open(lockfile, 'w') as f: try: - locks.lock(f, locks.LOCK_EX) + locks.lock(f, locks.LOCK_EX | locks.LOCK_NB) except IOError as e: - if e.errno == errno.EINTR: + if e.errno in (errno.EAGAIN, errno.EACCES, errno.EINTR): # Sleep for one second, then try again time.sleep(1) extension.settings.load() @@ -722,7 +722,17 @@ class ExtensionManager(object): locks.unlock(f) - os.unlink(lockfile) + try: + os.unlink(lockfile) + except OSError as e: + # A "No such file or directory" (ENOENT) is most likely due to + # another thread removing the lock file before this thread could. + # It's safe to ignore. We want to handle all others, though. + if e.errno != errno.ENOENT: + logging.error("Failed to unlock media lock file '%s' for " + "extension '%s': %s", + lockfile, ext_class.info, e, + exc_info=1) def _install_extension_media_internal(self, ext_class): """Installs extension data. diff --git a/djblets/extensions/tests.py b/djblets/extensions/tests.py index 8d8b20a8ab0709cfca206fdf976436525bab8753..0fa067acc6bc9b6a7fdc0d1d7c3505f42c10bc1d 100644 --- a/djblets/extensions/tests.py +++ b/djblets/extensions/tests.py @@ -493,65 +493,22 @@ class ExtensionManagerTest(SpyAgency, TestCase): # # With proper locking, these issues don't come up. That's what # this test case is attempting to check for. - def _sleep_and_call(manager, orig_func, *args): - # This works well enough to throw a monkey wrench into things. - # One thread will be slightly ahead of the other. - time.sleep(0.2) - - try: - orig_func(*args) - except Exception as e: - logging.error('%s\n', e, exc_info=1) - exceptions.append(e) - - def _init_extension(manager, *args): - _sleep_and_call(manager, orig_init_extension, *args) - - def _uninit_extension(manager, *args): - _sleep_and_call(manager, orig_uninit_extension, *args) - - def _loader(main_connection): - # Insert the connection from the main thread, so that we can - # perform lookups. We never write. - from django.db import connections - connections['default'] = main_connection - - self.manager.load(full_reload=True) # Enable one extension. This extension's state will get a bit messed # up if the thread locking fails. We only need one to trigger this. self.assertEqual(len(self.manager.get_installed_extensions()), 1) self.manager.enable_extension(self.extension_class.id) - orig_init_extension = self.manager._init_extension - orig_uninit_extension = self.manager._uninit_extension - self.spy_on(self.manager._load_extensions) - self.spy_on(self.manager._init_extension, call_fake=_init_extension) - self.spy_on(self.manager._uninit_extension, - call_fake=_uninit_extension) - - # Store the main connection. We're going to let the threads share it. - # This trick courtesy of the Django unit tests - # (django/tests/bakcends/tests.py) - from django.db import connections - main_connection = connections['default'] - main_connection.allow_thread_sharing = True - - exceptions = [] + self._spy_sleep_and_call(self.manager._init_extension) + self._spy_sleep_and_call(self.manager._uninit_extension) - # Make the load request twice, simultaneously. - t1 = threading.Thread(target=_loader, args=[main_connection]) - t2 = threading.Thread(target=_loader, args=[main_connection]) - t1.start() - t2.start() - t1.join() - t2.join() + self._run_thread_test(lambda: self.manager.load(full_reload=True)) self.assertEqual(len(self.manager._load_extensions.calls), 2) self.assertEqual(len(self.manager._uninit_extension.calls), 2) self.assertEqual(len(self.manager._init_extension.calls), 2) - self.assertEqual(exceptions, []) + self.assertEqual(self.exceptions, []) def test_enable_registers_static_bundles(self): """Testing ExtensionManager registers static bundles when enabling extension""" @@ -606,6 +563,36 @@ class ExtensionManagerTest(SpyAgency, TestCase): self.assertTrue(extension.registration.installed) self.assertIsNotNone(extension.settings.get(version_key)) + def test_install_media_concurrent_threads(self): + """Testing ExtensionManager updating media for existing + extension with concurrent threads + """ + version_key = ExtensionManager.VERSION_SETTINGS_KEY + + extension = self.extension_class(extension_manager=self.manager) + extension.registration.installed = True + extension.registration.enabled = True + extension.registration.save() + extension.__class__.instance = extension + + extension.settings.set(version_key, '0.5') + extension.settings.save() + + self.assertEqual(len(self.manager.get_installed_extensions()), 1) + + self.spy_on(self.manager._install_extension_media) + self.spy_on(self.manager._install_extension_media_internal, + call_original=False) + + self._run_thread_test( + lambda: self.manager._install_extension_media(extension.__class__)) + + self.assertEqual( + len(self.manager._install_extension_media.calls), 2) + self.assertEqual( + len(self.manager._install_extension_media_internal.calls), 1) + self.assertEqual(self.exceptions, []) + def test_disable_unregisters_static_bundles(self): """Testing ExtensionManager unregisters static bundles when disabling extension""" settings.PIPELINE_CSS = {} @@ -701,6 +688,54 @@ class ExtensionManagerTest(SpyAgency, TestCase): self.assertEqual(extension1.settings[setting_key], setting_val) self.assertEqual(extension2.settings[setting_key], setting_val) + def _run_thread_test(self, main_func): + def _thread_main(main_connection, main_func): + # Insert the connection from the main thread, so that we can + # perform lookups. We never write. + from django.db import connections + + connections['default'] = main_connection + + main_func() + + # Store the main connection. We're going to let the threads share it. + # This trick courtesy of the Django unit tests + # (django/tests/backends/tests.py). + from django.db import connections + + main_connection = connections['default'] + main_connection.allow_thread_sharing = True + + self.exceptions = [] + + t1 = threading.Thread(target=_thread_main, + args=[main_connection, main_func]) + t2 = threading.Thread(target=_thread_main, + args=[main_connection, main_func]) + t1.start() + t2.start() + t1.join() + t2.join() + + def _sleep_and_call(self, manager, orig_func, *args, **kwargs): + # This works well enough to throw a monkey wrench into things. + # One thread will be slightly ahead of the other. + time.sleep(0.2) + + try: + orig_func(*args, **kwargs) + except Exception as e: + logging.error('%s\n', e, exc_info=1) + self.exceptions.append(e) + + def _spy_sleep_and_call(self, func): + def _call(manager, *args, **kwargs): + self._sleep_and_call(manager, orig_func, *args, **kwargs) + + orig_func = func + + self.spy_on(func, call_fake=_call) + class SettingListWrapperTests(TestCase): """Unit tests for djblets.extensions.manager.SettingListWrapper.""" diff --git a/djblets/util/compat/__init__.py b/djblets/util/compat/__init__.py new file mode 100644 index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 diff --git a/djblets/util/compat/django/__init__.py b/djblets/util/compat/django/__init__.py new file mode 100644 index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 diff --git a/djblets/util/compat/django/core/__init__.py b/djblets/util/compat/django/core/__init__.py new file mode 100644 index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 diff --git a/djblets/util/compat/django/core/files/__init__.py b/djblets/util/compat/django/core/files/__init__.py new file mode 100644 index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 diff --git a/djblets/util/compat/django/core/files/locks.py b/djblets/util/compat/django/core/files/locks.py new file mode 100644 index 0000000000000000000000000000000000000000..bf892dd50f3fdf3c774ea60ba9d19ee8955a4b6d --- /dev/null +++ b/djblets/util/compat/django/core/files/locks.py @@ -0,0 +1,115 @@ +# TODO: Remove this file once we no longer support a version of Django +# prior to 1.7. +""" +Portable file locking utilities. + +Based partially on an example by Jonathan Feignberg in the Python +Cookbook [1] (licensed under the Python Software License) and a ctypes port by +Anatoly Techtonik for Roundup [2] (license [3]). + +[1] http://aspn.activestate.com/ASPN/Cookbook/Python/Recipe/65203 +[2] http://sourceforge.net/p/roundup/code/ci/default/tree/roundup/backends/portalocker.py +[3] http://sourceforge.net/p/roundup/code/ci/default/tree/COPYING.txt + +Example Usage:: + + >>> from django.core.files import locks + >>> with open('./file', 'wb') as f: + ... locks.lock(f, locks.LOCK_EX) + ... f.write('Django') +""" +import os + +__all__ = ('LOCK_EX', 'LOCK_SH', 'LOCK_NB', 'lock', 'unlock') + + +def _fd(f): + """Get a filedescriptor from something which could be a file or an fd.""" + return f.fileno() if hasattr(f, 'fileno') else f + + +if os.name == 'nt': + import msvcrt + from ctypes import (sizeof, c_ulong, c_void_p, c_int64, + Structure, Union, POINTER, windll, byref) + from ctypes.wintypes import BOOL, DWORD, HANDLE + + LOCK_SH = 0 # the default + LOCK_NB = 0x1 # LOCKFILE_FAIL_IMMEDIATELY + LOCK_EX = 0x2 # LOCKFILE_EXCLUSIVE_LOCK + + # --- Adapted from the pyserial project --- + # detect size of ULONG_PTR + if sizeof(c_ulong) != sizeof(c_void_p): + ULONG_PTR = c_int64 + else: + ULONG_PTR = c_ulong + PVOID = c_void_p + + # --- Union inside Structure by stackoverflow:3480240 --- + class _OFFSET(Structure): + _fields_ = [ + ('Offset', DWORD), + ('OffsetHigh', DWORD)] + + class _OFFSET_UNION(Union): + _anonymous_ = ['_offset'] + _fields_ = [ + ('_offset', _OFFSET), + ('Pointer', PVOID)] + + class OVERLAPPED(Structure): + _anonymous_ = ['_offset_union'] + _fields_ = [ + ('Internal', ULONG_PTR), + ('InternalHigh', ULONG_PTR), + ('_offset_union', _OFFSET_UNION), + ('hEvent', HANDLE)] + + LPOVERLAPPED = POINTER(OVERLAPPED) + + # --- Define function prototypes for extra safety --- + LockFileEx = windll.kernel32.LockFileEx + LockFileEx.restype = BOOL + LockFileEx.argtypes = [HANDLE, DWORD, DWORD, DWORD, DWORD, LPOVERLAPPED] + UnlockFileEx = windll.kernel32.UnlockFileEx + UnlockFileEx.restype = BOOL + UnlockFileEx.argtypes = [HANDLE, DWORD, DWORD, DWORD, LPOVERLAPPED] + + def lock(f, flags): + hfile = msvcrt.get_osfhandle(_fd(f)) + overlapped = OVERLAPPED() + ret = LockFileEx(hfile, flags, 0, 0, 0xFFFF0000, byref(overlapped)) + return bool(ret) + + def unlock(f): + hfile = msvcrt.get_osfhandle(_fd(f)) + overlapped = OVERLAPPED() + ret = UnlockFileEx(hfile, 0, 0, 0xFFFF0000, byref(overlapped)) + return bool(ret) +else: + try: + import fcntl + LOCK_SH = fcntl.LOCK_SH # shared lock + LOCK_NB = fcntl.LOCK_NB # non-blocking + LOCK_EX = fcntl.LOCK_EX + except (ImportError, AttributeError): + # File locking is not supported. + LOCK_EX = LOCK_SH = LOCK_NB = 0 + + # Dummy functions that don't do anything. + def lock(f, flags): + # File is not locked + return False + + def unlock(f): + # File is unlocked + return True + else: + def lock(f, flags): + ret = fcntl.flock(_fd(f), flags) + return (ret == 0) + + def unlock(f): + ret = fcntl.flock(_fd(f), fcntl.LOCK_UN) + return (ret == 0)