Make starting up a second isolate group deprecated

[gaaclarke]

Previously, it was required in some use cases to create multiple flutter engines that didn't share resources because dart isolate groups didn't exist and platform channels were unable to be used from background isolates. Now that both are possible via FlutterEngineGroup APIs and Isolate Platform Channels; we should deprecate the use of generating a second isolate group. Doing so can result in: unexpected behaviors from Dart (ex #113825), worse performance and there is no known reason why it should be done.

That means that anytime someone creates a new dart isolate group when there is one already in existence it should result in a loud warning in the terminal (since there is currently no way to block it at build time). We should also add a deprecation warning on FlutterEngine's constructor, encouraging usage of FlutterEngineGroup instead. Also, Android and iOS apps should use FlutterEngineGroups by default. Lastly, we should guarantee that users don't accidentally create multiple FlutterEngineGroups. The easiest way to make that guarantee is to convert FlutterEngineGroup to singletons and to deprecate their constructors as well.

In summary, the suggested fixes are:

1. Print out error message when creating second live dart isolate group
2. Deprecate FlutterEngine constructor
3. Start using FlutterEngineGroup by default
4. Deprecate public FlutterEngineGroup constructors
5. Make FlutterEngineGroup singletons

[endless7]

Hi - @gaaclarke, I'm interested in this issue. Can I get started on it?

[gaaclarke]

hi @endless7, let's' wait a bit. This is more of a proposal and I want to get more feedback from the team that we want to do this.

[gaaclarke]

Looks like android_alarm_manager_plus also spawns a background engine in a different isolate group: https://github.com/gaaclarke/plus_plugins/blob/7e898675fbd18cb6ac90f36c1e6bd126aea1c64d/packages/android_alarm_manager_plus/android/src/main/java/dev/fluttercommunity/plus/androidalarmmanager/FlutterBackgroundExecutor.java#L158:L158

[mkustermann]

Make FlutterEngineGroup singletons

There may be add2app scenarios where users want to use different flutter apps (i.e. different AOT-compiled apps) that render different screens. To share common code, such users should ideally compile one big AOT app with all sub-apps inside and use different main entrypoints for the sub-apps. Though it's unclear if users always do that (and/or we make it easy enough to do that).
=> As workaround we could use a global Map<AOT-asset, FlutterEngineGroup> map instead of singleton.

The problem I discovered is that it's really difficult for current creators of flutter engine (groups) to get a reference to an existing group: The reason is that flutter plugins may register with OS to be called for background execution when certain events happen. When that plugin's native code is then invoked it will want to execute dart code, but there's no way for it to know whether there's an existing engine (apart from making FlutterEngine / FlutterEngineGroup have a global registry of active engines). I wonder how such plugins even know which AOT-asset to use when creating an flutter engine (group), anyone knows?

As a side-note: Reading some flutter engine java code it seems that the FlutterEngine / FlutterEngineGroup / ... classes aren't supposed to be used concurrently. The data structures seem to neither be concurrency safe nor explicitly guarded by locks when accessed (see e.g. here). Is this intentional?

[dnfield]

I'm on board with this except for printing out the error message - I think that's very likely to get lost in logs and not understood.

I'm not sure if it would make sense to update the Dart API to allow some kind of opt-in/out if multiple groups are used?