Skip to content

Handle document destruction during a hanging range request (issue 22051) - #22164

Merged
Snuffleupagus merged 1 commit into
mozilla:masterfrom
Snuffleupagus:issue-22051
Oct 14, 2026
Merged

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

Conversation

@Snuffleupagus

@Snuffleupagus Snuffleupagus commented Oct 12, 2026 •

Copy link
Copy Markdown
Collaborator

Also, avoid any "indirect" calls to ChunkedStreamManager.prototype.#requestChunks if loading has been aborted.

@github-actions

github-actions Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Reference tests

Results for 79dce90 (run, references from mozilla/pdf.js.refs@23d8c278ca).

Platform Status Tests Total runtime Errors Different FBF No reference Report
Linux ⚠️ 1393 12m 41s 0 1 0 0 view differences
Windows βœ… 1393 14m 50s 0 0 0 0

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

@codecov-commenter

codecov-commenter commented Oct 12, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.75000% with 1 line in your changes missing coverage. Please review.
βœ… Project coverage is 90.36%. Comparing base (6c71c0b) to head (79dce90).

Files with missing lines Patch % Lines
src/core/chunked_stream.js 92.85% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #22164      +/-   ##
==========================================
+ Coverage   90.34%   90.36%   +0.02%     
==========================================
  Files         277      277              
  Lines       68030    68030              
==========================================
+ Hits        61461    61477      +16     
+ Misses       6569     6553      -16     
Flag Coverage Ξ”
browsertest 66.18% <87.50%> (-0.03%) ⬇️
fonttest 8.98% <ΓΈ> (ΓΈ)
integrationtest 69.37% <62.50%> (+<0.01%) ⬆️
unittest 60.47% <93.75%> (+0.03%) ⬆️
unittestcli 58.35% <93.75%> (+0.02%) ⬆️

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 12, 2026
@github-actions

github-actions Bot commented Oct 12, 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 12, 2026
@Snuffleupagus
Snuffleupagus marked this pull request as ready for review October 12, 2026 19:01
@calixteman
calixteman requested a balanced review from Copilot October 12, 2026 19:25

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟑 Changes recommended

Background range prefetch can produce an unhandled rejection during destruction.

2 open findings
What changed in this PR

Fixes document destruction while a range request is stalled.

Changes:

  • Propagates aborted range-request failures.
  • Defers cleanup so pending API calls can reject.
  • Adds a regression test for issue 22051.
File Description
src/​core/​chunked_stream.js Changes request rejection handling.
src/​core/​worker.js Allows pending requests to settle before cleanup.
test/​unit/​api_spec.js Tests destruction during a hanging request.

🧠 Review effort: Balanced


πŸ’‘ Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/core/chunked_stream.js Resolved
Comment thread src/core/worker.js Resolved
sync-pdfs-for-pdf-js Bot pushed a commit to mozilla/pdf.js.refs that referenced this pull request Oct 13, 2026
sync-pdfs-for-pdf-js Bot pushed a commit to mozilla/pdf.js.refs that referenced this pull request Oct 13, 2026
@Snuffleupagus
Snuffleupagus requested review from calixteman and removed request for calixteman October 13, 2026 12:07
Comment thread src/core/chunked_stream.js Resolved
Also, avoid any "indirect" calls to `ChunkedStreamManager.prototype.#requestChunks` if loading has been aborted.
sync-pdfs-for-pdf-js Bot pushed a commit to mozilla/pdf.js.refs that referenced this pull request Oct 13, 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.

@Snuffleupagus
Snuffleupagus merged commit 6c5ef4c into mozilla:master Oct 14, 2026
26 checks passed
@Snuffleupagus
Snuffleupagus deleted the issue-22051 branch October 14, 2026 11:02
sync-pdfs-for-pdf-js Bot pushed a commit to mozilla/pdf.js.refs that referenced this pull request Oct 14, 2026

This branch was previously deployed

1 inactive deployment
sync_pdfs β€” 79dce900 Deployed Oct 13, 2026 by Snuffleupagus via request #397
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.

PDFDocumentLoadingTask.destroy() never resolves when called while a page waits for range data

4 participants