Skip to content

Commit 8636810

Browse files
committed
feat: Add root-isolate predicate and pin secondary-isolate plugin behaviour (#5302)
Groundwork for the #5302 category guardrails. No behaviour change: the only production change is a new `@internal` getter, and nothing consumes it yet. - `amplifyIsInRootIsolate` in amplify_flutter: true for an app's main isolate and for a headless `FlutterEngine` entry point, false only inside `Isolate.spawn`. Built on `ServicesBinding.rootIsolateToken`, so it cannot false-positive on the root isolate. - Pins today's behaviour: a native-backed plugin added from a secondary isolate is accepted *silently*, while a pure-Dart plugin correctly still works, and the root isolate keeps tolerating an already-configured native SDK. The guard itself is deliberately not included. `AmplifyCategory.addPlugin` (amplify_core/lib/src/category/amplify_categories.dart:91-101) already catches `AmplifyAlreadyConfiguredException` and registers the plugin anyway, so the exception never reaches `amplify_flutter`. A guard in `hybrid_impl.dart` was written, proven dead by test, and reverted rather than shipped.
1 parent dd1b8fd commit 8636810

2 files changed

Lines changed: 193 additions & 0 deletions

File tree

packages/amplify/amplify_flutter/lib/src/amplify_isolate.dart

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,28 @@ RootIsolateToken? get amplifyRootIsolateToken {
2222
return RootIsolateToken.instance;
2323
}
2424

25+
/// {@template amplify_flutter.amplify_is_in_root_isolate}
26+
/// Whether the current isolate is a root isolate, meaning it owns a Flutter
27+
/// engine and therefore drives the process-wide native Amplify SDKs.
28+
///
29+
/// True on an app's main isolate, and also on the root isolate of a headless
30+
/// `FlutterEngine` — for example the entry point that records a push
31+
/// notification while the app is killed. Both of those own an engine, so both
32+
/// may configure native Amplify.
33+
///
34+
/// False only in an isolate started with `Isolate.spawn`, which has no engine
35+
/// of its own. Such an isolate can still use categories that run entirely in
36+
/// Dart, but the native SDKs already belong to the root isolate.
37+
///
38+
/// Always true on the web, which has no isolates.
39+
/// {@endtemplate}
40+
@internal
41+
bool get amplifyIsInRootIsolate {
42+
// `RootIsolateToken.instance` throws on the web rather than returning null.
43+
if (kIsWeb) return true;
44+
return ServicesBinding.rootIsolateToken != null;
45+
}
46+
2547
/// {@template amplify_flutter.amplify_isolate_is_initialized}
2648
/// Whether the current isolate can reach Amplify's platform channels.
2749
///
Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
// Groundwork and ground truth for the #5302 category guardrails.
5+
//
6+
// It establishes `amplifyIsInRootIsolate` — the predicate a guard has to be
7+
// built on — and pins what Amplify does *today* when a native-backed plugin is
8+
// added from a secondary isolate. The pinned behaviour is the bug: the plugin is
9+
// accepted silently. When the guard lands, the `silently accepts` test below is
10+
// expected to invert, which is the point of pinning it now.
11+
12+
@TestOn('vm')
13+
library;
14+
15+
import 'dart:async';
16+
import 'dart:isolate';
17+
18+
import 'package:amplify_core/amplify_core.dart';
19+
import 'package:amplify_flutter/src/amplify_isolate.dart';
20+
import 'package:amplify_flutter/src/hybrid_impl.dart';
21+
import 'package:flutter_test/flutter_test.dart';
22+
23+
/// Stands in for a native-backed plugin whose native side is already configured
24+
/// by another isolate.
25+
///
26+
/// amplify-android reports exactly this: an `AlreadyConfiguredException` mapped
27+
/// to `AmplifyAlreadyConfiguredException`, verified on device in #7266.
28+
class NativeAlreadyConfiguredPlugin extends AnalyticsPluginInterface {
29+
// Deliberately does not call super: it models native registration failing
30+
// outright rather than completing.
31+
@override
32+
// ignore: must_call_super
33+
Future<void> addPlugin({
34+
required AmplifyAuthProviderRepository authProviderRepo,
35+
}) async {
36+
throw const AmplifyAlreadyConfiguredException(
37+
'Amplify has already been configured.',
38+
);
39+
}
40+
}
41+
42+
/// A plugin that runs entirely in Dart, which #7266 proved must keep working in
43+
/// a secondary isolate.
44+
class PureDartPlugin extends AnalyticsPluginInterface {
45+
@override
46+
Future<void> configure({
47+
AmplifyOutputs? config,
48+
required AmplifyAuthProviderRepository authProviderRepo,
49+
}) async {}
50+
}
51+
52+
/// What [addPluginInIsolate] observed, sent back over a [SendPort].
53+
typedef IsolateReport = Map<String, Object?>;
54+
55+
/// Builds a fresh [AmplifyHybridImpl] in the isolate this runs in and adds
56+
/// either the native-backed or the pure-Dart plugin to it.
57+
Future<void> addPluginInIsolate((SendPort, bool) message) async {
58+
final (sendPort, useNativePlugin) = message;
59+
final report = <String, Object?>{'inRootIsolate': amplifyIsInRootIsolate};
60+
final amplify = AmplifyHybridImpl();
61+
try {
62+
await amplify.addPlugin(
63+
useNativePlugin ? NativeAlreadyConfiguredPlugin() : PureDartPlugin(),
64+
);
65+
report['errorType'] = null;
66+
} on Object catch (e) {
67+
report['errorType'] = e.runtimeType.toString();
68+
report['error'] = '$e';
69+
}
70+
report['pluginCount'] = amplify.Analytics.plugins.length;
71+
sendPort.send(report);
72+
}
73+
74+
/// Spawns [addPluginInIsolate] and waits for its [IsolateReport].
75+
Future<IsolateReport> runInIsolate({required bool useNativePlugin}) async {
76+
final receivePort = ReceivePort();
77+
final errorPort = ReceivePort();
78+
final result = Completer<IsolateReport>();
79+
80+
receivePort.listen((message) {
81+
if (!result.isCompleted) {
82+
result.complete((message as Map).cast<String, Object?>());
83+
}
84+
});
85+
errorPort.listen((message) {
86+
if (!result.isCompleted) {
87+
result.completeError(StateError('Isolate errored: $message'));
88+
}
89+
});
90+
91+
final isolate = await Isolate.spawn(
92+
addPluginInIsolate,
93+
(receivePort.sendPort, useNativePlugin),
94+
onError: errorPort.sendPort,
95+
errorsAreFatal: true,
96+
debugName: 'guardrail-probe',
97+
);
98+
try {
99+
return await result.future.timeout(const Duration(seconds: 30));
100+
} finally {
101+
isolate.kill(priority: Isolate.immediate);
102+
receivePort.close();
103+
errorPort.close();
104+
}
105+
}
106+
107+
void main() {
108+
TestWidgetsFlutterBinding.ensureInitialized();
109+
110+
group('amplifyIsInRootIsolate', () {
111+
test('is true on a root isolate', () {
112+
// `flutter_tester` gives a real engine root isolate, which is the same
113+
// state a headless `FlutterEngine` entry point runs in. This is what keeps
114+
// `amplifyBackgroundProcessing` out of any guard built on this predicate:
115+
// that function calls `WidgetsFlutterBinding.ensureInitialized()` and a
116+
// plain `MethodChannel`, so it can only ever run with an engine attached.
117+
expect(amplifyIsInRootIsolate, isTrue);
118+
});
119+
120+
test('is false in a spawned isolate', () async {
121+
final report = await runInIsolate(useNativePlugin: false);
122+
123+
expect(report['inRootIsolate'], isFalse);
124+
});
125+
});
126+
127+
group('adding plugins from a secondary isolate', () {
128+
test('works for a plugin that runs entirely in Dart', () async {
129+
final report = await runInIsolate(useNativePlugin: false);
130+
131+
expect(report['errorType'], isNull);
132+
expect(report['pluginCount'], 1);
133+
});
134+
135+
// CURRENT BEHAVIOUR, and the bug #5302 needs fixed. `AmplifyCategory
136+
// .addPlugin` (packages/amplify_core/lib/src/category/amplify_categories
137+
// .dart:91-101) catches `AmplifyAlreadyConfiguredException` and adds the
138+
// plugin anyway, so the caller sees success while native never registered
139+
// this isolate's plugin. Nothing in `amplify_flutter` observes the exception,
140+
// because the category swallowed it first.
141+
test('silently accepts a native-backed plugin', () async {
142+
final report = await runInIsolate(useNativePlugin: true);
143+
144+
expect(report['inRootIsolate'], isFalse);
145+
expect(
146+
report['errorType'],
147+
isNull,
148+
reason: 'If this now throws, the guard has landed — invert this test',
149+
);
150+
expect(
151+
report['pluginCount'],
152+
1,
153+
reason: 'Plugin is registered on the Dart side despite native failing',
154+
);
155+
});
156+
});
157+
158+
group('adding plugins on the root isolate', () {
159+
test('still tolerates an already-configured native SDK', () async {
160+
// The app-restart / hot-restart path: native outlives the Dart isolate, so
161+
// re-registering must stay a no-op. Any guard must not change this.
162+
final amplify = AmplifyHybridImpl();
163+
164+
await expectLater(
165+
amplify.addPlugin(NativeAlreadyConfiguredPlugin()),
166+
completes,
167+
);
168+
expect(amplify.Analytics.plugins, hasLength(1));
169+
});
170+
});
171+
}

0 commit comments

Comments
 (0)