Fix: take media.lock before the archiver's transaction

The archiver updated the document row inside a transaction and then waited on media.lock. A concurrent file move holding media.lock and updating the same row could deadlock with it, and the unique archive filename was generated outside the lock from an instance loaded before the parse.

The archiver now acquires media.lock before opening its transaction, refreshes the document under the lock, and moves the archive and thumbnail inside that block, so it never waits on media.lock while holding the row.

src/documents/tasks.py

@@ -421,7 +421,13 @@ def update_document_content_maybe_archive_file(
 
             thumbnail = parser.get_thumbnail(document.source_path, mime_type)
 
-            with transaction.atomic():
+            # media.lock first, then the transaction: never wait on media.lock
+            # while this transaction holds the document row. Also keeps the
+            # unique archive filename generation under the lock.
+            with media_lock(), transaction.atomic():
+                # Loaded before the (long) parse; read current metadata now
+                # that the lock is held, for the archive filename.
+                document.refresh_from_db()
                 oldDocument = Document.objects.get(pk=document.pk)
                 if parser.get_archive_path():
                     checksum = compute_checksum(parser.get_archive_path())
@@ -486,11 +492,10 @@ def update_document_content_maybe_archive_file(
                         modified=timezone.now(),
                     )
 
-                with media_lock():
-                    if parser.get_archive_path():
-                        create_source_path_directory(document.archive_path)
-                        shutil.move(parser.get_archive_path(), document.archive_path)
-                    shutil.move(thumbnail, document.thumbnail_path)
+                if parser.get_archive_path():
+                    create_source_path_directory(document.archive_path)
+                    shutil.move(parser.get_archive_path(), document.archive_path)
+                shutil.move(thumbnail, document.thumbnail_path)
 
             document.refresh_from_db()
             logger.info(

src/documents/tests/test_management.py

@@ -23,6 +23,7 @@ from documents.tasks import update_document_content_maybe_archive_file
 from paperless_testing.assertions import FileSystemAssertsMixin
 from paperless_testing.dirs import DirectoriesMixin
 from paperless_testing.factories import DocumentFactory
+from paperless_testing.locks import record_media_lock_acquisitions
 from paperless_testing.samples import SIMPLE_DIGITAL_PDF
 
 sample_file: Path = SIMPLE_DIGITAL_PDF
@@ -63,6 +64,28 @@ class TestArchiver(DirectoriesMixin, FileSystemAssertsMixin, TestCase):
         self.assertTrue(filecmp.cmp(sample_file, doc.source_path))
         self.assertEqual(doc.archive_filename, "none/A.pdf")
 
+    def test_handle_document_takes_media_lock_outside_transaction(self) -> None:
+        """
+        GIVEN:
+            - A document whose archive version is being generated
+        WHEN:
+            - update_document_content_maybe_archive_file runs
+        THEN:
+            - media.lock is acquired
+            - media.lock is never first acquired while a transaction holds the
+              document row
+        """
+        doc = self.make_models()
+        shutil.copy(sample_file, Path(self.dirs.originals_dir) / f"{doc.id:07}.pdf")
+
+        with record_media_lock_acquisitions() as acquisitions:
+            update_document_content_maybe_archive_file(doc.pk)
+
+        self.assertTrue(acquisitions)
+        self.assertFalse(any(a.in_user_transaction for a in acquisitions))
+        doc.refresh_from_db()
+        self.assertIsFile(doc.archive_path)
+
     def test_unknown_mime_type(self) -> None:
         doc = self.make_models()
         doc.mime_type = "sdgfh"