Skip to content

fix[issue #142]: SIGSEV during concurrent rendering of PDF in Image() or ImageDPI() - #147

Closed
intx4 wants to merge 2 commits into
gen2brain:masterfrom
intx4:master
Closed

fix[issue #142]: SIGSEV during concurrent rendering of PDF in Image() or ImageDPI()#147
intx4 wants to merge 2 commits into
gen2brain:masterfrom
intx4:master

Conversation

@intx4

@intx4 intx4 commented Apr 23, 2026

Copy link
Copy Markdown

Summary

This PR fixes intermittent SIGSEGVs during concurrent rendering by adding proper MuPDF lock wiring during context creation and tightening resource lifetimes in the cgo path.

Root cause: fz_new_context_imp was called with nil lock callbacks, which violates MuPDF’s multithreading contract when rendering overlaps across contexts/goroutines.

What changed

  • Added MuPDF lock callbacks (fz_locks_context) in cgo:

    • Implemented lock/unlock callbacks and FZ_LOCK_MAX mutex array.
    • Initialized once per process and shared by all contexts.
    • Passed lock context into fz_new_context_imp(...) for every Document.
  • Hardened context/document lifecycle:

    • Centralized context creation in shared init path.
    • Added constructor cleanup on partial failures.
    • Made Close() cleanup ordering explicit and defensive (stream -> doc -> context -> C buffer).
  • Fixed NewFromMemory lifetime hazards:

    • Switched to C-owned byte buffer (C.CBytes) instead of relying on Go slice memory.
    • Kept stream reference with fz_keep_stream and released temporary stream ref.
    • Freed C buffer on Close().
  • Added/updated documentation:

    • README now clearly states:
      • parallel rendering is supported across separate Document instances,
      • concurrency on the same Document (including racing with Close) is not supported.
    • Added code comments in fitz_cgo.go documenting same-document concurrency limits and lock-free method expectations.

Why this fixes the crash

MuPDF requires caller-provided locks for safe multithreaded use. With lock callbacks registered, internal shared paths (alloc/glyph/freetype and related global state) are synchronized correctly across concurrent rendering workloads, preventing unsafe overlap that could lead to SIGSEGV.

Compatibility

  • Existing call sites (New, NewFromMemory, NewFromReader, ImageDPI, etc.) continue to work unchanged.

Tests

  • Added a test file to verify the fix
  • You need to provide your own concurrency.pdf file for the test to work. I could not share the one I used as it contains sensitive data (it is an artifact from my app production environment)

Notes

  • This PR intentionally does not add same-document concurrent access support.
  • Callers should continue using one Document per worker/goroutine for parallel conversion workflows.

…r concurrent use across multiple Document instances, while clearly documenting that a single Document is still not concurrent-safe.
@intx4 intx4 changed the title fix: implemented fix focuses on making MuPDF context creation safe for… fix[issue 142]: implemented fix focuses on making MuPDF context creation safe for… Apr 23, 2026
@intx4

intx4 commented Apr 23, 2026

Copy link
Copy Markdown
Author

This fixes #142

@intx4 intx4 changed the title fix[issue 142]: implemented fix focuses on making MuPDF context creation safe for… fix[issue 142]: SIGSEV during concurrent rendering of PDF in Image() or ImageDPI() Apr 28, 2026
@intx4 intx4 changed the title fix[issue 142]: SIGSEV during concurrent rendering of PDF in Image() or ImageDPI() fix[issue #142]: SIGSEV during concurrent rendering of PDF in Image() or ImageDPI() Apr 28, 2026
@gen2brain

Copy link
Copy Markdown
Owner

Thanks, sorry, I do not have the time to test this now. I will need the go-fitz library, so I will probably update binaries and update it to the latest version and will test this. Are these failures just because of a missing PDF in testdata, can't we use already existing PDFs to test this?

@gen2brain

Copy link
Copy Markdown
Owner

This installs one global lock array that is shared, basically it destroys the parallelism.

@gen2brain gen2brain closed this Jun 26, 2026
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.

2 participants