acquireLatestImageAsync hands back a fence it already gave away #376
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: unom/punktfunk#376
Reference in New Issue
Block a user
Every
pf-decodeSIGABRTon the Shield (Android 11, armeabi-v7a) is fdsan catching a double-close of the acquire fence the ASC presenter passes toASurfaceTransaction_setBuffer. Reported as a race between decode/present cycles — it isn't. Nothing concurrent is involved, and our transaction bookkeeping is correct.The fence is not ours to give
AImageReader::acquireLatestImage(AOSPmedia/ndk/NdkImageReader.cpp, verified on theandroid-11.0.0_r48tag and still unfixed on main) drains the reader with a singleint*out-param that it overwrites once per image, then releases each dropped image using whatever that out-param currently holds — the successor's fence — and returns the last value written:So the moment a burst gives it two images to collapse, the caller receives an fd the reader already owns and closed, plus one leaked fd per extra drop. We then hand that stale fd to
setBuffer, which takes ownership and closes it a second time.Why the three crash shapes
The one that settles it aborts before any compositor work:
The
~layer_state_t(18 of 23 crashes) andParcel::freeDataNoInitvariants are the same fd arriving later, once the number has churned — which is also why it looks nondeterministic: it needs ≥2 queued images at drain time, and whether it aborts atsetBufferor downstream depends on who re-grabbed the fd number.Only the latency present intent was affected — the only path that called
acquireLatest. Smooth already drained withacquireNextand was never exposed.The fix
Drain both intents with
acquireNextImageAsync, whose fence is always a freshdup()we exclusively own, and let latency do its own newest-wins — the loop the smoothing FIFO already ran. Superseded candidates drop exactly as before: image back to the pool, its own acquire fence closed. The AOSP fd leak goes away with it, and reader drops now land inskippedinstead of vanishing inside the reader lock.The trap is written up in the module doc, with a matching warning on the vendored
ndkwrapper so the obvious call doesn't get reached for again.Checks
cargo ndk -t arm64-v8a check --libandclippy --libclean;cargo fmt --checkclean; no remaining callers ofacquire_latest_image_async.⚠️ Not verified on the Shield — the confirmation is the AOSP source plus an exact backtrace match. Worth a run on the device before merge.
Every pf-decode SIGABRT on the Shield is fdsan catching a double-close of the acquire fence the ASC presenter passes to ASurfaceTransaction_setBuffer, in three shapes: inside Fence::Fence(int) under setBuffer when the number had already been re-owned ("fd N is owned by unique_fd, was expected to be unowned"), at the end of Transaction::apply when the layer state is torn down, and in Parcel::freeDataNoInit once the number churns. The fence is not ours to give. AImageReader::acquireLatestImage drains with a single int* out-param it overwrites per image, then releases each dropped image with whatever that out-param currently holds — the successor's fence — and returns the last value written. So as soon as a burst gives it two images to collapse, the caller receives an fd the reader has already adopted and closed, plus one leaked fd per extra drop. This is unfixed as of AOSP main, so the newest-wins collapse has to happen on our side. Drain both present intents with acquireNextImageAsync, whose fence is always a fresh dup we exclusively own, and let latency pick the newest itself — the loop the smoothing FIFO already ran. Superseded candidates drop as before: image back to the pool, its own acquire fence closed. Reader drops now show up in `skipped` instead of vanishing inside the reader lock.