Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit fac1169

Browse files
jakemac53Commit Queue
authored and
Commit Queue
committed
Track async microtasks scheduled by macros, and ensure they have all completed before a macro returns.
Does not track certain events such as async I/O, but will refuse to execute the scheduled callbacks when those do complete. Bug: #55426 Change-Id: Ie81100e9e4dbe49d050bad875cc9b6a65969863d Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/370120 Reviewed-by: Johnni Winther <johnniwinther@google.com> Reviewed-by: Morgan :) <davidmorgan@google.com> Auto-Submit: Jake Macdonald <jakemac@google.com> Commit-Queue: Johnni Winther <johnniwinther@google.com>
1 parent 549a1b1 commit fac1169

File tree

8 files changed

+350
-51
lines changed

8 files changed

+350
-51
lines changed

‎pkg/_macros/CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
## 0.3.1
2+
3+
- Make it an error for macros to complete with pending async work scheduled.
4+
15
## 0.3.0
26

37
- Remove type parameter on internal `StaticType` implementation.

‎pkg/_macros/lib/src/executor/client.dart‎

Lines changed: 88 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -215,8 +215,8 @@ final class MacroExpansionClient {
215215
remoteInstance: request.introspector,
216216
serializationZoneId: request.serializationZoneId);
217217

218-
MacroExecutionResult result =
219-
awaitexecuteTypesMacro(instance, request.target, introspector);
218+
MacroExecutionResult result =awaitrunPhase(
219+
() =>executeTypesMacro(instance, request.target, introspector));
220220
return SerializableResponse(
221221
responseType: MessageType.macroExecutionResult,
222222
response: result,
@@ -244,8 +244,8 @@ final class MacroExpansionClient {
244244
remoteInstance: request.introspector,
245245
serializationZoneId: request.serializationZoneId);
246246

247-
MacroExecutionResult result = await executeDeclarationsMacro(
248-
instance, request.target, introspector);
247+
MacroExecutionResult result = await runPhase(() =>
248+
executeDeclarationsMacro(instance, request.target, introspector));
249249
return SerializableResponse(
250250
responseType: MessageType.macroExecutionResult,
251251
response: result,
@@ -272,8 +272,8 @@ final class MacroExpansionClient {
272272
remoteInstance: request.introspector,
273273
serializationZoneId: request.serializationZoneId);
274274

275-
MacroExecutionResult result =
276-
awaitexecuteDefinitionMacro(instance, request.target, introspector);
275+
MacroExecutionResult result =awaitrunPhase(
276+
() =>executeDefinitionMacro(instance, request.target, introspector));
277277
return SerializableResponse(
278278
responseType: MessageType.macroExecutionResult,
279279
response: result,
@@ -339,3 +339,85 @@ void Function(Serializer) _sendIOSinkResultFactory(IOSink sink) =>
339339
'ProcessExecutor');
340340
}
341341
};
342+
343+
/// Runs [phase] in a [Zone] which tracks scheduled tasks, completing with a
344+
/// [StateError] if [phase] returns a value while additional tasks or timers
345+
/// are still scheduled.
346+
Future<MacroExecutionResult> runPhase(
347+
Future<MacroExecutionResult> Function() phase) {
348+
final completer = Completer<MacroExecutionResult>();
349+
350+
var pendingMicrotasks = 0;
351+
var activeTimers = 0;
352+
Zone.current
353+
.fork(
354+
specification: ZoneSpecification(
355+
handleUncaughtError: (self, parent, zone, error, stackTrace) {
356+
if (completer.isCompleted) return;
357+
completer.completeError(error, stackTrace);
358+
},
359+
createTimer: (self, parent, zone, duration, f) {
360+
activeTimers++;
361+
return _WrappedTimer(
362+
parent.createTimer(zone, duration, () {
363+
activeTimers--;
364+
f();
365+
}),
366+
onCancel: () => activeTimers--);
367+
},
368+
createPeriodicTimer: (self, parent, zone, duration, f) {
369+
activeTimers++;
370+
return _WrappedTimer(parent.createPeriodicTimer(zone, duration, f),
371+
onCancel: () => activeTimers--);
372+
},
373+
scheduleMicrotask: (self, parent, zone, f) {
374+
pendingMicrotasks++;
375+
parent.scheduleMicrotask(zone, () {
376+
pendingMicrotasks--;
377+
assert(pendingMicrotasks >= 0);
378+
// This should only happen if we have previously competed with an
379+
// error. Just skip this scheduled task in that case.
380+
if (completer.isCompleted) return;
381+
f();
382+
});
383+
},
384+
))
385+
.runGuarded(() => phase().then((value) {
386+
if (completer.isCompleted) return;
387+
if (pendingMicrotasks != 0) {
388+
throw StateError(
389+
'Macro completed but has $pendingMicrotasks async tasks still '
390+
'pending. Macros must complete all async work prior to '
391+
'returning.');
392+
}
393+
if (activeTimers != 0) {
394+
throw StateError(
395+
'Macro completed but has $activeTimers active timers. '
396+
'Macros must cancel all timers prior to returning.');
397+
}
398+
completer.complete(value);
399+
}));
400+
return completer.future;
401+
}
402+
403+
/// Wraps a [Timer] to track when it is cancelled and calls [onCancel], if the
404+
/// timer is still active.
405+
class _WrappedTimer implements Timer {
406+
final Timer timer;
407+
408+
final void Function() onCancel;
409+
410+
_WrappedTimer(this.timer, {required this.onCancel});
411+
412+
@override
413+
void cancel() {
414+
if (isActive) onCancel();
415+
timer.cancel();
416+
}
417+
418+
@override
419+
bool get isActive => timer.isActive;
420+
421+
@override
422+
int get tick => timer.tick;
423+
}

‎pkg/_macros/pubspec.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
name: _macros
2-
version: 0.3.0
2+
version: 0.3.1
33
description: >-
44
This is a private SDK vendored package, which is re-exported by the public
55
`macros` package, which is a pub package. Every change to this package is

‎pkg/_macros/test/executor/executor_test.dart‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,14 @@ import '../util.dart';
2121
void main() {
2222
late MacroExecutor executor;
2323
late File kernelOutputFile;
24+
final danglingTaskMacroName = 'DanglingTaskMacro';
25+
final danglingTimerMacroName = 'DanglingTimerMacro';
26+
final danglingPeriodicTimerMacroName = 'DanglingPeriodicTimerMacro';
2427
final diagnosticMacroName = 'DiagnosticMacro';
2528
final simpleMacroName = 'SimpleMacro';
29+
late MacroInstanceIdentifier danglingTaskMacroId;
30+
late MacroInstanceIdentifier danglingTimerMacroId;
31+
late MacroInstanceIdentifier danglingPeriodicTimerMacroId;
2632
late MacroInstanceIdentifier diagnosticMacroInstanceId;
2733
late MacroInstanceIdentifier simpleMacroInstanceId;
2834
late Uri macroUri;
@@ -46,6 +52,9 @@ void main() {
4652
var bootstrapContent = bootstrapMacroIsolate({
4753
macroUri.toString(): {
4854
simpleMacroName: ['', 'named'],
55+
danglingTaskMacroName: [''],
56+
danglingTimerMacroName: [''],
57+
danglingPeriodicTimerMacroName: [''],
4958
diagnosticMacroName: [''],
5059
}
5160
}, mode);
@@ -129,6 +138,21 @@ void main() {
129138
expect(simpleMacroInstanceId, isNotNull,
130139
reason: 'Can create an instance with named arguments.');
131140

141+
danglingTaskMacroId = await executor.instantiateMacro(
142+
macroUri, danglingTaskMacroName, '', Arguments([], {}));
143+
expect(danglingTaskMacroId, isNotNull);
144+
145+
danglingTimerMacroId = await executor.instantiateMacro(
146+
macroUri, danglingTimerMacroName, '', Arguments([], {}));
147+
expect(danglingTimerMacroId, isNotNull);
148+
149+
danglingPeriodicTimerMacroId = await executor.instantiateMacro(
150+
macroUri,
151+
danglingPeriodicTimerMacroName,
152+
'',
153+
Arguments([], {}));
154+
expect(danglingPeriodicTimerMacroId, isNotNull);
155+
132156
diagnosticMacroInstanceId = await executor.instantiateMacro(
133157
macroUri, diagnosticMacroName, '', Arguments([], {}));
134158
expect(diagnosticMacroInstanceId, isNotNull);
@@ -401,6 +425,36 @@ class LibraryInfo {
401425
'''),
402426
);
403427
});
428+
429+
test('Cannot leave dangling tasks', () async {
430+
expect(
431+
() => executor.executeTypesPhase(danglingTaskMacroId,
432+
Fixtures.library, TestTypePhaseIntrospector()),
433+
throwsA(
434+
isA<UnexpectedMacroException>().having((e) => e.message,
435+
'message', contains('1 async tasks still pending')),
436+
));
437+
});
438+
439+
test('Cannot leave dangling timers', () async {
440+
expect(
441+
() => executor.executeTypesPhase(danglingTimerMacroId,
442+
Fixtures.library, TestTypePhaseIntrospector()),
443+
throwsA(
444+
isA<UnexpectedMacroException>().having((e) => e.message,
445+
'message', contains('1 active timers')),
446+
));
447+
448+
expect(
449+
() => executor.executeTypesPhase(
450+
danglingPeriodicTimerMacroId,
451+
Fixtures.library,
452+
TestTypePhaseIntrospector()),
453+
throwsA(
454+
isA<UnexpectedMacroException>().having((e) => e.message,
455+
'message', contains('1 active timers')),
456+
));
457+
});
404458
});
405459

406460
group('in the declaration phase', () {
@@ -666,6 +720,40 @@ class LibraryInfo {
666720
equalsIgnoringWhitespace('final LibraryInfo library;'),
667721
);
668722
});
723+
724+
test('Cannot leave dangling tasks', () async {
725+
expect(
726+
() => executor.executeDeclarationsPhase(
727+
danglingTaskMacroId,
728+
Fixtures.library,
729+
Fixtures.testDeclarationPhaseIntrospector),
730+
throwsA(
731+
isA<UnexpectedMacroException>().having((e) => e.message,
732+
'message', contains('1 async tasks still pending')),
733+
));
734+
});
735+
736+
test('Cannot leave dangling timers', () async {
737+
expect(
738+
() => executor.executeDeclarationsPhase(
739+
danglingTimerMacroId,
740+
Fixtures.library,
741+
Fixtures.testDeclarationPhaseIntrospector),
742+
throwsA(
743+
isA<UnexpectedMacroException>().having((e) => e.message,
744+
'message', contains('1 active timers')),
745+
));
746+
747+
expect(
748+
() => executor.executeDeclarationsPhase(
749+
danglingPeriodicTimerMacroId,
750+
Fixtures.library,
751+
Fixtures.testDeclarationPhaseIntrospector),
752+
throwsA(
753+
isA<UnexpectedMacroException>().having((e) => e.message,
754+
'message', contains('1 active timers')),
755+
));
756+
});
669757
});
670758

671759
group('in the definition phase', () {
@@ -961,6 +1049,40 @@ class LibraryInfo {
9611049
augment final LibraryInfo library = LibraryInfo(Uri.parse('package:foo/bar.dart'), '3.0', [MyClass, MyEnum, MyMixin, ]);
9621050
'''));
9631051
});
1052+
1053+
test('Cannot leave dangling tasks', () async {
1054+
expect(
1055+
() => executor.executeDefinitionsPhase(
1056+
danglingTaskMacroId,
1057+
Fixtures.library,
1058+
Fixtures.testDefinitionPhaseIntrospector),
1059+
throwsA(
1060+
isA<UnexpectedMacroException>().having((e) => e.message,
1061+
'message', contains('1 async tasks still pending')),
1062+
));
1063+
});
1064+
1065+
test('Cannot leave dangling timers', () async {
1066+
expect(
1067+
() => executor.executeDefinitionsPhase(
1068+
danglingTimerMacroId,
1069+
Fixtures.library,
1070+
Fixtures.testDefinitionPhaseIntrospector),
1071+
throwsA(
1072+
isA<UnexpectedMacroException>().having((e) => e.message,
1073+
'message', contains('1 active timers')),
1074+
));
1075+
1076+
expect(
1077+
() => executor.executeDefinitionsPhase(
1078+
danglingPeriodicTimerMacroId,
1079+
Fixtures.library,
1080+
Fixtures.testDefinitionPhaseIntrospector),
1081+
throwsA(
1082+
isA<UnexpectedMacroException>().having((e) => e.message,
1083+
'message', contains('1 active timers')),
1084+
));
1085+
});
9641086
});
9651087

9661088
test('and report diagnostics', () async {

‎pkg/_macros/test/executor/simple_macro.dart‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -802,6 +802,75 @@ Future<FunctionBodyCode> _buildFunctionAugmentation(
802802
]);
803803
}
804804

805+
class DanglingTaskMacro
806+
implements
807+
LibraryTypesMacro,
808+
LibraryDeclarationsMacro,
809+
LibraryDefinitionMacro {
810+
@override
811+
void buildTypesForLibrary(Library library, TypeBuilder builder) {
812+
Future.value('hello').then((_) => Future.value('world'));
813+
}
814+
815+
@override
816+
void buildDeclarationsForLibrary(
817+
Library library, DeclarationBuilder builder) {
818+
Future.value('hello').then((_) => Future.value('world'));
819+
}
820+
821+
@override
822+
FutureOr<void> buildDefinitionForLibrary(
823+
Library library, LibraryDefinitionBuilder builder) {
824+
Future.value('hello').then((_) => Future.value('world'));
825+
}
826+
}
827+
828+
class DanglingTimerMacro
829+
implements
830+
LibraryTypesMacro,
831+
LibraryDeclarationsMacro,
832+
LibraryDefinitionMacro {
833+
@override
834+
void buildTypesForLibrary(Library library, TypeBuilder builder) {
835+
Timer.run(() {});
836+
}
837+
838+
@override
839+
void buildDeclarationsForLibrary(
840+
Library library, DeclarationBuilder builder) {
841+
Timer.run(() {});
842+
}
843+
844+
@override
845+
FutureOr<void> buildDefinitionForLibrary(
846+
Library library, LibraryDefinitionBuilder builder) {
847+
Timer.run(() {});
848+
}
849+
}
850+
851+
class DanglingPeriodicTimerMacro
852+
implements
853+
LibraryTypesMacro,
854+
LibraryDeclarationsMacro,
855+
LibraryDefinitionMacro {
856+
@override
857+
void buildTypesForLibrary(Library library, TypeBuilder builder) {
858+
Timer.periodic(Duration(seconds: 1), (_) {});
859+
}
860+
861+
@override
862+
void buildDeclarationsForLibrary(
863+
Library library, DeclarationBuilder builder) {
864+
Timer.periodic(Duration(seconds: 1), (_) {});
865+
}
866+
867+
@override
868+
FutureOr<void> buildDefinitionForLibrary(
869+
Library library, LibraryDefinitionBuilder builder) {
870+
Timer.periodic(Duration(seconds: 1), (_) {});
871+
}
872+
}
873+
805874
extension on String {
806875
String capitalize() => '${this[0].toUpperCase()}${substring(1)}';
807876
}

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /