fix(createImage): surface decode failures instead of hanging forever - #699
Merged
espresso3389 merged 1 commit intoAug 24, 2026
Merged
Conversation
PdfImageExt.createImage() built a Completer and passed decodeImageFromPixels() a success-only callback. That callback has no error channel, so when the engine fails to decode or - far more often - fails to allocate the intermediate texture for a large page on a memory-constrained device, the Completer is never completed at all. The caller does not get an exception, it gets a future that never returns. Three things follow from that: - The `try`/`catch` in PdfPageView._updateImage() cannot fire, because there is nothing to catch. It only ever caught a synchronous throw. - `pageImage.dispose()` sits after the await, so the malloc'd pdfium buffer is never freed. Every failed render leaks a full page bitmap. - The engine's own error still escapes into the zone as an unhandled async error with no stack pointing at pdfrx, which is very hard to attribute. Drive the codec directly instead. This is what decodeImageFromPixels() does internally (ImmutableBuffer -> ImageDescriptor.raw -> instantiateCodec -> getNextFrame), except awaited, so a failure propagates to the caller as an exception. try/finally releases the buffer, descriptor and codec on both paths. With that fixed, PdfPageView._updateImage()'s catch becomes reachable, so two things there need adjusting: - Move `pageImage.dispose()` into a `finally`. It was duplicated across the try and the catch, and skipped entirely if createImage() threw after the first dispose. - Restore the previous `_pageSize` on failure. `_pageSize` is committed before the render and guarded by an equality check, so a page that failed once would never be re-rendered at that size again - the hang would just become a permanently blank page. Only restore it if a newer call has not already claimed `_pageSize`. PdfViewer's two createImage() call sites already use `finally`, so they only benefit from the fix. Adds tests for createImage(), which had none. The last one claims a 4000x3000 image over a 4x3 buffer and asserts a throw; before this change it hung until the suite timed out.
Contributor
Author
|
@espresso3389 you're a machine! TY for the quick merges ❤️ |
Owner
|
This fix has been released in pdfrx 2.4.8. Written by Codex |
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.
Fixes #696.
The problem
PdfImageExt.createImage()creates aCompleter<Image>and handsdecodeImageFromPixels()a success-only callback:ImageDecoderCallbackhas no error parameter, anddecodeImageFromPixelschains.then()without a.catchError. So when the engine fails — in practiceCould not allocate intermediate for pixel conversion, i.e. a large page on a memory-constrained device — the completer is never completed at all. The caller does not get an exception, it gets a future that never returns.Three consequences:
try/catchinPdfPageView._updateImage()cannot fire, because there is nothing to catch.createImageisn'tasync, so thatcatchonly ever saw a synchronous throw from argument validation.pageImage.dispose()— which ismalloc.free(_buffer)— sits after the never-returningawait. Every failed render leaks a full page bitmap.The fix
Drive the codec directly. This is exactly what
decodeImageFromPixelsdoes internally (ImmutableBuffer→ImageDescriptor.raw→instantiateCodec→getNextFrame), except awaited, so a failure propagates to the caller as an exception.try/finallyreleases the buffer, descriptor and codec on both paths; the SDK's version disposes the buffer and descriptor only in its last.then(), so both leak if the chain rejects earlier.Behaviour on success is unchanged: same pixel format, same target size logic, same disposal order as the SDK.
With that fixed, the
catchin_updateImage()becomes reachable, so two things there needed adjusting:pageImage.dispose()moved into afinally. It was duplicated across thetryand thecatch, and skipped entirely ifcreateImage()threw._pageSizeis restored on failure. It is committed before the render and guarded byif (pageSize == _pageSize) return;, so a page that failed once would never be re-rendered at that size again. Without this, fixing the hang just turns one permanently-blank page into another. It only restores if a newer call hasn't already claimed_pageSize, so a superseded render can't stomp a live one.PdfViewer's twocreateImage()call sites already usefinally, so they only benefit from the first change.Tests
createImage()had no test coverage, so I added some: native-size decode, downscaling topixelSizeThreshold, the already-under-threshold no-op, and the regression itself — aPdfImageclaiming 4000x3000 over a 4x3 buffer. That last one asserts a throw; onmasterit hangs until the suite times out:Note what that shows: the engine did report the failure, as
Exception: Codec failed to produce an image, possibly due to invalid image data.— it just arrived as an unhandled error in the zone rather than at theawait, which is consequence 3 above. With the fix the same case throws at the call site and the test passes in under a second.Checks
flutter analyzeclean,dart formatreports no changes, and the new tests pass. Of the pre-existing tests,pdfium_loading_test.dartandpdf_viewer_test.dartneed a real pdfium binary and network access respectively, so I couldn't run them here — happy to adjust if CI flags anything.No CHANGELOG entry, matching #698. Let me know if you'd prefer one.