Repository navigation
[pigeon] Fix JNI/FFI typed data memory lifetime bugs and update docs - #13061
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates Pigeon to version 29.0.5, modifying the Dart generator to copy typed data arrays into new lists rather than returning backing buffers directly, which addresses potential use-after-free and native memory issues. It also updates the native interop and migration documentation to detail dependency requirements, tool versions, and lazy initialization patterns. Feedback on the changes suggests that the manual reachability check in _getValueFromPigeonTypedData is redundant because NSData implements Finalizable, meaning the check can be removed to avoid unnecessary Objective-C message sends.
| // `bytes` is owned by `data`, which is released by a finalizer once it is | ||
| // unreachable. Using `data` after the copy keeps it alive until the copy | ||
| // above has completed. | ||
| if (data.length != lengthInBytes) { | ||
| throw StateError('PigeonTypedData changed while being read.'); | ||
| } | ||
| return result; |
There was a problem hiding this comment.
In Dart, NSData (which extends NSObject) implements Finalizable. The Dart VM guarantees that any local variable of a type implementing Finalizable is kept alive (not garbage collected) at least until the execution leaves the outermost scope where it is referenced.\n\nBecause of this guarantee, data is automatically kept alive until _getValueFromPigeonTypedData returns, making the manual reachability check (data.length != lengthInBytes) and the associated FFI call redundant. Removing this check avoids an unnecessary Objective-C message send (objc_msgSend) for every typed data read, improving performance.
| // `bytes` is owned by `data`, which is released by a finalizer once it is | |
| // unreachable. Using `data` after the copy keeps it alive until the copy | |
| // above has completed. | |
| if (data.length != lengthInBytes) { | |
| throw StateError('PigeonTypedData changed while being read.'); | |
| } | |
| return result; | |
| return result; |
There was a problem hiding this comment.
This isn't quite true
…er#193641) flutter/packages@0ba9a82...d5ec6db 2026-10-01 faheemabbas766@gmail.com [tool] Enforce README package table order (flutter/packages#12316) 2026-09-30 jessiewong401@gmail.com [various] Allow plugin example apps to build and test on JDK 25 (flutter/packages#13031) 2026-09-30 149176071+m1roxx@users.noreply.github.com [go_router] Expose Navigator clipBehavior on ShellRoute and StatefulShellBranch (flutter/packages#12646) 2026-09-30 stuartmorgan@google.com [google_maps_flutter] Convert unit tests to Kotlin (flutter/packages#13072) 2026-09-30 36861262+QuncCccccc@users.noreply.github.com [material_ui] Migrate M3 ListTile template to use new gen_defaults (flutter/packages#13056) 2026-09-30 15619084+vashworth@users.noreply.github.com Allow tests to use macOS 15.7 or macOS 26.6 (flutter/packages#13007) 2026-09-30 43054281+camsim99@users.noreply.github.com [camera_android_camerax] Fix exposure offset setting error thrown when canceled by a new request (flutter/packages#12582) 2026-09-30 engine-flutter-autoroll@skia.org Roll Flutter from 55b8f88 to d649d2b (27 revisions) (flutter/packages#13080) 2026-09-30 36861262+QuncCccccc@users.noreply.github.com [material_ui] Migrate M3 InputDecorator template to use new gen_defaults (flutter/packages#13024) 2026-09-30 tarrinneal@gmail.com [pigeon] Fix JNI/FFI typed data memory lifetime bugs and update docs (flutter/packages#13061) 2026-09-30 43054281+camsim99@users.noreply.github.com [camera_android_camerax] Correct `pre-push` skill version validation logic (flutter/packages#12371) If this roll has caused a breakage, revert this CL and set the roller to dry run mode using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC flutter-ecosystem@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Fixes flutter/flutter#193465
JNI (Android):
JArray.getRangereturns a typed list backed bymalloc'dmemory whose finalizer runs when the isolate that created it exits. If a
Pigeon call was made in
Isolate.run, the returnedInt64List/Float64List/etc.was freed as soon as the result was sent back, so the receiving isolate read
garbage. The generated codec now copies these into Dart-heap typed lists.
FFI (iOS/macOS): When reading
PigeonTypedData, theNSDatathat owns thebytes could become unreachable (and be released by its finalizer) while
fromListwas still copying from its buffer. The generated code now usesdataafter the copy so it stays alive, and returns empty lists directly for empty
data instead of calling
asTypedListon a possibly-null pointer.The JNI fix adds one copy per array. Removing it would need
package:jnitofill Dart-heap lists directly; that's better done upstream.
Pre-Review Checklist
[shared_preferences]///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2