Skip to content

Commit

Permalink
Fix #613 -- Lock single row instead of full user table
Browse files Browse the repository at this point in the history
Some databases showed poor performance locking the whole user table, especially on considerable table sizes.
To mitigate this issue, we changed to locking only a single users' row. The current hijacker will be locked, until the transaction is completed. The transaction wraps the entire request. Thanks, @vinodpandey, for reporting the issue and providing a patch.
  • Loading branch information
vinodpandey committed Nov 28, 2023
1 parent 30afdff commit a36fbb8
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 3 deletions.
4 changes: 3 additions & 1 deletion hijack/tests/test_views.py
Expand Up @@ -18,7 +18,9 @@ def test_dispatch__custom_manager(self, admin_user, rf):
class LockedView(views.LockUserTableMixin, View):
pass

LockedView().dispatch(rf.get("/"))
request = rf.get("/")
request.user = admin_user
LockedView().dispatch(request)


class TestSuccessUrlMixin:
Expand Down
4 changes: 2 additions & 2 deletions hijack/views.py
Expand Up @@ -63,8 +63,8 @@ def get_redirect_url(self):
class LockUserTableMixin:
@transaction.atomic()
def dispatch(self, request, *args, **kwargs):
# Lock entire user table to avoid race conditions
next(get_user_model()._base_manager.select_for_update().iterator())
# Lock user row to avoid race conditions
get_user_model()._base_manager.select_for_update().get(pk=request.user.pk)
return super().dispatch(request, *args, **kwargs)


Expand Down

0 comments on commit a36fbb8

Please sign in to comment.