From cd20b29917fa1eacaaeacc4e91712bc120702405 Mon Sep 17 00:00:00 2001 From: MarkZ Date: Fri, 18 Apr 2025 15:08:48 -0700 Subject: [PATCH] [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 Reviewed-by: Srujan Gaddam Reviewed-by: Nicholas Shahan --- .../lib/js/ddc/ddc_module_loader.js | 68 +++++++++++++++++-- pkg/reload_test/lib/ddc_helpers.dart | 62 +++++++++++++---- .../js_dev_runtime/private/preambles/d8.js | 8 +++ .../hot_restart_timer/main.2.restart.dart | 35 ++++++++++ 4 files changed, 156 insertions(+), 17 deletions(-) create mode 100644 tests/hot_reload/hot_restart_timer/main.2.restart.dart diff --git a/pkg/dev_compiler/lib/js/ddc/ddc_module_loader.js b/pkg/dev_compiler/lib/js/ddc/ddc_module_loader.js index df9d4e615f6..c1cd432aed7 100644 --- a/pkg/dev_compiler/lib/js/ddc/ddc_module_loader.js +++ b/pkg/dev_compiler/lib/js/ddc/ddc_module_loader.js @@ -1520,15 +1520,46 @@ if (!self.deferred_loader) { this.initializeAndLinkLibrary('dart:web_gl'); } + // Runs the 'main' method on `entryPointLibrary` while attaching + // `capturedMainHandler` and `mainErrorCallback`. + _runMain(entryPointLibrary, args = []) { + // TODO(35113): Provide the ability to pass arguments in a type safe way. + let runMainAndHandleErrors = () => { + try { + let mainValue = entryPointLibrary.main(args); + // Attach the error callback to main's future if it's async. + if (dartDevEmbedderConfig.mainErrorCallback != null && mainValue != null && + mainValue.catchError != null) { + mainValue.catchError((e) => { + dartDevEmbedderConfig.mainErrorCallback(); + throw e; + }); + } + } catch (e) { + // Invoke the error callback if main is sync. This doesn't conflict + // with the rethrow in the async path since DDC catches async errors + // in a separate code path. + if (dartDevEmbedderConfig.mainErrorCallback != null) { + dartDevEmbedderConfig.mainErrorCallback(); + } + throw e; + } + }; + if (dartDevEmbedderConfig.capturedMainHandler) { + dartDevEmbedderConfig.capturedMainHandler(runMainAndHandleErrors); + } else { + runMainAndHandleErrors(); + } + } + // See docs on `DartDevEmbedder.runMain`. - runMain(entryPointLibraryName, dartSdkRuntimeOptions) { + runMain(entryPointLibraryName, dartSdkRuntimeOptions, capturedMainHandler, mainErrorCallback) { this.setDartSDKRuntimeOptions(dartSdkRuntimeOptions); console.log('Starting application from main method in: ' + entryPointLibraryName + '.'); let entryPointLibrary = this.initializeAndLinkLibrary(entryPointLibraryName); this.savedEntryPointLibraryName = entryPointLibraryName; this.savedDartSdkRuntimeOptions = dartSdkRuntimeOptions; - // TODO(35113): Provide the ability to pass arguments in a type safe way. - entryPointLibrary.main([]); + this._runMain(entryPointLibrary); } setDartSDKRuntimeOptions(options) { @@ -1647,8 +1678,7 @@ if (!self.deferred_loader) { console.log('Hot restarting application from main method in: ' + this.savedEntryPointLibraryName + ' (generation: ' + this.hotRestartGeneration + ').'); - // TODO(35113): Provide the ability to pass arguments in a type safe way. - entryPointLibrary.main([]); + this._runMain(entryPointLibrary); } } @@ -1967,10 +1997,38 @@ if (!self.deferred_loader) { const debugger_ = new Debugger(); + /** Holds public configurations for the `DartDevEmbedder`. + * These properties may be modified during the runtime of the app. + */ + class DartDevEmbedderConfiguration { + /* + * An optional handler that acts as a wrapper around the invocation of the + * Dart program's 'main' method. Passed an opaque function as an argument + * that invokes 'main' when called. + * @type {?function(function())} + */ + capturedMainHandler = null; + + /* + * An optional callback that is invoked when 'main' throws. + * @type {?function()} + */ + mainErrorCallback = null; + } + + const dartDevEmbedderConfig = new DartDevEmbedderConfiguration(); + /** The API for embedding a Dart application in the page at development time * that supports stateful hot reloading. */ class DartDevEmbedder { + /** + * Expose the DartDevEmbedderConfig publicly. + */ + get config() { + return dartDevEmbedderConfig; + } + /** * Runs the Dart main method. * diff --git a/pkg/reload_test/lib/ddc_helpers.dart b/pkg/reload_test/lib/ddc_helpers.dart index e8718dfb40c..cb26f8521bb 100644 --- a/pkg/reload_test/lib/ddc_helpers.dart +++ b/pkg/reload_test/lib/ddc_helpers.dart @@ -173,8 +173,8 @@ self.\$injectedFilesAndLibrariesToReload = function(fileGeneration) { return [fileUrls, libraryIds]; } -// D8 does not support the core Timer API methods beside `setTimeout` so our -// D8 preambles provide a custom implementation. +// D8 does not support the core Timer API methods, so our D8 preamble provides +// a custom implementation. // // Timers in this implementation are simulated, so they all complete before // native JS `await` boundaries. If this boundary occurs before our runtime's @@ -183,17 +183,37 @@ self.\$injectedFilesAndLibrariesToReload = function(fileGeneration) { // // To resolve this, we record and increment hot restart generations early // and wrap timer functions with custom cancellation logic. -self.setTimeout = function(setTimeout) { - let currentHotRestartIteration = - self.\$dartLoader.loader.intendedHotRestartGeneration; +self.setInterval = function(setInterval) { return function(f, ms) { + var timerId; + let currentHotRestartIteration = + self.\$dartLoader.loader.intendedHotRestartGeneration; var internalCallback = function() { if (currentHotRestartIteration == - self.\$dartLoader.loader.intendedHotRestartGeneration) { + self.\$dartLoader.loader.intendedHotRestartGeneration) { f(); + } else { + self.clearInterval(timerId); } } - setTimeout(internalCallback, ms); + timerId = setInterval(internalCallback, ms); + }; +}(self.setInterval); + +self.setTimeout = function(setTimeout) { + return function(f, ms) { + var timerId; + let currentHotRestartIteration = + self.\$dartLoader.loader.intendedHotRestartGeneration; + var internalCallback = function() { + if (currentHotRestartIteration == + self.\$dartLoader.loader.intendedHotRestartGeneration) { + f(); + } else { + self.clearTimeout(timerId); + } + } + timerId = setTimeout(internalCallback, ms); }; }(self.setTimeout); @@ -202,15 +222,33 @@ self.setTimeout = function(setTimeout) { // by deleting the our custom implementation in D8's preamble. self.scheduleImmediate = void 0; +// Set embedder configurations. + +// Invoke main through the d8 preamble to ensure the code is running within the +// fake event loop. +dartDevEmbedder.config.capturedMainHandler = + dartDevEmbedder.config.capturedHotRestartMainHandler = + (runMain) => { + self.dartMainRunner((_) => { + runMain(); + }, []); + }; + + +// D8 timers execute synchronously, so periodic timers must be explicitly +// cancelled when an async main reaches an error state. +dartDevEmbedder.config.mainErrorCallback = + dartDevEmbedder.config.hotRestartMainErrorCallback = + () => { + self.clearAllTimers(); + } + // Begin loading libraries loader.nextAttempt(); -// Invoke main through the d8 preamble to ensure the code is running -// within the fake event loop. -self.dartMainRunner(function () { - dartDevEmbedder.runMain("$entrypointModuleName", {}); -}); +dartDevEmbedder.runMain("$entrypointModuleName", {}); '''; + return d8BootstrapJS; } diff --git a/sdk/lib/_internal/js_dev_runtime/private/preambles/d8.js b/sdk/lib/_internal/js_dev_runtime/private/preambles/d8.js index dea312bd061..921f3f72419 100644 --- a/sdk/lib/_internal/js_dev_runtime/private/preambles/d8.js +++ b/sdk/lib/_internal/js_dev_runtime/private/preambles/d8.js @@ -262,6 +262,13 @@ if (typeof global != "undefined") self = global; // Node.js. delete timerIds[id]; } + function cancelAllTimers() { + for (const id in timerIds) { + timerIds[id].$timerId = undefined; + } + timerIds = {}; + } + function eventLoop(action) { while (action) { try { @@ -290,6 +297,7 @@ if (typeof global != "undefined") self = global; // Node.js. self.setInterval = addInterval; self.clearInterval = cancelTimer; self.scheduleImmediate = addTask; + self.clearAllTimers = cancelAllTimers; // Some js-interop code accesses 'window' as 'self.window' if (typeof self.window == "undefined") self.window = self; diff --git a/tests/hot_reload/hot_restart_timer/main.2.restart.dart b/tests/hot_reload/hot_restart_timer/main.2.restart.dart new file mode 100644 index 00000000000..ba341f40901 --- /dev/null +++ b/tests/hot_reload/hot_restart_timer/main.2.restart.dart @@ -0,0 +1,35 @@ +// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'dart:async'; +import 'package:expect/expect.dart'; +import 'package:reload_test/reload_test_utils.dart'; + +Future main() async {} + +/** DIFF **/ +/* + import 'package:expect/expect.dart'; + import 'package:reload_test/reload_test_utils.dart'; + +-bool beforeRestart = true; +-bool calledBeforeRestart = false; +-bool calledAfterRestart = false; +-void callback(_) { +- if (beforeRestart) { +- calledBeforeRestart = true; +- } else { +- throw Exception('Should never run.'); +- } +-} +- +-Future main() async { +- Timer.periodic(Duration(milliseconds: 10), callback); +- await new Future.delayed(Duration(milliseconds: 100)); +- Expect.isTrue(calledBeforeRestart); +- +- await hotRestart(); +-} ++Future main() async {} +*/