fix: prevent PdfiumLibraryBindingsAlreadyInitialized race condition#1255
Open
ethancls wants to merge 1 commit into
Open
fix: prevent PdfiumLibraryBindingsAlreadyInitialized race condition#1255ethancls wants to merge 1 commit into
ethancls wants to merge 1 commit into
Conversation
Wrap the Pdfium instance in a global `Mutex<Option<Pdfium>>` so that `Pdfium::bind_to_library()` is only called once. Concurrent PDF processing (scanner, page prerendering, or rapid page requests) would previously fail because every call to `renderer()` tried to bind the native library, which succeeds at most once per process. The `PdfiumRef` wrapper holds the lock guard and derefs to `Pdfium`, so callers need no changes. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses a concurrency failure in PDF processing by ensuring Pdfium::bind_to_library() is only performed once per process and by serializing PDFium usage behind a global mutex to avoid concurrent initialization and access issues.
Changes:
- Introduces a global
static PDFIUM: Mutex<Option<Pdfium>>to cache a single initializedPdfiuminstance. - Adds a
PdfiumRefwrapper around the mutex guard to provide ergonomicDeref<Target = Pdfium>access while keeping the lock held. - Updates
PdfProcessor::renderer()to initialize on first use and reuse the cached instance thereafter.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+34
to
+42
| /// Held while PDFium is in use; derefs to `Pdfium` for ergonomic access. | ||
| pub struct PdfiumRef<'a>(MutexGuard<'a, Option<Pdfium>>); | ||
|
|
||
| impl<'a> Deref for PdfiumRef<'a> { | ||
| type Target = Pdfium; | ||
| fn deref(&self) -> &Self::Target { | ||
| self.0.as_ref().expect("PDFium not initialized") | ||
| } | ||
| } |
Comment on lines
+200
to
+205
| if let Some(path) = pdfium_path { | ||
| let bindings = Pdfium::bind_to_library(path).map_err(|e| { | ||
| tracing::error!(provided_path = ?path, ?e, "Failed to bind to PDFium library at provided path"); | ||
| FileError::PdfConfigurationError | ||
| })?; | ||
| *guard = Some(Pdfium::new(bindings)); |
Collaborator
|
There is already an active effort to improve this situation in #1209 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When multiple PDFs are processed concurrently (e.g. during library scanning), every call to
PdfProcessor::renderer()attemptsPdfium::bind_to_library(), which can only succeed once per process. The second and subsequent concurrent calls fail with:This breaks PDF thumbnail generation, page rendering, and the built-in reader for all PDF files when more than one PDF is scanned or opened at the same time.
Fix
Wrap the
Pdfiuminstance in a globalMutex<Option<Pdfium>>:PdfiuminstancePdfiumRefwrapper: holds theMutexGuardand implementsDeref<Target=Pdfium>, so all existing callers work without changesChanges
core/src/filesystem/media/format/pdf.rs— 23 lines added, 16 removed. No other files touched.Related
OnceLock<Mutex<Pdfium>>but the locking mechanism was left as a follow-up itemTested
Built and deployed on macOS (ARM64 Docker) with a library containing 100+ PDFs — all thumbnails now generate correctly, no more 500 errors.
🤖 Generated with Claude Code