Skip to content

Ensure that Wasm-decoding of images work if multiple PDFs are parsed in parallel in one worker-thread (fixes issue 22131) - #22145

Merged
Snuffleupagus merged 1 commit into
mozilla:masterfrom
Snuffleupagus:issue-22131
Oct 12, 2026
Merged

Snuffleupagus merged 1 commit into
mozilla:masterfrom
Snuffleupagus:issue-22131

Conversation

@Snuffleupagus

Copy link
Copy Markdown
Collaborator

The issue only applies when a single worker-thread is used to parse multiple PDF documents in parallel, and with useWorkerFetch = false set, which is always a bad idea since the PDFs will then compete for parsing resources and thus slow things down.
This only applies in the following situations:

  • In Node.js environments, where fake-workers are used.
  • In browsers, if the user manually loads the pdf.worker.mjs into the main-thread; this is however NOT supported.
  • When using GlobalWorkerOptions.workerPort, or when manually passing the same worker option to multiple getDocument calls.

For users affected by this issue the best performing solution would be to either ensure that every getDocument invocation uses its own worker-thread, or that PDFs are parsed in series if workers aren't available.

Please note: This could be seen as a hack, since the "correct" solution would be to ensure that the WasmImage class always uses the MessageHandler that correspond to the currently parsed PDF.
Fixing that would however result in a lot of code and complexity, for a use-case that doesn't apply to the Firefox PDF Viewer, hence we cheat a bit by falling back to any active MessageHandler. While it may not be the "right" one, it should still work fine in general (unless each getDocument invocation uses a unique BinaryDataFactory, which seems like a weird enough use-case that we don't bother with it).

@codecov-commenter

codecov-commenter commented Oct 11, 2026 •

Copy link
Copy Markdown

Codecov Report

βœ… All modified and coverable lines are covered by tests.
βœ… Project coverage is 90.21%. Comparing base (da95557) to head (9851e5e).

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #22145      +/-   ##
==========================================
- Coverage   90.30%   90.21%   -0.09%     
==========================================
  Files         277      277              
  Lines       67949    67956       +7     
==========================================
- Hits        61360    61309      -51     
- Misses       6589     6647      +58     
Flag Coverage Ξ”
browsertest 66.21% <83.33%> (-0.01%) ⬇️
fonttest 8.98% <33.33%> (ΓΈ)
integrationtest 69.13% <50.00%> (-0.15%) ⬇️
unittest 60.19% <100.00%> (-0.01%) ⬇️
unittestcli 58.31% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

β˜” View full report in Codecov by Harness.
πŸ“’ Have feedback on the report? Share it here.

πŸš€ New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • πŸ“¦ JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

sync-pdfs-for-pdf-js Bot pushed a commit to mozilla/pdf.js.refs that referenced this pull request Oct 11, 2026
@github-actions

github-actions Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Viewer preview

πŸ—‘οΈ Viewer previews removed.

sync-pdfs-for-pdf-js Bot pushed a commit to mozilla/pdf.js.refs that referenced this pull request Oct 11, 2026
…in parallel in one worker-thread (fixes issue 22131)

The issue only applies when *a single* worker-thread is used to parse multiple PDF documents *in parallel*, and with `useWorkerFetch = false` set, which is *always a bad idea* since the PDFs will then compete for parsing resources and thus slow things down.
This only applies in the following situations:
 - In Node.js environments, where fake-workers are used.
 - In browsers, if the user manually loads the `pdf.worker.mjs` into the main-thread; this is however NOT supported.
 - When using `GlobalWorkerOptions.workerPort`, or when manually passing the same `worker` option to multiple `getDocument` calls.

For users affected by this issue the best performing solution would be to either ensure that every `getDocument` invocation uses its own worker-thread, or that PDFs are parsed in series if workers aren't available.

*Please note:* This could be seen as a hack, since the "correct" solution would be to ensure that the `WasmImage` class always uses the `MessageHandler` that correspond to the currently parsed PDF.
Fixing that would however result in a lot of code and complexity, for a use-case that doesn't apply to the Firefox PDF Viewer, hence we cheat a bit by falling back to any active `MessageHandler`. While it may not be the "right" one, it should still work fine in general (unless each `getDocument` invocation uses a unique `BinaryDataFactory`, which seems like a weird enough use-case that we don't bother with it).
@github-actions

github-actions Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Reference tests

Results for 9851e5e (run, references from mozilla/pdf.js.refs@a836b1e6b7).

Platform Status Tests Total runtime Errors Different FBF No reference Report
Linux βœ… 1393 10m 38s 0 0 0 0
Windows βœ… 1393 16m 2s 0 0 0 0

The report is kept for 30 days and replaced by the next run.

sync-pdfs-for-pdf-js Bot pushed a commit to mozilla/pdf.js.refs that referenced this pull request Oct 11, 2026

@calixteman calixteman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you.

This branch was previously deployed

1 inactive deployment
sync_pdfs β€” 9851e5ea Deployed Oct 11, 2026 by Snuffleupagus via request #359
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: In Node.js, rendering a JPEG 2000 image never finishes after another document opened later was destroyed

3 participants