Repository navigation
[beta] Expose engineId on native plugin bindings and registrars on Android, iOS, and macOS - #196145
Conversation
…iOS, and macOS (flutter#195881) Exposes the engine identifier (corresponding to `PlatformDispatcher.instance.engineId` in Dart) directly on native plugin bindings and registrars across Android, iOS, and macOS so that FFI/JNI plugins can associate and clean up per-engine native state during plugin registration and detachment without access to the underlying engine instance: - **Android**: Adds `FlutterPluginBinding.getEngineId()` returning `flutterEngine.getEngineId()`. - **iOS**: Adds `@property(readonly) int64_t engineIdentifier` to `FlutterBaseRegistrar` (inherited by `FlutterPluginRegistrar` and `FlutterApplicationRegistrar`), cached at registrar initialization so it remains valid during `detachFromEngineForRegistrar:` when the weak engine reference is already `nil`. - **macOS**: Adds `@property(readonly) int64_t engineIdentifier` to `FlutterPluginRegistrar`, cached at registrar initialization. work towards flutter#195876 (linux and windows will follow in another pr) ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant in-code documentation (doc comments with `///`). - [x] If this PR introduces a new feature or capability, I created and linked a website documentation issue or PR in [flutter/website] (or verified none is needed). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/CONTRIBUTING.ai.md [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [flutter/website]: https://github.com/flutter/website [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
|
This pull request was opened from and to a release candidate branch. This should only be done as part of the official Flutter release process. If you are attempting to make a regular contribution to the Flutter project, please close this PR and follow the instructions at Tree Hygiene for detailed instructions on contributing to Flutter. Reviewers: Use caution before merging pull requests to release branches. Ensure the proper procedure has been followed. |
There was a problem hiding this comment.
Code Review
This pull request exposes the unique engine identifier (engineId / engineIdentifier) on the plugin registrar and binding interfaces across Android, iOS, and macOS platforms, allowing plugins to identify their associated FlutterEngine. It also adds corresponding unit tests for each platform to verify this functionality. I have no feedback to provide.
|
Landing with the hope that this can be part of the 3.50 stable release. |
371f772
into
flutter:flutter-3.50-candidate.0
Issue Link:
#195876
Impact Description:
Native plugins using JNI/FFI (such as plugins generated with Pigeon's native interop support) register directly in native memory rather than on a per-engine
BinaryMessenger, and have no non-deprecated public API onFlutterPluginBinding(Android) orFlutterPluginRegistrar(iOS/macOS) to obtain the engine identifier during plugin attachment and detachment. In apps running multipleFlutterEngineinstances in the same process (such as background engines alongside the main UI engine), one engine attaching or detaching can overwrite or unregister another engine's JNI/FFI host API instance (#194920). This blocks shipping multi-engine-safe FFI/JNI plugins (flutter/packages#13359) against the 3.50 stable release.Scope of the API break: This supersedes #195991, which was closed as a breaking change. As @stuartmorgan noted, the
@requiredengineIdentifierproperty onFlutterBaseRegistrar(iOS) andFlutterPluginRegistrar(macOS) only affects hand-written native unit-test fakes that conform to those protocols directly. Production plugin and app code only consumes registrars provided by the engine and is unaffected. Following that, @zanderso commented that if the change is safer than the earlier signals indicated, it should be considered for a hotfix release in this cycle. Affected test fakes can addvar engineIdentifier: Int64 = 0(Swift) or@synthesize engineIdentifier;(Objective-C), which compiles both before and after this change. First-party fakes were already updated in flutter/packages#13574. On Android,FlutterPluginBindingis a concrete class, so addinggetEngineId()does not affect mocks.Changelog Description:
[flutter/195876] When running multiple
FlutterEngines on Android, iOS, or macOS, FFI/JNI plugins could not determine which engine aFlutterPluginBindingorFlutterPluginRegistrarbelonged to.Workaround:
On Android, plugins can call the deprecated
FlutterPluginBinding.getFlutterEngine().getEngineId(). On iOS and macOS, there is no public API workaround (engineIdentifieris only declared in internal headers, and the registrar's weak engine reference is alreadynilduringdetachFromEngineForRegistrar:).Risk:
What is the risk level of this cherry-pick?
Test Coverage:
Are you confident that your fix is well-tested by automated tests?
Validation Steps:
FlutterPluginBinding.getEngineId()returns the associatedFlutterEngine.getEngineId().registrar.engineIdentifieronFlutterPluginRegistrarandFlutterApplicationRegistrarmatches the owningFlutterEngine.engineIdentifier, both during registration and duringdetachFromEngineForRegistrar:.registrar.engineIdentifieronFlutterPluginRegistrarmatches the owningFlutterEngine.engineIdentifier.