Repository navigation
Expose engineId on native plugin bindings and registrars on Android, iOS, and macOS - #195881
auto-submit[bot] merged 4 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request exposes the unique engine identifier from plugin registrars across Android, iOS, macOS, Linux, and Windows platforms. The review feedback suggests adding null checks in the Windows and Linux C APIs to prevent potential crashes, and declaring the engineIdentifier property as nonatomic in the iOS and macOS Objective-C headers to adhere to the style guide.
|
We don't have a dart API to manage state, so we need to do this in the native code. So having a native code API to get the engine id to manage state per engine LGTM. cc @knopp @robert-ancell @loic-sharma who participated in #163476 @loic-sharma are you the right person to review this? |
|
The |
|
@knopp This is for Pigeon targetting FFI and JNI. I don't think the setup and tear-down code has access to Dart in that case. cc @tarrinneal |
|
Why does the plugin API need access to the engine / engineId in the first place? Isn't everything relevant exposed through plugin registrant? |
|
+1 to @knopp's comments. For example, on iOS plugins receive a
I don't fully understand this. Do you have code you could share? I'd imagine this is pure Dart? |
|
The issue isn't how the native plugin accesses engine APIs—it's how a Dart FFI/JNI call knows which native plugin instance to talk to when multiple FlutterEngines exist in the same process. When Engine A and Engine B both start in the same process, Flutter registers two native plugin instances: With MethodChannels, Dart in Engine A sent messages over Engine A's |
|
see #195877 for more detail |
|
If there is a better solution, let me know. I worked through this a while before deciding this was the best way I could figure out. Seems like a minimal change with low risk (though my knowledge here is limited) |
|
autosubmit label was removed for flutter/flutter/195881, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label. |
|
autosubmit label was removed for flutter/flutter/195881, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label. |
flutter/flutter#195881 added a required engineIdentifier property to FlutterBaseRegistrar (iOS) and FlutterPluginRegistrar (macOS), which breaks compilation of the test-only registrar fakes and blocks the Flutter roll (#13569). Implement the property in each fake in a way that also compiles against stable, where the property does not exist yet. Fixes flutter/flutter#196065 ## Pre-Review Checklist **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot 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. [^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.
…e-aware unregistration (#13576) Associates Native Interop (`useJni` / `useFfi`) `@HostApi` and `@FlutterApi` registrations with the Flutter engine that made them. Lookups route to the calling engine's registration when the engine is known, and detaching one engine removes only that engine's registration. Previously, attaching a secondary `FlutterEngine` overwrote any existing registration under the same `name`. Detaching that engine via `register(null)` then cleared the entry for all remaining engines in the process. For example, `shared_preferences_android` failed in the main engine after a background engine detached (flutter/flutter#194920). > [!NOTE] > Per review, the standalone `@FlutterApi` fixes (the `pigeonVar_` prefix for internal `_<Api>Registrar` members so `register` / `dartApi` no longer collide on JNI, `implement` with an empty `name`, and the `objective_c <9.6.3` test pin) are split out into #13609 so they can land on 29.x first. This PR contains only the unregistration and multi-engine changes, which depend on each other. ### Engine identity without deprecated APIs or new stable requirements `register` / `unregister` now take the engine handle that plugins and apps already have. Pigeon derives an engine key from it, so moving to `engineId` once flutter/flutter#195881 reaches `stable` is an internal change, not another breaking change: | Caller | Engine key | |---|---| | Kotlin app: `MyApiRegistrar().register(api, flutterEngine)` | `FlutterEngine.engineId` (public on stable; exact) | | Kotlin plugin: `MyApiRegistrar().register(api, binding)` | `binding.getEngineId()` when the embedding provides a non-zero value (#195881, detected at runtime, survives R8). Otherwise a per-engine key from `binding.binaryMessenger`. | | Swift: `MyApiSetup.register(api:registrar:)` | `registrar.engineIdentifier` when available and non-zero (detected at runtime). Otherwise a per-engine key from the registrar's messenger. | Generated code never calls deprecated embedding APIs such as `FlutterPluginBinding.getFlutterEngine()`. An engine ID of `0` is treated as unknown, because real IDs are never `0`. `0` shows up for registrars whose engine is already deallocating, and for unstubbed test doubles, and must not be shared between engines. Dart lookups (`createWithNativeInteropApi()` / `<HostApi>ForNativeInterop.getInstance()`) choose a registration in this order: 1. The one whose key matches `PlatformDispatcher.instance.engineId`. 2. Otherwise, if the isolate has no engine ID, one from the engine this isolate first resolved a handle from. This is tracked per generated Pigeon file. 3. Otherwise, the most recent registration. Native `@FlutterApi` lookups use the same keys through `<FlutterApi>Registrar` `getInstance(binding | flutterEngine)` in Kotlin and `getInstance(registrar:)` in Swift. **Exact vs. best-effort routing.** Routing is exact whenever native code knows the engine identifier: Kotlin `register(api, flutterEngine)` always, and plugin bindings / registrars on Flutter versions that expose it. Otherwise it is best-effort. It is correct when each engine's Dart resolves its first handle from each Pigeon file at startup, but it can pick another engine's registration when: - Engines are created back-to-back before their Dart starts. - A file's first lookup happens lazily while another engine is attached. - Lookups come from `Isolate.spawn` / `Isolate.run` isolates. macOS plugins receive no detach callback, so macOS registrations currently outlive their engine. The README and guides document these limits and recommend platform channels / `@ProxyApi` for engine-stateful APIs in those cases. Full engine routing remains tracked in flutter/flutter#195877. ### Summary of Changes * **Engine-keyed registration stacks & `unregister`**: Each `name` keeps an ordered stack of `(engine key, instance)` entries. * Re-registering the same instance for the same engine moves it to the top and keeps its existing wrapper. * `unregister(api, engine)` removes only that engine's entry, even out of order. If the engine key can't be resolved, it falls back to matching by instance identity, but only when a single engine holds that instance. * **Detached `@HostApi` safety**: Calls through a handle whose instance was unregistered throw `unregistered-instance` (Kotlin/Swift) instead of crashing. Swift `Setup` wrappers use `resolveApi() throws` instead of `api!`. * **Engine teardown safety (Swift)**: Engine keys are cached per registrar. `unregister` from `detachFromEngine(for:)` therefore works while the engine is deallocating and its messenger is already gone. * **Registry thread safety**: Native Interop calls from background isolates run on worker threads. Registration maps, wrapper API references, and engine-key caches are therefore guarded internally: * Kotlin uses `synchronized` / `@Synchronized` and `@Volatile`. * Swift uses `NSLock` / `NSRecursiveLock`. Removed entries are released after unlocking, so their `deinit` doesn't run while the lock is held. * This doesn't make registration a thread-safe API contract. Registration is still expected on the main thread. * **Dart `@FlutterApi`**: * `<FlutterApi>.setUp(null)` deregisters only the current isolate's native callback. * Registering via `setUp(a)` or the registrar, then calling `setUp(null)`, now correctly removes `a`. * Adds `<FlutterApi>Registrar.unregister`. * Registrations are tagged with the isolate's engine so native lookups reach the right isolate. * **Documentation**: * Updates the README, Native Interop guide, migration guide, migration skill, and `CHANGELOG.md`, including the routing guarantees and limits above. * Notes that `registrar.publish(instance)` is required for iOS to call `detachFromEngine(for:)`. * **Tests**: * Generator unit tests for Dart, Kotlin, and Swift. * Kotlin `NativeInteropRegistrarTest`, iOS `RunnerTests`, and macOS `BasicCompileTest`. These cover: * the same API on two engines * out-of-order unregister * the identity fallback * resolve order * `@FlutterApi` lookup by engine * Real secondary-engine and multi-isolate integration tests. * An integration test asserting that registrations use the exact `PlatformDispatcher.instance.engineId` when the platform exposes it. It runs in minified Android release builds to cover R8. ### Breaking Native Interop API Changes (`30.0.0`) * **Engine-aware registration**: * `@HostApi` `register` / `unregister` take the engine: `MyApiRegistrar().register(api, binding)` / `register(api, flutterEngine)` in Kotlin, and `MyApiSetup.register(api:registrar:)` in Swift. `unregister` follows the same shape. * Native `@FlutterApi` lookup takes the engine: `getInstance(binding | flutterEngine)` in Kotlin and `getInstance(registrar:)` in Swift. The name-only native `getInstance` is removed. * **Explicit deregistration**: * Replaces `register(null)` / `register(api: nil)` / `registerInstance(null | nil)` with `unregister` / `unregisterInstance`, which remove only the given instance for the given engine. * `register` / `registerInstance` require a non-null API and return `void` / `Unit` / `Void`. * **Swift `@HostApi` `AnyObject` constraint**: FFI Swift `@HostApi` protocols are constrained to `AnyObject` so instances can be matched by identity. * **Reserved names**: `pigeonResolveInstance`, `pigeonEngineKey`, and `pigeonEngineKeyOf` are reserved as `@HostApi` method names. Part of flutter/flutter#195877 Part of flutter/flutter#194920 ## Pre-Review Checklist **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot 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. [^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.
Exposes the engine identifier (corresponding to
PlatformDispatcher.instance.engineIdin 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:FlutterPluginBinding.getEngineId()returningflutterEngine.getEngineId().@property(readonly) int64_t engineIdentifiertoFlutterBaseRegistrar(inherited byFlutterPluginRegistrarandFlutterApplicationRegistrar), cached at registrar initialization so it remains valid duringdetachFromEngineForRegistrar:when the weak engine reference is alreadynil.@property(readonly) int64_t engineIdentifiertoFlutterPluginRegistrar, cached at registrar initialization.work towards #195876 (linux and windows will follow in another pr)
Pre-launch Checklist
///).