Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add context manager for unpinning #57

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 22 additions & 5 deletions multidb/pinning.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@


__all__ = ['this_thread_is_pinned', 'pin_this_thread', 'unpin_this_thread',
'use_primary_db', 'use_master', 'db_write']
'use_primary_db', 'use_secondary_db', 'use_master', 'db_write']


_locals = threading.local()
Expand All @@ -33,8 +33,8 @@ def unpin_this_thread():
_locals.pinned = False


class UsePrimaryDB(object):
"""A contextmanager/decorator to use the primary database."""
class _UseDB(object):
"""A contextmanager/decorator to use the specified database."""
def __call__(self, func):
@wraps(func)
def decorator(*args, **kw):
Expand All @@ -45,13 +45,29 @@ def decorator(*args, **kw):
def __enter__(self):
_locals.old = getattr(_locals, 'old', [])
_locals.old.append(this_thread_is_pinned())
pin_this_thread()

def __exit__(self, type, value, tb):
if not _locals.old.pop():
previous_state = _locals.old.pop()
if previous_state:
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ This introduces a subtle behaviour change. Previously, manual use of unpin_this_thread() would leak out of the context block, for example when:

  1. a thread is pinned
  2. enters the context block (_locals.old = True)
  3. is manually unpinned (with unpin_this_thread())
  4. exits the context block (not _locals.old == False so no action is taken)
  5. the thread will be unpinned when it reaches here

This new code will always restore the previous pinning state when exiting a context block. In my view this is more predictable, but I'm open to your opinion on this.

pin_this_thread()
else:
unpin_this_thread()


class UsePrimaryDB(_UseDB):
"""A contextmanager/decorator to use the primary database."""
def __enter__(self):
super(UsePrimaryDB, self).__enter__()
pin_this_thread()


class UseSecondaryDB(_UseDB):
"""A contextmanager/decorator to use the secondary database."""
def __enter__(self):
super(UseSecondaryDB, self).__enter__()
unpin_this_thread()


class DeprecatedUseMaster(UsePrimaryDB):
def __enter__(self):
warnings.warn(
Expand All @@ -63,6 +79,7 @@ def __enter__(self):


use_primary_db = UsePrimaryDB()
use_secondary_db = UseSecondaryDB()
use_master = DeprecatedUseMaster()


Expand Down
250 changes: 249 additions & 1 deletion multidb/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@
pinning_cookie_samesite, pinning_cookie_secure,
pinning_seconds, PinningRouterMiddleware)
from multidb.pinning import (this_thread_is_pinned, pin_this_thread,
unpin_this_thread, use_primary_db, db_write)
unpin_this_thread, use_primary_db,
use_secondary_db, db_write)


class UnpinningTestCase(TestCase):
Expand Down Expand Up @@ -218,6 +219,22 @@ def check_outer():
check_outer()
assert not this_thread_is_pinned()

def test_decorator_nested_mixed(self):
@use_primary_db
def check_inner():
assert this_thread_is_pinned()

@use_secondary_db
def check_outer():
assert not this_thread_is_pinned()
check_inner()
assert not this_thread_is_pinned()

unpin_this_thread()
assert not this_thread_is_pinned()
check_outer()
assert not this_thread_is_pinned()

def test_decorator_resets(self):
@use_primary_db
def check():
Expand All @@ -243,6 +260,22 @@ def check_outer():
check_outer()
assert this_thread_is_pinned()

def test_decorator_resets_nested_mixed(self):
@use_primary_db
def check_inner():
assert this_thread_is_pinned()

@use_secondary_db
def check_outer():
assert not this_thread_is_pinned()
check_inner()
assert not this_thread_is_pinned()

pin_this_thread()
assert this_thread_is_pinned()
check_outer()
assert this_thread_is_pinned()

def test_context_manager(self):
unpin_this_thread()
assert not this_thread_is_pinned()
Expand All @@ -260,6 +293,16 @@ def test_context_manager_nested(self):
assert this_thread_is_pinned()
assert not this_thread_is_pinned()

def test_context_manager_nested_mixed(self):
unpin_this_thread()
assert not this_thread_is_pinned()
with use_secondary_db:
assert not this_thread_is_pinned()
with use_primary_db:
assert this_thread_is_pinned()
assert not this_thread_is_pinned()
assert not this_thread_is_pinned()

def test_context_manager_resets(self):
pin_this_thread()
assert this_thread_is_pinned()
Expand All @@ -277,6 +320,16 @@ def test_context_manager_resets_nested(self):
assert this_thread_is_pinned()
assert this_thread_is_pinned()

def test_context_manager_resets_nested_mixed(self):
pin_this_thread()
assert this_thread_is_pinned()
with use_secondary_db:
assert not this_thread_is_pinned()
with use_primary_db:
assert this_thread_is_pinned()
assert not this_thread_is_pinned()
assert this_thread_is_pinned()

def test_context_manager_exception(self):
unpin_this_thread()
assert not this_thread_is_pinned()
Expand Down Expand Up @@ -334,6 +387,201 @@ def thread2_worker():
self.assertEqual(pinned[1], False)


class UseSecondaryDBTests(TestCase):
def test_decorator(self):
@use_secondary_db
def check():
assert not this_thread_is_pinned()
pin_this_thread()
assert this_thread_is_pinned()
check()
assert this_thread_is_pinned()

def test_decorator_nested(self):
@use_secondary_db
def check_inner():
assert not this_thread_is_pinned()

@use_secondary_db
def check_outer():
assert not this_thread_is_pinned()
check_inner()
assert not this_thread_is_pinned()

pin_this_thread()
assert this_thread_is_pinned()
check_outer()
assert this_thread_is_pinned()

def test_decorator_nested_mixed(self):
@use_secondary_db
def check_inner():
assert not this_thread_is_pinned()

@use_primary_db
def check_outer():
assert this_thread_is_pinned()
check_inner()
assert this_thread_is_pinned()

pin_this_thread()
assert this_thread_is_pinned()
check_outer()
assert this_thread_is_pinned()

def test_decorator_resets(self):
@use_secondary_db
def check():
assert not this_thread_is_pinned()
unpin_this_thread()
assert not this_thread_is_pinned()
check()
assert not this_thread_is_pinned()

def test_decorator_resets_nested(self):
@use_secondary_db
def check_inner():
assert not this_thread_is_pinned()

@use_secondary_db
def check_outer():
assert not this_thread_is_pinned()
check_inner()
assert not this_thread_is_pinned()

unpin_this_thread()
assert not this_thread_is_pinned()
check_outer()
assert not this_thread_is_pinned()

def test_decorator_resets_nested_mixed(self):
@use_secondary_db
def check_inner():
assert not this_thread_is_pinned()

@use_primary_db
def check_outer():
assert this_thread_is_pinned()
check_inner()
assert this_thread_is_pinned()

unpin_this_thread()
assert not this_thread_is_pinned()
check_outer()
assert not this_thread_is_pinned()

def test_context_manager(self):
pin_this_thread()
assert this_thread_is_pinned()
with use_secondary_db:
assert not this_thread_is_pinned()
assert this_thread_is_pinned()

def test_context_manager_nested(self):
pin_this_thread()
assert this_thread_is_pinned()
with use_secondary_db:
assert not this_thread_is_pinned()
with use_secondary_db:
assert not this_thread_is_pinned()
assert not this_thread_is_pinned()
assert this_thread_is_pinned()

def test_context_manager_nested_mixed(self):
pin_this_thread()
assert this_thread_is_pinned()
with use_primary_db:
assert this_thread_is_pinned()
with use_secondary_db:
assert not this_thread_is_pinned()
assert this_thread_is_pinned()
assert this_thread_is_pinned()

def test_context_manager_resets(self):
unpin_this_thread()
assert not this_thread_is_pinned()
with use_secondary_db:
assert not this_thread_is_pinned()
assert not this_thread_is_pinned()

def test_context_manager_resets_nested(self):
unpin_this_thread()
assert not this_thread_is_pinned()
with use_secondary_db:
assert not this_thread_is_pinned()
with use_secondary_db:
assert not this_thread_is_pinned()
assert not this_thread_is_pinned()
assert not this_thread_is_pinned()

def test_context_manager_resets_nested_mixed(self):
unpin_this_thread()
assert not this_thread_is_pinned()
with use_primary_db:
assert this_thread_is_pinned()
with use_secondary_db:
assert not this_thread_is_pinned()
assert this_thread_is_pinned()
assert not this_thread_is_pinned()

def test_context_manager_exception(self):
pin_this_thread()
assert this_thread_is_pinned()
with self.assertRaises(ValueError):
with use_secondary_db:
assert not this_thread_is_pinned()
raise ValueError
assert this_thread_is_pinned()

def test_multithreaded_unpinning(self):
thread1_lock = Lock()
thread2_lock = Lock()
thread1_lock.acquire()
thread2_lock.acquire()
orchestrator = Lock()
orchestrator.acquire()

pinned = {}

def thread1_worker():
pin_this_thread()
with use_secondary_db:
orchestrator.release()
thread1_lock.acquire()

pinned[1] = this_thread_is_pinned()

def thread2_worker():
unpin_this_thread()
with use_secondary_db:
orchestrator.release()
thread2_lock.acquire()

pinned[2] = this_thread_is_pinned()
orchestrator.release()

thread1 = Thread(target=thread1_worker)
thread2 = Thread(target=thread2_worker)

# thread1 starts, entering `use_primary_db` from a pinned state
thread1.start()
orchestrator.acquire()

# thread2 starts, entering `use_primary_db` from an unpinned state
thread2.start()
orchestrator.acquire()

# thread2 finishes, returning to an unpinned state
thread2_lock.release()
thread2.join()
self.assertEqual(pinned[2], False)

# thread1 finishes, returning to a pinned state
thread1_lock.release()
thread1.join()
self.assertEqual(pinned[1], True)


class DeprecationTestCase(TestCase):
def test_masterslaverouter(self):
with warnings.catch_warnings(record=True) as w:
Expand Down