Skip to content

Fix viewport buffer lifecycle: use-after-dispose race and ignored allocation failure - #48

Open
HaukeRa wants to merge 1 commit into
dshatz:mainfrom
HaukeRa:fix/viewport-buffer-lifecycle
Open

HaukeRa wants to merge 1 commit into
dshatz:mainfrom
HaukeRa:fix/viewport-buffer-lifecycle

Conversation

@HaukeRa

@HaukeRa HaukeRa commented Aug 17, 2026

Copy link
Copy Markdown

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 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).

  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.

…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>
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.

1 participant