Skip to content

Commit cd20b29

Browse files
MarkzipanCommit Queue
authored andcommitted
[reload_test] Resolving d8 timer problems in the reload suite.
d8 tests weren't executing code after a hot restart due to several factors: 1) subsequent calls to `main` after a hot restart weren't being added to an event loop. 2) periodic timers, which use `setInterval` weren't updated to check for the hot restart generation. 3) d8's simulated timers run synchronously, which interacts poorly with our async implementation. Periodic timers never cede to the async task that handles changing hot restart generation, so they would run forever whenever an error was thrown. Changes: * Added a helper to d8.js that cancels all timers. * d8 now cancels all timers if an async main registers an error (via a handler on main). * `setInterval` is now implemented. * The embedder now accepts a publicly modifiable config object. `capturedMainHandler` and `mainErrorCallback` can be set via this object. Change-Id: I523752ea69e8fd1f1ec0f6f585a484b670534cfc Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/421680 Commit-Queue: Mark Zhou <[email protected]> Reviewed-by: Srujan Gaddam <[email protected]> Reviewed-by: Nicholas Shahan <[email protected]>
1 parent 91294c1 commit cd20b29

File tree

4 files changed

+155
-16
lines changed

4 files changed

+155
-16
lines changed

pkg/dev_compiler/lib/js/ddc/ddc_module_loader.js

Lines changed: 63 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1520,15 +1520,46 @@ if (!self.deferred_loader) {
15201520
this.initializeAndLinkLibrary('dart:web_gl');
15211521
}
15221522

1523+
// Runs the 'main' method on `entryPointLibrary` while attaching
1524+
// `capturedMainHandler` and `mainErrorCallback`.
1525+
_runMain(entryPointLibrary, args = []) {
1526+
// TODO(35113): Provide the ability to pass arguments in a type safe way.
1527+
let runMainAndHandleErrors = () => {
1528+
try {
1529+
let mainValue = entryPointLibrary.main(args);
1530+
// Attach the error callback to main's future if it's async.
1531+
if (dartDevEmbedderConfig.mainErrorCallback != null && mainValue != null &&
1532+
mainValue.catchError != null) {
1533+
mainValue.catchError((e) => {
1534+
dartDevEmbedderConfig.mainErrorCallback();
1535+
throw e;
1536+
});
1537+
}
1538+
} catch (e) {
1539+
// Invoke the error callback if main is sync. This doesn't conflict
1540+
// with the rethrow in the async path since DDC catches async errors
1541+
// in a separate code path.
1542+
if (dartDevEmbedderConfig.mainErrorCallback != null) {
1543+
dartDevEmbedderConfig.mainErrorCallback();
1544+
}
1545+
throw e;
1546+
}
1547+
};
1548+
if (dartDevEmbedderConfig.capturedMainHandler) {
1549+
dartDevEmbedderConfig.capturedMainHandler(runMainAndHandleErrors);
1550+
} else {
1551+
runMainAndHandleErrors();
1552+
}
1553+
}
1554+
15231555
// See docs on `DartDevEmbedder.runMain`.
1524-
runMain(entryPointLibraryName, dartSdkRuntimeOptions) {
1556+
runMain(entryPointLibraryName, dartSdkRuntimeOptions, capturedMainHandler, mainErrorCallback) {
15251557
this.setDartSDKRuntimeOptions(dartSdkRuntimeOptions);
15261558
console.log('Starting application from main method in: ' + entryPointLibraryName + '.');
15271559
let entryPointLibrary = this.initializeAndLinkLibrary(entryPointLibraryName);
15281560
this.savedEntryPointLibraryName = entryPointLibraryName;
15291561
this.savedDartSdkRuntimeOptions = dartSdkRuntimeOptions;
1530-
// TODO(35113): Provide the ability to pass arguments in a type safe way.
1531-
entryPointLibrary.main([]);
1562+
this._runMain(entryPointLibrary);
15321563
}
15331564

15341565
setDartSDKRuntimeOptions(options) {
@@ -1647,8 +1678,7 @@ if (!self.deferred_loader) {
16471678
console.log('Hot restarting application from main method in: ' +
16481679
this.savedEntryPointLibraryName + ' (generation: ' +
16491680
this.hotRestartGeneration + ').');
1650-
// TODO(35113): Provide the ability to pass arguments in a type safe way.
1651-
entryPointLibrary.main([]);
1681+
this._runMain(entryPointLibrary);
16521682
}
16531683
}
16541684

@@ -1967,10 +1997,38 @@ if (!self.deferred_loader) {
19671997

19681998
const debugger_ = new Debugger();
19691999

2000+
/** Holds public configurations for the `DartDevEmbedder`.
2001+
* These properties may be modified during the runtime of the app.
2002+
*/
2003+
class DartDevEmbedderConfiguration {
2004+
/*
2005+
* An optional handler that acts as a wrapper around the invocation of the
2006+
* Dart program's 'main' method. Passed an opaque function as an argument
2007+
* that invokes 'main' when called.
2008+
* @type {?function(function())}
2009+
*/
2010+
capturedMainHandler = null;
2011+
2012+
/*
2013+
* An optional callback that is invoked when 'main' throws.
2014+
* @type {?function()}
2015+
*/
2016+
mainErrorCallback = null;
2017+
}
2018+
2019+
const dartDevEmbedderConfig = new DartDevEmbedderConfiguration();
2020+
19702021
/** The API for embedding a Dart application in the page at development time
19712022
* that supports stateful hot reloading.
19722023
*/
19732024
class DartDevEmbedder {
2025+
/**
2026+
* Expose the DartDevEmbedderConfig publicly.
2027+
*/
2028+
get config() {
2029+
return dartDevEmbedderConfig;
2030+
}
2031+
19742032
/**
19752033
* Runs the Dart main method.
19762034
*

pkg/reload_test/lib/ddc_helpers.dart

Lines changed: 49 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -173,8 +173,8 @@ self.\$injectedFilesAndLibrariesToReload = function(fileGeneration) {
173173
return [fileUrls, libraryIds];
174174
}
175175
176-
// D8 does not support the core Timer API methods beside `setTimeout` so our
177-
// D8 preambles provide a custom implementation.
176+
// D8 does not support the core Timer API methods, so our D8 preamble provides
177+
// a custom implementation.
178178
//
179179
// Timers in this implementation are simulated, so they all complete before
180180
// native JS `await` boundaries. If this boundary occurs before our runtime's
@@ -183,17 +183,37 @@ self.\$injectedFilesAndLibrariesToReload = function(fileGeneration) {
183183
//
184184
// To resolve this, we record and increment hot restart generations early
185185
// and wrap timer functions with custom cancellation logic.
186+
self.setInterval = function(setInterval) {
187+
return function(f, ms) {
188+
var timerId;
189+
let currentHotRestartIteration =
190+
self.\$dartLoader.loader.intendedHotRestartGeneration;
191+
var internalCallback = function() {
192+
if (currentHotRestartIteration ==
193+
self.\$dartLoader.loader.intendedHotRestartGeneration) {
194+
f();
195+
} else {
196+
self.clearInterval(timerId);
197+
}
198+
}
199+
timerId = setInterval(internalCallback, ms);
200+
};
201+
}(self.setInterval);
202+
186203
self.setTimeout = function(setTimeout) {
187-
let currentHotRestartIteration =
188-
self.\$dartLoader.loader.intendedHotRestartGeneration;
189204
return function(f, ms) {
205+
var timerId;
206+
let currentHotRestartIteration =
207+
self.\$dartLoader.loader.intendedHotRestartGeneration;
190208
var internalCallback = function() {
191209
if (currentHotRestartIteration ==
192-
self.\$dartLoader.loader.intendedHotRestartGeneration) {
210+
self.\$dartLoader.loader.intendedHotRestartGeneration) {
193211
f();
212+
} else {
213+
self.clearTimeout(timerId);
194214
}
195215
}
196-
setTimeout(internalCallback, ms);
216+
timerId = setTimeout(internalCallback, ms);
197217
};
198218
}(self.setTimeout);
199219
@@ -202,15 +222,33 @@ self.setTimeout = function(setTimeout) {
202222
// by deleting the our custom implementation in D8's preamble.
203223
self.scheduleImmediate = void 0;
204224
225+
// Set embedder configurations.
226+
227+
// Invoke main through the d8 preamble to ensure the code is running within the
228+
// fake event loop.
229+
dartDevEmbedder.config.capturedMainHandler =
230+
dartDevEmbedder.config.capturedHotRestartMainHandler =
231+
(runMain) => {
232+
self.dartMainRunner((_) => {
233+
runMain();
234+
}, []);
235+
};
236+
237+
238+
// D8 timers execute synchronously, so periodic timers must be explicitly
239+
// cancelled when an async main reaches an error state.
240+
dartDevEmbedder.config.mainErrorCallback =
241+
dartDevEmbedder.config.hotRestartMainErrorCallback =
242+
() => {
243+
self.clearAllTimers();
244+
}
245+
205246
// Begin loading libraries
206247
loader.nextAttempt();
207248
208-
// Invoke main through the d8 preamble to ensure the code is running
209-
// within the fake event loop.
210-
self.dartMainRunner(function () {
211-
dartDevEmbedder.runMain("$entrypointModuleName", {});
212-
});
249+
dartDevEmbedder.runMain("$entrypointModuleName", {});
213250
''';
251+
214252
return d8BootstrapJS;
215253
}
216254

sdk/lib/_internal/js_dev_runtime/private/preambles/d8.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,13 @@ if (typeof global != "undefined") self = global; // Node.js.
262262
delete timerIds[id];
263263
}
264264

265+
function cancelAllTimers() {
266+
for (const id in timerIds) {
267+
timerIds[id].$timerId = undefined;
268+
}
269+
timerIds = {};
270+
}
271+
265272
function eventLoop(action) {
266273
while (action) {
267274
try {
@@ -290,6 +297,7 @@ if (typeof global != "undefined") self = global; // Node.js.
290297
self.setInterval = addInterval;
291298
self.clearInterval = cancelTimer;
292299
self.scheduleImmediate = addTask;
300+
self.clearAllTimers = cancelAllTimers;
293301

294302
// Some js-interop code accesses 'window' as 'self.window'
295303
if (typeof self.window == "undefined") self.window = self;
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'dart:async';
6+
import 'package:expect/expect.dart';
7+
import 'package:reload_test/reload_test_utils.dart';
8+
9+
Future<void> main() async {}
10+
11+
/** DIFF **/
12+
/*
13+
import 'package:expect/expect.dart';
14+
import 'package:reload_test/reload_test_utils.dart';
15+
16+
-bool beforeRestart = true;
17+
-bool calledBeforeRestart = false;
18+
-bool calledAfterRestart = false;
19+
-void callback(_) {
20+
- if (beforeRestart) {
21+
- calledBeforeRestart = true;
22+
- } else {
23+
- throw Exception('Should never run.');
24+
- }
25+
-}
26+
-
27+
-Future<void> main() async {
28+
- Timer.periodic(Duration(milliseconds: 10), callback);
29+
- await new Future.delayed(Duration(milliseconds: 100));
30+
- Expect.isTrue(calledBeforeRestart);
31+
-
32+
- await hotRestart();
33+
-}
34+
+Future<void> main() async {}
35+
*/

0 commit comments

Comments
 (0)