Attempt to handle threading-related issues with extension reloading.
Review Request #5866 — Created May 22, 2014 and submitted
We observed some issues in a mod_worker environment where attempts to
reload extensions would fail, causing state to become corrupted, leading
to more and more errors. Some of these cases were made more
bullet-proof, but we didn't fix the core issue.This change attempts to prevent what may be causing the issue by
allowing only one thread to reload the list of extensions at a time.We have a lock set up for reloading extensions (in case two users try to
reload in the admin UI at the same time).There's a separate lock in the middleware that's a bit smarter about
calling load(). It checks if the extension state has expired, tries to
grab a lock, and then checks again for expiration. The goal there is to
not have a bunch of threads that are waiting decide they're all going to
trigger their own reloads.Unit tests have been written that hit issues both during uninit and
during init of extensions. These mirror the errors we've been getting
on reviews.reviewboard.org with Review Bot. With the fix, these tests
pass.
I don't have a real-world repro case yet (we'll see in a couple days on
reviews.reviewboard.org if we start to hit any problems), but I was able
to reproduce some of the errors we through unit tests.Basically, there's a number of possible failure points, depending on what
instructions are being executed, but the tests manage to reliably trigger
one error during init and one during uninit. Those trigger only if I turn off
locking. Everything passes if locking is on.
Description | From | Last Updated |
---|---|---|
local variable 'extension' is assigned to but never used |
reviewbot | |
Col: 80 E501 line too long (80 > 79 characters) |
reviewbot | |
local variable 'hook' is assigned to but never used |
reviewbot | |
local variable 'hook' is assigned to but never used |
reviewbot | |
Can we just get rid of the outer is_expired call and always do this with the lock acquired? |
david | |
local variable 'hook' is assigned to but never used |
reviewbot | |
local variable 'hook' is assigned to but never used |
reviewbot |
-
This is a review from Review Bot. Tool: Pyflakes Processed Files: djblets/extensions/manager.py djblets/extensions/middleware.py djblets/extensions/tests.py Ignored Files:
-
-
-
- Change Summary:
-
- Fixed a line length issue.
- Removed an unused variable and unnecessary statement.
- Commit:
-
8e113da3f9061e31c81fed4652e7e7cca8fe6af08891572d565ceb39007cdadf1c7546f96fd6e639
-
This is a review from Review Bot. Tool: PEP8 Style Checker Processed Files: djblets/extensions/manager.py djblets/extensions/middleware.py djblets/extensions/tests.py Ignored Files: