diff --git a/spoolman/database/database.py b/spoolman/database/database.py index 6f9c7559..782df215 100644 --- a/spoolman/database/database.py +++ b/spoolman/database/database.py @@ -152,7 +152,9 @@ class Database: busy_since = busy_since or time.monotonic() if time.monotonic() - busy_since > BACKUP_LOCK_TIMEOUT_SECONDS: raise sqlite3.OperationalError( - f"Database stayed locked for over {BACKUP_LOCK_TIMEOUT_SECONDS} seconds, giving up on the backup.", + f"Database {self.connection_url.database} stayed locked for over " + f"{BACKUP_LOCK_TIMEOUT_SECONDS} seconds, giving up on the backup. " + "Is another program holding it open?", ) if self.connection_url.database == target_path: @@ -164,9 +166,12 @@ class Database: # transaction manager that commits on exit, it does not close the connection. Leaving the # handles open leaks them on every backup, and on Windows it makes the caller's unlink/move # of the file we just wrote fail with "used by another process" (POSIX unlink hides this). + # nolock=1: the target is a scratch file nothing else opens, so it needs no file locks. Backup + # folders on network shares often refuse SQLite's locks (CIFS without nobrl, NFS without + # lockd), which failed every backup with a "locked" database that was not locked at all (#1246). with ( closing(sqlite3.connect(self.connection_url.database)) as src, - closing(sqlite3.connect(target_path)) as dst, + closing(sqlite3.connect(f"{Path(target_path).absolute().as_uri()}?nolock=1", uri=True)) as dst, ): src.backup(dst, pages=1, progress=progress) diff --git a/tests/test_backup_rotation.py b/tests/test_backup_rotation.py index 09065c60..d447dd6e 100644 --- a/tests/test_backup_rotation.py +++ b/tests/test_backup_rotation.py @@ -157,3 +157,33 @@ def test_backup_gives_up_on_a_lock_that_never_clears( db.backup_and_rotate(backups) holder.execute("ROLLBACK") assert not (backups / f"{BACKUP_NAME}.pending").exists() + + +def test_backup_needs_no_lock_on_the_target(db: Database, backups: Path, monkeypatch: pytest.MonkeyPatch): + """Backup folders on network shares often refuse file locks, which failed every backup (#1246). + + Simulate that by holding an exclusive lock on the target from the moment it is created. + """ + monkeypatch.setattr(database, "BACKUP_LOCK_TIMEOUT_SECONDS", 0.5) + pending = backups / f"{BACKUP_NAME}.pending" + real_connect = sqlite3.connect + holders: list[sqlite3.Connection] = [] + + def connect(*args: object, **kwargs: object) -> sqlite3.Connection: + conn = real_connect(*args, **kwargs) + if pending.exists() and not holders: + holders.append(real_connect(pending, isolation_level=None)) + holders[0].execute("BEGIN EXCLUSIVE") + return conn + + monkeypatch.setattr(sqlite3, "connect", connect) + try: + result = db.backup_and_rotate(backups) + finally: + for holder in holders: + holder.close() + + assert holders, "the target was never locked, so this test proves nothing" + assert result.created is True + with closing(real_connect(backups / BACKUP_NAME)) as conn: + assert conn.execute("SELECT note FROM spool").fetchall() == [("original",)]