Repository navigation
Conversation
…ocation failure Two defects in the viewport buffer path killed consuming apps in the field (Kotlin/Native turns an uncaught worker exception into process death): 1. ConsumerBufferPool.getBufferViewport disposed the previous viewport buffer inline whenever a larger viewport was requested — with no coordination with an in-flight render still holding it (PdfView renders in a debounced produceState coroutine on Dispatchers.IO, so zoom/rotation restarts it mid-flight) or with the displayed image, which references the buffer's pixels zero-copy via extractSubset. Calling withAddress on the disposed buffer dies natively (SIGABRT in SkBitmap::peekPixels on JVM, SIGSEGV on iOS); the silent variant has the native renderer writing through a dangling pointer. This is likely the root cause of dshatz#41 ("recycled bitmap" while zooming on Android — the same race in the Android buffer implementation). Replaced buffers are now retired instead of disposed and cleaned up on a later request once their consumer freed them. Handed-out viewport buffers are marked unfree; PdfViewport releases the replaced image (buffer-identity guarded, since same-size requests reuse the instance). 2. ConsumerBufferUtil.allocate ignored allocPixels' Boolean result, so a failed allocation (memory pressure, oversized viewport at deep zoom) handed out a pixel-less bitmap whose peekPixels() returns null — surfacing much later as a misleading "Could not read bitmap buffer address" inside a render. Observed in production via crash reporting on iOS. Allocation failures now fail fast with a descriptive message, and PdfState contains buffer failures into its existing error state (CancellationException rethrown) instead of letting them escape the render coroutine. New nonAndroidConsumerTest suite covers fail-fast allocation, held- buffer survival on viewport growth, the retire/free/dispose lifecycle, and unfree marking; the existing consumerBufferTest expectation is updated to the new lifecycle. Verified on jvmTest and iosSimulatorArm64Test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Two defects in the viewport buffer path killed consuming apps in the field (Kotlin/Native turns an uncaught worker exception into process death):
ConsumerBufferPool.getBufferViewport disposed the previous viewport buffer inline whenever a larger viewport was requested — with no coordination with an in-flight render still holding it (PdfView renders in a debounced produceState coroutine on Dispatchers.IO, so zoom/rotation restarts it mid-flight) or with the displayed image, which references the buffer's pixels zero-copy via extractSubset. Calling withAddress on the disposed buffer dies natively (SIGABRT in SkBitmap::peekPixels on JVM, SIGSEGV on iOS); the silent variant has the native renderer writing through a dangling pointer. This is likely the root cause of An error is reported when zooming a PDF on the Android side #41 ("recycled bitmap" while zooming on Android — the same race in the Android buffer implementation).
Replaced buffers are now retired instead of disposed and cleaned up on a later request once their consumer freed them. Handed-out viewport buffers are marked unfree; PdfViewport releases the replaced image (buffer-identity guarded, since same-size requests reuse the instance).
ConsumerBufferUtil.allocate ignored allocPixels' Boolean result, so a failed allocation (memory pressure, oversized viewport at deep zoom) handed out a pixel-less bitmap whose peekPixels() returns null — surfacing much later as a misleading "Could not read bitmap buffer address" inside a render. Observed in production via crash reporting on iOS. Allocation failures now fail fast with a descriptive message, and PdfState contains buffer failures into its existing error state (CancellationException rethrown) instead of letting them escape the render coroutine.
New nonAndroidConsumerTest suite covers fail-fast allocation, held- buffer survival on viewport growth, the retire/free/dispose lifecycle, and unfree marking; the existing consumerBufferTest expectation is updated to the new lifecycle. Verified on jvmTest and iosSimulatorArm64Test.