mirror of
https://github.com/paperless-ngx/paperless-ngx.git
synced 2026-11-05 06:19:25 +00:00
Fix: take media.lock before emptying the trash
Emptying the trash deleted documents inside Django's delete transaction, and the post_delete file cleanup waited on media.lock while the deleted rows were locked. A concurrent file move holding media.lock and updating one of those rows could deadlock with it. empty_trash now holds media.lock around the delete, so the cleanup handler re-enters the lock instead of waiting while rows are locked.
This commit is contained in:
1 parent
54a8198167
commit
505df89660
2 files changed
+33
-1
No files matched your search
@@ -540,7 +540,11 @@ def empty_trash(doc_ids=None) -> None:
|
||||
deleted_document_ids = list(documents.values_list("id", flat=True))
|
||||
# Temporarily connect the cleanup handler
|
||||
models.signals.post_delete.connect(cleanup_document_deletion, sender=Document)
|
||||
documents.delete() # this is effectively a hard delete
|
||||
# media.lock before Django's delete transaction: post_delete runs
|
||||
# cleanup_document_deletion inside it while the rows are locked, and
|
||||
# that handler re-enters the lock instead of waiting.
|
||||
with media_lock():
|
||||
documents.delete() # this is effectively a hard delete
|
||||
logger.info(f"Deleted {len(deleted_document_ids)} documents from trash")
|
||||
|
||||
if settings.AUDIT_LOG_ENABLED:
|
||||
|
||||
@@ -25,6 +25,7 @@ from documents.file_moves import request_file_move
|
||||
from documents.models import CustomField
|
||||
from documents.models import Document
|
||||
from documents.tasks import bulk_update_documents
|
||||
from documents.tasks import empty_trash
|
||||
from paperless.celery import app as celery_app
|
||||
from paperless.middleware import FileMoveMiddleware
|
||||
from paperless_testing.factories import CorrespondentFactory
|
||||
@@ -833,3 +834,30 @@ class TestCeleryTaskScope:
|
||||
|
||||
assert (paperless_dirs.originals_dir / "first-renamed.pdf").is_file()
|
||||
assert (paperless_dirs.originals_dir / "second-renamed.pdf").is_file()
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
class TestEmptyTrashLockOrdering:
|
||||
def test_hard_delete_takes_media_lock_outside_transaction(
|
||||
self,
|
||||
document: Document,
|
||||
paperless_dirs: PaperlessDirs,
|
||||
) -> None:
|
||||
"""
|
||||
GIVEN:
|
||||
- A trashed document with its file on disk
|
||||
WHEN:
|
||||
- The trash is emptied for it
|
||||
THEN:
|
||||
- media.lock is never first acquired inside a transaction
|
||||
- The document row and its file are gone
|
||||
"""
|
||||
document.delete()
|
||||
|
||||
with record_media_lock_acquisitions() as acquisitions:
|
||||
empty_trash([document.pk])
|
||||
|
||||
assert acquisitions
|
||||
assert not any(a.in_user_transaction for a in acquisitions)
|
||||
assert not Document.global_objects.filter(pk=document.pk).exists()
|
||||
assert not (paperless_dirs.originals_dir / "original.pdf").exists()
|
||||
Reference in new issue
Block a user