Skip to content

Commit fa2f2f7

Browse files
Fix database locking and race conditions in cover_enforcer
Resolves issue where CWA hangs when editing metadata and cover simultaneously, particularly on network shares or when NETWORK_SHARE_MODE is enabled. Changes: - Increase SQLite connection timeout from 30s to 60s throughout cover_enforcer.py to handle slower network share operations - Add retry logic with exponential backoff (3 attempts) to calibredb export, specifically handling "database is locked" errors - Replace os.system() with subprocess.run() for ebook-polish operations: * Better error handling and logging * 120s timeout to prevent indefinite hangs * Capture stderr/stdout for debugging - Add 0.5s delay before ebook-polish execution to ensure file buffers are flushed and locks released - Explicit database connection closure before file modification operations All changes occur in background service; no UI impact. Fixes #904
1 parent aff0fa5 commit fa2f2f7

1 file changed

Lines changed: 72 additions & 12 deletions

File tree

scripts/cover_enforcer.py

Lines changed: 72 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ def __init__(self, book_dir: str, file_path: str):
119119

120120
def get_split_library(self) -> dict[str, str] | None:
121121
"""Checks whether or not the user has split library enabled. Returns None if they don't and the path of the Split Library location if True."""
122-
con = sqlite3.connect("/config/app.db", timeout=30)
122+
con = sqlite3.connect("/config/app.db", timeout=60)
123123
cur = con.cursor()
124124
split_library = cur.execute('SELECT config_calibre_split FROM settings;').fetchone()[0]
125125

@@ -157,9 +157,44 @@ def get_title_and_author(self) -> tuple[str, str, str]:
157157

158158
def get_new_metadata_path(self) -> str:
159159
"""Uses the export function of the calibredb utility to export any new metadata for the given book to metadata_temp, and returns the path to the new metadata.opf"""
160-
subprocess.run(["calibredb", "export", "--with-library", self.calibre_library, "--to-dir", metadata_temp_dir, self.book_id], env=self.calibre_env, check=True)
161-
temp_files = [os.path.join(dirpath,f) for (dirpath, dirnames, filenames) in os.walk(metadata_temp_dir) for f in filenames]
162-
return [f for f in temp_files if f.endswith('.opf')][0]
160+
# Add retry logic with exponential backoff to handle database locks
161+
max_retries = 3
162+
for attempt in range(max_retries):
163+
try:
164+
# Add small delay before first attempt to allow other operations to complete
165+
if attempt > 0:
166+
delay = 2 ** attempt # Exponential backoff: 2s, 4s
167+
print(f"[cover-metadata-enforcer] Retrying calibredb export (attempt {attempt + 1}/{max_retries}) after {delay}s delay...", flush=True)
168+
time.sleep(delay)
169+
else:
170+
# Small initial delay to ensure database writes are flushed
171+
time.sleep(0.5)
172+
173+
result = subprocess.run(
174+
["calibredb", "export", "--with-library", self.calibre_library, "--to-dir", metadata_temp_dir, self.book_id],
175+
env=self.calibre_env, check=False, capture_output=True, text=True, timeout=60
176+
)
177+
178+
if result.returncode == 0:
179+
temp_files = [os.path.join(dirpath,f) for (dirpath, dirnames, filenames) in os.walk(metadata_temp_dir) for f in filenames]
180+
opf_files = [f for f in temp_files if f.endswith('.opf')]
181+
if opf_files:
182+
return opf_files[0]
183+
else:
184+
raise FileNotFoundError("No .opf file found after calibredb export")
185+
else:
186+
if attempt < max_retries - 1 and "database is locked" in result.stderr.lower():
187+
continue # Retry on database lock
188+
else:
189+
raise subprocess.CalledProcessError(result.returncode, result.args, result.stdout, result.stderr)
190+
except subprocess.TimeoutExpired:
191+
if attempt < max_retries - 1:
192+
continue
193+
else:
194+
raise
195+
196+
# If all retries failed
197+
raise RuntimeError(f"Failed to export metadata for book {self.book_id} after {max_retries} attempts")
163198

164199

165200
def export_as_dict(self) -> dict[str,str | None]:
@@ -203,7 +238,7 @@ def __init__(self, args):
203238

204239
# Read Calibre-Web setting: config_unicode_filename (True -> transliterate non-English in filenames)
205240
try:
206-
with sqlite3.connect("/config/app.db", timeout=30) as con:
241+
with sqlite3.connect("/config/app.db", timeout=60) as con:
207242
cur = con.cursor()
208243
self.unicode_filename = bool(cur.execute('SELECT config_unicode_filename FROM settings;').fetchone()[0])
209244
except Exception:
@@ -222,7 +257,7 @@ def _ascii_transliterate(self, s: str) -> str:
222257

223258
def get_split_library(self) -> dict[str, str] | None:
224259
"""Checks whether or not the user has split library enabled. Returns None if they don't and the path of the Split Library location if True."""
225-
con = sqlite3.connect("/config/app.db", timeout=30)
260+
con = sqlite3.connect("/config/app.db", timeout=60)
226261
cur = con.cursor()
227262
split_library = cur.execute('SELECT config_calibre_split FROM settings;').fetchone()[0]
228263

@@ -268,7 +303,7 @@ def _recalculate_checksum_after_modification(self, book_id: str, file_format: st
268303
"metadata.db"
269304
)
270305

271-
con = sqlite3.connect(metadb_path, timeout=30)
306+
con = sqlite3.connect(metadb_path, timeout=60)
272307

273308
try:
274309
success = store_checksum(
@@ -372,9 +407,12 @@ def get_book_dir_from_log(self, log_info: dict) -> str:
372407
(self.split_library or {}).get("db_path", self.calibre_library),
373408
"metadata.db",
374409
)
375-
with sqlite3.connect(metadb_path, timeout=30) as con:
410+
con = sqlite3.connect(metadb_path, timeout=60)
411+
try:
376412
cur = con.cursor()
377413
row = cur.execute('SELECT path FROM books WHERE id = ?', (book_id,)).fetchone()
414+
finally:
415+
con.close()
378416
if row and row[0]:
379417
resolved = os.path.join(self.calibre_library, row[0])
380418
resolved = resolved if resolved.endswith(os.sep) else resolved + os.sep
@@ -507,10 +545,32 @@ def enforce_cover(self, book_dir: str) -> list:
507545
for file in supported_files:
508546
book = Book(book_dir, file)
509547
self.replace_old_metadata(book.old_metadata_path, book.new_metadata_path)
510-
if Path(book.cover_path).exists():
511-
os.system(f'ebook-polish -c "{book.cover_path}" -o "{book.new_metadata_path}" -U "{file}" "{file}"')
512-
else:
513-
os.system(f'ebook-polish -o "{book.new_metadata_path}" -U "{file}" "{file}"')
548+
549+
# Use subprocess instead of os.system for better error handling
550+
# Add small delay to ensure any file locks are released
551+
time.sleep(0.5)
552+
553+
try:
554+
if Path(book.cover_path).exists():
555+
result = subprocess.run(
556+
['ebook-polish', '-c', book.cover_path, '-o', book.new_metadata_path, '-U', file, file],
557+
capture_output=True, text=True, timeout=120, check=False
558+
)
559+
else:
560+
result = subprocess.run(
561+
['ebook-polish', '-o', book.new_metadata_path, '-U', file, file],
562+
capture_output=True, text=True, timeout=120, check=False
563+
)
564+
565+
if result.returncode != 0:
566+
print(f"[cover-metadata-enforcer] Warning: ebook-polish returned {result.returncode} for {file}", flush=True)
567+
if result.stderr:
568+
print(f"[cover-metadata-enforcer] Error output: {result.stderr.strip()}", flush=True)
569+
except subprocess.TimeoutExpired:
570+
print(f"[cover-metadata-enforcer] Error: ebook-polish timed out for {file}", flush=True)
571+
except Exception as e:
572+
print(f"[cover-metadata-enforcer] Error running ebook-polish for {file}: {e}", flush=True)
573+
514574
self.empty_metadata_temp()
515575
print(f"[cover-metadata-enforcer]: DONE: '{book.title_author}.{book.file_format}': Cover & Metadata updated", flush=True)
516576

0 commit comments

Comments
 (0)