Skip to content

fix: serialize MuPDF metadata reads - #2

Merged
Matbe34 merged 1 commit into
masterfrom
fix/pdf-concurrent-metadata
May 4, 2026
Merged

fix: serialize MuPDF metadata reads#2
Matbe34 merged 1 commit into
masterfrom
fix/pdf-concurrent-metadata

Conversation

@Matbe34

@Matbe34 Matbe34 commented May 4, 2026

Copy link
Copy Markdown
Owner

Summary

PDFService.GetMetadata and GetPageDimensions held `RLock` and called into MuPDF (`s.doc.Metadata()`, `s.doc.Bound()`), but MuPDF's `fz_context` is not safe for concurrent access. Two readers proceeding under `RLock` would race inside MuPDF and crash the test binary mid-run (no `--- FAIL` line, just a package-level abort).

`TestConcurrentOperations` exposes this — fails ~3/5 runs locally on master. The previous green CI runs were luck.

Fix: upgrade those two methods to exclusive `Lock`. `GetPageCount` stays `RLock` (only reads a Go int, no MuPDF re-entry).

Performance impact

Negligible. Both methods are microsecond-scale MuPDF reads; the workload is a single-user desktop app with one PDF open. The "lost parallelism" was undefined behavior that segfaulted under load — not a real perf baseline. `GetPageCount` (the hot path) is unaffected.

Test plan

  • `go test -count=1 -run TestConcurrentOperations ./internal/pdf/` — 10/10 PASS after fix (was 3/10 PASS on master)
  • Full pdf suite 5/5 PASS after fix

@Matbe34
Matbe34 merged commit d1fce9c into master May 4, 2026
2 checks passed
@Matbe34
Matbe34 deleted the fix/pdf-concurrent-metadata branch May 4, 2026 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant