From 96275cc57c47dac1b4bcbf0ea1096f65588d5874 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=AA=E3=81=A4=E3=81=8D?= Date: Sun, 27 Oct 2024 23:35:08 -0700 Subject: [PATCH 01/22] Implement `sass --embedded` in pure JS mode --- bin/sass.dart | 3 +- lib/src/embedded/compilation_dispatcher.dart | 18 +-- lib/src/embedded/concurrency.dart | 10 ++ lib/src/embedded/executable.dart | 35 ++---- lib/src/embedded/isolate_dispatcher.dart | 49 ++++---- lib/src/embedded/isolate_main.dart | 12 ++ lib/src/embedded/js/concurrency.dart | 11 ++ lib/src/embedded/js/executable.dart | 27 +++++ lib/src/embedded/js/io.dart | 66 +++++++++++ lib/src/embedded/js/isolate.dart | 17 +++ lib/src/embedded/js/isolate_main.dart | 14 +++ lib/src/embedded/js/js.dart | 13 +++ lib/src/embedded/js/reusable_isolate.dart | 112 +++++++++++++++++++ lib/src/embedded/js/sync_message_port.dart | 15 +++ lib/src/embedded/js/sync_receive_port.dart | 33 ++++++ lib/src/embedded/js/worker_threads.dart | 71 ++++++++++++ lib/src/embedded/options.dart | 32 ++++++ lib/src/embedded/reusable_isolate.dart | 6 +- lib/src/embedded/sync_receive_port.dart | 13 +++ lib/src/embedded/unavailable.dart | 10 -- lib/src/embedded/utils.dart | 12 +- lib/src/embedded/vm/sync_receive_port.dart | 19 ++++ package.json | 3 +- package/package.json | 3 +- tool/grind.dart | 3 + 25 files changed, 528 insertions(+), 79 deletions(-) create mode 100644 lib/src/embedded/concurrency.dart create mode 100644 lib/src/embedded/isolate_main.dart create mode 100644 lib/src/embedded/js/concurrency.dart create mode 100644 lib/src/embedded/js/executable.dart create mode 100644 lib/src/embedded/js/io.dart create mode 100644 lib/src/embedded/js/isolate.dart create mode 100644 lib/src/embedded/js/isolate_main.dart create mode 100644 lib/src/embedded/js/js.dart create mode 100644 lib/src/embedded/js/reusable_isolate.dart create mode 100644 lib/src/embedded/js/sync_message_port.dart create mode 100644 lib/src/embedded/js/sync_receive_port.dart create mode 100644 lib/src/embedded/js/worker_threads.dart create mode 100644 lib/src/embedded/options.dart create mode 100644 lib/src/embedded/sync_receive_port.dart delete mode 100644 lib/src/embedded/unavailable.dart create mode 100644 lib/src/embedded/vm/sync_receive_port.dart diff --git a/bin/sass.dart b/bin/sass.dart index dc0b079c8..382724305 100644 --- a/bin/sass.dart +++ b/bin/sass.dart @@ -18,8 +18,7 @@ import 'package:sass/src/io.dart'; import 'package:sass/src/stylesheet_graph.dart'; import 'package:sass/src/utils.dart'; import 'package:sass/src/embedded/executable.dart' - // Never load the embedded protocol when compiling to JS. - if (dart.library.js) 'package:sass/src/embedded/unavailable.dart' + if (dart.library.js) 'package:sass/src/embedded/js/executable.dart' as embedded; Future main(List args) async { diff --git a/lib/src/embedded/compilation_dispatcher.dart b/lib/src/embedded/compilation_dispatcher.dart index 190583890..e6fa3c46f 100644 --- a/lib/src/embedded/compilation_dispatcher.dart +++ b/lib/src/embedded/compilation_dispatcher.dart @@ -3,17 +3,16 @@ // https://opensource.org/licenses/MIT. import 'dart:convert'; -import 'dart:io'; -import 'dart:isolate'; +import 'dart:isolate' if (dart.library.js) 'js/isolate.dart'; import 'dart:typed_data'; -import 'package:native_synchronization/mailbox.dart'; import 'package:path/path.dart' as p; import 'package:protobuf/protobuf.dart'; import 'package:pub_semver/pub_semver.dart'; import 'package:sass/sass.dart' as sass; import 'package:sass/src/importer/node_package.dart' as npi; +import '../io.dart' show FileSystemException; import '../logger.dart'; import '../value/function.dart'; import '../value/mixin.dart'; @@ -23,6 +22,7 @@ import 'host_callable.dart'; import 'importer/file.dart'; import 'importer/host.dart'; import 'logger.dart'; +import 'sync_receive_port.dart'; import 'util/proto_extensions.dart'; import 'utils.dart'; @@ -35,8 +35,8 @@ final _outboundRequestId = 0; /// A class that dispatches messages to and from the host for a single /// compilation. final class CompilationDispatcher { - /// The mailbox for receiving messages from the host. - final Mailbox _mailbox; + /// The synchronous receive port for receiving messages from the host. + final SyncReceivePort _receivePort; /// The send port for sending messages to the host. final SendPort _sendPort; @@ -52,8 +52,8 @@ final class CompilationDispatcher { late Uint8List _compilationIdVarint; /// Creates a [CompilationDispatcher] that receives encoded protocol buffers - /// through [_mailbox] and sends them through [_sendPort]. - CompilationDispatcher(this._mailbox, this._sendPort); + /// through [_receivePort] and sends them through [_sendPort]. + CompilationDispatcher(this._receivePort, this._sendPort); /// Listens for incoming `CompileRequests` and runs their compilations. void listen() { @@ -427,9 +427,9 @@ final class CompilationDispatcher { /// Receive a packet from the host. Uint8List _receive() { try { - return _mailbox.take(); + return _receivePort.receive(); } on StateError catch (_) { - // The [_mailbox] has been closed, exit the current isolate immediately + // The [SyncReceivePort] has been closed, exit the current isolate immediately // to avoid bubble the error up as [SassException] during [_sendRequest]. Isolate.exit(); } diff --git a/lib/src/embedded/concurrency.dart b/lib/src/embedded/concurrency.dart new file mode 100644 index 000000000..66ca26081 --- /dev/null +++ b/lib/src/embedded/concurrency.dart @@ -0,0 +1,10 @@ +// Copyright 2024 Google Inc. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +import 'dart:ffi'; + +/// More than MaxMutatorThreadCount isolates in the same isolate group +/// can deadlock the Dart VM. +/// See https://github.com/sass/dart-sass/pull/2019 +int get concurrencyLimit => sizeOf() <= 4 ? 7 : 15; diff --git a/lib/src/embedded/executable.dart b/lib/src/embedded/executable.dart index d6eb24636..5c1ff52ca 100644 --- a/lib/src/embedded/executable.dart +++ b/lib/src/embedded/executable.dart @@ -3,40 +3,19 @@ // https://opensource.org/licenses/MIT. import 'dart:io'; -import 'dart:convert'; import 'package:stream_channel/stream_channel.dart'; import 'isolate_dispatcher.dart'; +import 'options.dart'; import 'util/length_delimited_transformer.dart'; void main(List args) { - switch (args) { - case ["--version", ...]: - var response = IsolateDispatcher.versionResponse(); - response.id = 0; - stdout.writeln( - JsonEncoder.withIndent(" ").convert(response.toProto3Json()), - ); - return; - - case [_, ...]: - stderr.writeln( - "sass --embedded is not intended to be executed with additional " - "arguments.\n" - "See https://github.com/sass/dart-sass#embedded-dart-sass for " - "details.", - ); - // USAGE error from https://bit.ly/2poTt90 - exitCode = 64; - return; + if (parseOptions(args)) { + IsolateDispatcher( + StreamChannel.withGuarantees(stdin, stdout, allowSinkErrors: false) + .transform(lengthDelimited), + gracefulShutdown: false) + .listen(); } - - IsolateDispatcher( - StreamChannel.withGuarantees( - stdin, - stdout, - allowSinkErrors: false, - ).transform(lengthDelimited), - ).listen(); } diff --git a/lib/src/embedded/isolate_dispatcher.dart b/lib/src/embedded/isolate_dispatcher.dart index 1ae8015f5..6df9044d3 100644 --- a/lib/src/embedded/isolate_dispatcher.dart +++ b/lib/src/embedded/isolate_dispatcher.dart @@ -3,19 +3,17 @@ // https://opensource.org/licenses/MIT. import 'dart:async'; -import 'dart:ffi'; -import 'dart:io'; -import 'dart:isolate'; +import 'dart:io' if (dart.library.js) 'js/io.dart'; import 'dart:typed_data'; -import 'package:native_synchronization/mailbox.dart'; import 'package:pool/pool.dart'; import 'package:protobuf/protobuf.dart'; import 'package:stream_channel/stream_channel.dart'; -import 'compilation_dispatcher.dart'; +import 'concurrency.dart' if (dart.library.js) 'js/concurrency.dart'; import 'embedded_sass.pb.dart'; -import 'reusable_isolate.dart'; +import 'isolate_main.dart' if (dart.library.js) 'js/isolate_main.dart'; +import 'reusable_isolate.dart' if (dart.library.js) 'js/reusable_isolate.dart'; import 'util/proto_extensions.dart'; import 'utils.dart'; @@ -25,6 +23,10 @@ class IsolateDispatcher { /// The channel of encoded protocol buffers, connected to the host. final StreamChannel _channel; + /// Whether to wait for all worker isolates to exit before exiting the main + /// isolate or not. + final bool _gracefulShutdown; + /// All isolates that have been spawned to dispatch to. /// /// Only used for cleaning up the process when the underlying channel closes. @@ -38,16 +40,13 @@ class IsolateDispatcher { /// A pool controlling how many isolates (and thus concurrent compilations) /// may be live at once. - /// - /// More than MaxMutatorThreadCount isolates in the same isolate group - /// can deadlock the Dart VM. - /// See https://github.com/sass/dart-sass/pull/2019 - final _isolatePool = Pool(sizeOf() <= 4 ? 7 : 15); + final _isolatePool = Pool(concurrencyLimit); /// Whether [_channel] has been closed or not. var _closed = false; - IsolateDispatcher(this._channel); + IsolateDispatcher(this._channel, {bool gracefulShutdown = true}) + : _gracefulShutdown = gracefulShutdown; void listen() { _channel.stream.listen( @@ -107,8 +106,12 @@ class IsolateDispatcher { _handleError(error, stackTrace); }, onDone: () { - _closed = true; - _allIsolates.stream.listen((isolate) => isolate.kill()); + if (_gracefulShutdown) { + _closed = true; + _allIsolates.stream.listen((isolate) => isolate.kill()); + } else { + exit(exitCode); + } }, ); } @@ -125,7 +128,7 @@ class IsolateDispatcher { _inactiveIsolates.remove(isolate); } else { var future = ReusableIsolate.spawn( - _isolateMain, + isolateMain, onError: (Object error, StackTrace stackTrace) { _handleError(error, stackTrace); }, @@ -158,7 +161,11 @@ class IsolateDispatcher { _channel.sink.add(packet); case 2: _channel.sink.add(packet); - exit(exitCode); + if (_gracefulShutdown) { + _channel.sink.close(); + } else { + exit(exitCode); + } } }); @@ -188,7 +195,11 @@ class IsolateDispatcher { compilationId ?? errorId, handleError(error, stackTrace, messageId: messageId), ); - _channel.sink.close(); + if (_gracefulShutdown) { + _channel.sink.close(); + } else { + exit(exitCode); + } } /// Sends [message] to the host. @@ -199,7 +210,3 @@ class IsolateDispatcher { void sendError(int compilationId, ProtocolError error) => _send(compilationId, OutboundMessage()..error = error); } - -void _isolateMain(Mailbox mailbox, SendPort sendPort) { - CompilationDispatcher(mailbox, sendPort).listen(); -} diff --git a/lib/src/embedded/isolate_main.dart b/lib/src/embedded/isolate_main.dart new file mode 100644 index 000000000..070992a14 --- /dev/null +++ b/lib/src/embedded/isolate_main.dart @@ -0,0 +1,12 @@ +// Copyright 2024 Google Inc. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +import 'dart:isolate' show SendPort; + +import 'compilation_dispatcher.dart'; +import 'sync_receive_port.dart'; + +void isolateMain(SyncReceivePort receivePort, SendPort sendPort) { + CompilationDispatcher(receivePort, sendPort).listen(); +} diff --git a/lib/src/embedded/js/concurrency.dart b/lib/src/embedded/js/concurrency.dart new file mode 100644 index 000000000..4966120ec --- /dev/null +++ b/lib/src/embedded/js/concurrency.dart @@ -0,0 +1,11 @@ +// Copyright 2024 Google Inc. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +import 'dart:js_interop'; + + +@JS('os.cpus') +external JSArray _cpus(); + +int get concurrencyLimit => _cpus().length; diff --git a/lib/src/embedded/js/executable.dart b/lib/src/embedded/js/executable.dart new file mode 100644 index 000000000..724d10c3c --- /dev/null +++ b/lib/src/embedded/js/executable.dart @@ -0,0 +1,27 @@ +// Copyright 2024 Google Inc. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +import 'package:stream_channel/stream_channel.dart'; + +import '../isolate_dispatcher.dart'; +import '../isolate_main.dart'; +import '../options.dart'; +import '../util/length_delimited_transformer.dart'; +import 'io.dart'; +import 'sync_receive_port.dart'; +import 'worker_threads.dart'; + +void main(List args) { + if (parseOptions(args)) { + if (isMainThread) { + IsolateDispatcher(StreamChannel.withGuarantees(stdin, stdout, + allowSinkErrors: false) + .transform(lengthDelimited)) + .listen(); + } else { + var port = workerData! as MessagePort; + isolateMain(JSSyncReceivePort(port), JSSendPort(port)); + } + } +} diff --git a/lib/src/embedded/js/io.dart b/lib/src/embedded/js/io.dart new file mode 100644 index 000000000..981a0fb65 --- /dev/null +++ b/lib/src/embedded/js/io.dart @@ -0,0 +1,66 @@ +// Copyright 2024 Google Inc. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +import 'dart:async'; +import 'dart:js_interop'; +import 'dart:typed_data'; + +@JS('process.exitCode') +external int? get _exitCode; +int get exitCode => _exitCode ?? 0; + +@JS('process.exitCode') +external set exitCode(int code); + +@JS('process.exit') +external void exit([int code]); + +@JS() +extension type _ReadStream(JSObject _) implements JSObject { + external void destroy(); + external void on(String type, JSFunction listener); +} + +@JS('process.stdin') +external _ReadStream get _stdin; + +@JS() +extension type _WriteStream(JSObject _) implements JSObject { + external void write(JSUint8Array chunk); +} + +@JS('process.stdout') +external _WriteStream get _stdout; + +Stream> get stdin { + var controller = StreamController( + onCancel: () { + _stdin.destroy(); + }, + sync: true); + _stdin.on( + 'data', + (JSUint8Array chunk) { + controller.sink.add(chunk.toDart); + }.toJS); + _stdin.on( + 'end', + () { + controller.sink.close(); + }.toJS); + _stdin.on( + 'error', + (JSObject e) { + controller.sink.addError(e); + }.toJS); + return controller.stream; +} + +StreamSink> get stdout { + var controller = StreamController(sync: true); + controller.stream.listen((buffer) { + _stdout.write(buffer.toJS); + }); + return controller.sink; +} diff --git a/lib/src/embedded/js/isolate.dart b/lib/src/embedded/js/isolate.dart new file mode 100644 index 000000000..1db935114 --- /dev/null +++ b/lib/src/embedded/js/isolate.dart @@ -0,0 +1,17 @@ +// Copyright 2024 Google LLC. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +import 'dart:isolate' show SendPort; +export 'dart:isolate' show SendPort; + +import 'io.dart' as io; + +abstract class Isolate { + static Never exit([SendPort? finalMessagePort, Object? message]) { + if (message != null) { + finalMessagePort?.send(message); + } + io.exit(io.exitCode) as Never; + } +} diff --git a/lib/src/embedded/js/isolate_main.dart b/lib/src/embedded/js/isolate_main.dart new file mode 100644 index 000000000..1224877fd --- /dev/null +++ b/lib/src/embedded/js/isolate_main.dart @@ -0,0 +1,14 @@ +// Copyright 2024 Google Inc. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +import 'dart:js_interop'; + +import 'js.dart'; + +@JS('process.argv') +external JSArray get _argv; + +(String, JSArray) isolateMain() { + return ((_argv[1]! as JSString).toDart, _argv.slice(2) as JSArray); +} diff --git a/lib/src/embedded/js/js.dart b/lib/src/embedded/js/js.dart new file mode 100644 index 000000000..040cbf0b2 --- /dev/null +++ b/lib/src/embedded/js/js.dart @@ -0,0 +1,13 @@ +// Copyright 2024 Google Inc. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +import 'dart:js_interop'; + +extension JSTypedArrayExtension on JSTypedArray { + external JSArrayBuffer get buffer; +} + +extension JSArrayExtension on JSArray { + external JSArray slice([int start, int end]); +} diff --git a/lib/src/embedded/js/reusable_isolate.dart b/lib/src/embedded/js/reusable_isolate.dart new file mode 100644 index 000000000..6919d82e6 --- /dev/null +++ b/lib/src/embedded/js/reusable_isolate.dart @@ -0,0 +1,112 @@ +// Copyright 2024 Google Inc. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +import 'dart:async'; +import 'dart:js_interop'; +import 'dart:typed_data'; + +import 'io.dart'; +import 'js.dart'; +import 'sync_message_port.dart'; +import 'worker_threads.dart'; + +/// The entrypoint for a [ReusableIsolate]. +/// +/// This must return a Record of filename and argv for creating the Worker. +typedef ReusableIsolateEntryPoint = (String, JSArray) Function(); + +class ReusableIsolate { + /// The worker. + final Worker _worker; + + /// The [MessagePort] used to receive messages to the [Worker]. + final MessagePort _receivePort; + + /// The [SyncMessagePort] used to send to the [Worker]. + final SyncMessagePort _sendPort; + + /// The subscription to [_receivePort]. + final StreamSubscription _subscription; + + /// Whether the current isolate has been borrowed. + bool _borrowed = false; + + ReusableIsolate._( + this._worker, this._sendPort, this._receivePort, this._subscription); + + /// Spawns a [ReusableIsolate] that runs the the entrypoint script. + static Future spawn(ReusableIsolateEntryPoint entryPoint, + {Function? onError}) async { + var (filename, argv) = entryPoint(); + var channel = SyncMessagePort.createChannel(); + var worker = Worker( + filename, + WorkerOptions( + workerData: channel.port2, + transferList: [channel.port2].toJS, + argv: argv)); + worker.once( + 'exit', + (int code) { + // Worker exit code 1 means it is killed by worker.terminate() + if (code > exitCode && code != 1) { + exitCode = code; + } + }.toJS); + var controller = StreamController(sync: true); + var sendPort = SyncMessagePort(channel.port1); + var receivePort = channel.port1; + receivePort.on( + 'message', + ((JSUint8Array buffer) { + controller.add(buffer.toDart); + }).toJS); + return ReusableIsolate._(worker, sendPort, receivePort, + controller.stream.listen(_defaultOnData)); + } + + /// Subscribe to messages from [_receivePort]. + void borrow(void onData(dynamic event)?) { + if (_borrowed) { + throw StateError('ReusableIsolate has already been borrowed.'); + } + _borrowed = true; + _subscription.onData(onData); + } + + /// Unsubscribe to messages from [_receivePort]. + void release() { + if (!_borrowed) { + throw StateError('ReusableIsolate has not been borrowed.'); + } + _borrowed = false; + _subscription.onData(_defaultOnData); + } + + /// Sends [message] to the isolate. + /// + /// Throws a [StateError] if this is called while the isolate isn't borrowed, + /// or if a second message is sent before the isolate has processed the first + /// one. + void send(Uint8List message) { + if (!_borrowed) { + throw StateError('Cannot send a message before being borrowed.'); + } + var array = message.toJS; + _sendPort.postMessage(array, [array.buffer].toJS); + } + + /// Shuts down the isolate. + void kill() { + _sendPort.close(); + _worker.terminate(); + _receivePort.close(); + } +} + +/// The default handler for data events from the wrapped isolate when it's not +/// borrowed. +void _defaultOnData(dynamic _) { + throw StateError("Shouldn't receive a message before being borrowed."); +} diff --git a/lib/src/embedded/js/sync_message_port.dart b/lib/src/embedded/js/sync_message_port.dart new file mode 100644 index 000000000..2630ec959 --- /dev/null +++ b/lib/src/embedded/js/sync_message_port.dart @@ -0,0 +1,15 @@ +// Copyright 2024 Google LLC. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +import 'dart:js_interop'; +import 'worker_threads.dart'; + +@JS('sync_message_port.SyncMessagePort') +extension type SyncMessagePort._(JSObject _) implements JSObject { + external static MessageChannel createChannel(); + external SyncMessagePort(MessagePort port); + external void postMessage(JSAny? value, [JSArray transferList]); + external JSAny? receiveMessage(); + external void close(); +} diff --git a/lib/src/embedded/js/sync_receive_port.dart b/lib/src/embedded/js/sync_receive_port.dart new file mode 100644 index 000000000..37b915e36 --- /dev/null +++ b/lib/src/embedded/js/sync_receive_port.dart @@ -0,0 +1,33 @@ +// Copyright 2024 Google Inc. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +import 'dart:js_interop'; +import 'dart:typed_data'; + +import '../sync_receive_port.dart'; +import 'isolate.dart'; +import 'js.dart'; +import 'sync_message_port.dart'; +import 'worker_threads.dart'; + +final class JSSyncReceivePort implements SyncReceivePort { + final SyncMessagePort _port; + + JSSyncReceivePort(MessagePort port) : _port = SyncMessagePort(port); + + Uint8List receive() { + return (_port.receiveMessage()! as JSUint8Array).toDart; + } +} + +final class JSSendPort implements SendPort { + final MessagePort _port; + + JSSendPort(this._port); + + void send(Object? message) { + var array = (message! as Uint8List).toJS; + _port.postMessage(array, [array.buffer].toJS); + } +} diff --git a/lib/src/embedded/js/worker_threads.dart b/lib/src/embedded/js/worker_threads.dart new file mode 100644 index 000000000..854e8c698 --- /dev/null +++ b/lib/src/embedded/js/worker_threads.dart @@ -0,0 +1,71 @@ +// Copyright 2024 Google Inc. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +import 'dart:js_interop'; + +@JS('worker_threads.isMainThread') +external bool get isMainThread; + +@JS('worker_threads.workerData') +external JSAny? get workerData; + +@JS('worker_threads.Worker') +extension type Worker._(JSObject _) implements JSObject { + external Worker(String filename, WorkerOptions options); + external void once(String type, JSFunction listener); + external void terminate(); +} + +@JS() +extension type WorkerOptions._(JSObject _) implements JSObject { + external WorkerOptions( + {JSArray argv, + JSObject env, + bool eval, + JSArray execArgv, + bool stdin, + bool stdout, + bool stderr, + JSAny workerData, + bool trackUnmanagedFds, + JSArray transferList, + ResourceLimits resourceLimits}); + external JSArray get argv; + external JSObject get env; + external bool get eval; + external JSArray get execArgv; + external bool get stdin; + external bool get stdout; + external bool get stderr; + external JSAny get workerData; + external bool get trackUnmanagedFds; + external JSArray get transferList; + external ResourceLimits get resourceLimits; +} + +@JS() +extension type ResourceLimits._(JSObject _) implements JSObject { + external ResourceLimits( + {int maxYoungGenerationSizeMb, + int maxOldGenerationSizeMb, + int codeRangeSizeMb, + int stackSizeMb}); + external int get maxYoungGenerationSizeMb; + external int get maxOldGenerationSizeMb; + external int get codeRangeSizeMb; + external int get stackSizeMb; +} + +@JS() +extension type MessageChannel._(JSObject _) implements JSObject { + external MessagePort get port1; + external MessagePort get port2; +} + +@JS() +extension type MessagePort._(JSObject _) implements JSObject { + external void postMessage(JSAny? value, [JSArray transferList]); + external void on(String type, JSFunction listener); + external void close(); +} diff --git a/lib/src/embedded/options.dart b/lib/src/embedded/options.dart new file mode 100644 index 000000000..e680a23dd --- /dev/null +++ b/lib/src/embedded/options.dart @@ -0,0 +1,32 @@ +// Copyright 2024 Google Inc. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +import 'dart:convert'; + +import '../io.dart'; +import 'isolate_dispatcher.dart'; + +/// Returns true if should start embedded compiler, +/// and false if should exit. +bool parseOptions(List args) { + switch (args) { + case ["--version", ...]: + var response = IsolateDispatcher.versionResponse(); + response.id = 0; + safePrint(JsonEncoder.withIndent(" ").convert(response.toProto3Json())); + return false; + + case [_, ...]: + printError( + "sass --embedded is not intended to be executed with additional " + "arguments.\n" + "See https://github.com/sass/dart-sass#embedded-dart-sass for " + "details."); + // USAGE error from https://bit.ly/2poTt90 + exitCode = 64; + return false; + } + + return true; +} diff --git a/lib/src/embedded/reusable_isolate.dart b/lib/src/embedded/reusable_isolate.dart index dc21d6873..ae9b0762f 100644 --- a/lib/src/embedded/reusable_isolate.dart +++ b/lib/src/embedded/reusable_isolate.dart @@ -9,6 +9,8 @@ import 'dart:typed_data'; import 'package:native_synchronization/mailbox.dart'; import 'package:native_synchronization/sendable.dart'; +import 'sync_receive_port.dart'; + /// The entrypoint for a [ReusableIsolate]. /// /// This must be a static global function. It's run when the isolate is spawned, @@ -19,7 +21,7 @@ import 'package:native_synchronization/sendable.dart'; /// If the [sendPort] sends a message before [ReusableIsolate.borrow] is called, /// this will throw an unhandled [StateError]. typedef ReusableIsolateEntryPoint = FutureOr Function( - Mailbox mailbox, SendPort sink); + SyncReceivePort receivePort, SendPort sendPort); class ReusableIsolate { /// The wrapped isolate. @@ -109,5 +111,5 @@ void _isolateMain( (ReusableIsolateEntryPoint, Sendable, SendPort) message, ) { var (entryPoint, sendableMailbox, sendPort) = message; - entryPoint(sendableMailbox.materialize(), sendPort); + entryPoint(MailboxSyncReceivePort(sendableMailbox.materialize()), sendPort); } diff --git a/lib/src/embedded/sync_receive_port.dart b/lib/src/embedded/sync_receive_port.dart new file mode 100644 index 000000000..2bc1e0060 --- /dev/null +++ b/lib/src/embedded/sync_receive_port.dart @@ -0,0 +1,13 @@ +// Copyright 2024 Google Inc. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +import 'dart:typed_data'; +export 'vm/sync_receive_port.dart' + if (dart.library.js) 'js/sync_receive_port.dart'; + +/// A common interface that is implemented by wrapping +/// Dart Mailbox or JS SyncMessagePort. +abstract interface class SyncReceivePort { + Uint8List receive(); +} diff --git a/lib/src/embedded/unavailable.dart b/lib/src/embedded/unavailable.dart deleted file mode 100644 index bf03a52bf..000000000 --- a/lib/src/embedded/unavailable.dart +++ /dev/null @@ -1,10 +0,0 @@ -// Copyright 2023 Google Inc. Use of this source code is governed by an -// MIT-style license that can be found in the LICENSE file or at -// https://opensource.org/licenses/MIT. - -import '../io.dart'; - -void main(List args) async { - printError('sass --embedded is unavailable in pure JS mode.'); - exitCode = 1; -} diff --git a/lib/src/embedded/utils.dart b/lib/src/embedded/utils.dart index ea7430cdd..96fda8404 100644 --- a/lib/src/embedded/utils.dart +++ b/lib/src/embedded/utils.dart @@ -2,7 +2,6 @@ // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. -import 'dart:io'; import 'dart:typed_data'; import 'package:protobuf/protobuf.dart'; @@ -10,6 +9,7 @@ import 'package:source_span/source_span.dart'; import 'package:stack_trace/stack_trace.dart'; import 'package:term_glyph/term_glyph.dart' as term_glyph; +import '../io.dart'; import '../syntax.dart'; import 'embedded_sass.pb.dart' as proto; import 'embedded_sass.pb.dart' hide SourceSpan, Syntax; @@ -140,15 +140,17 @@ ProtocolError handleError( }) { if (error is ProtocolError) { error.id = messageId ?? errorId; - stderr.write("Host caused ${error.type.name.toLowerCase()} error"); - if (error.id != errorId) stderr.write(" with request ${error.id}"); - stderr.writeln(": ${error.message}"); + var buffer = StringBuffer(); + buffer.write("Host caused ${error.type.name.toLowerCase()} error"); + if (error.id != errorId) buffer.write(" with request ${error.id}"); + buffer.write(": ${error.message}"); + printError(buffer.toString()); // PROTOCOL error from https://bit.ly/2poTt90 exitCode = 76; // EX_PROTOCOL return error; } else { var errorMessage = "$error\n${Chain.forTrace(stackTrace)}"; - stderr.write("Internal compiler error: $errorMessage"); + printError("Internal compiler error: $errorMessage"); exitCode = 70; // EX_SOFTWARE return ProtocolError() ..type = ProtocolErrorType.INTERNAL diff --git a/lib/src/embedded/vm/sync_receive_port.dart b/lib/src/embedded/vm/sync_receive_port.dart new file mode 100644 index 000000000..0ad7b04d5 --- /dev/null +++ b/lib/src/embedded/vm/sync_receive_port.dart @@ -0,0 +1,19 @@ +// Copyright 2024 Google Inc. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +import 'dart:typed_data'; + +import 'package:native_synchronization/mailbox.dart'; + +import '../sync_receive_port.dart'; + +final class MailboxSyncReceivePort implements SyncReceivePort { + final Mailbox _mailbox; + + MailboxSyncReceivePort(this._mailbox); + + Uint8List receive() { + return _mailbox.take(); + } +} diff --git a/package.json b/package.json index 0eb2cbfa9..03ebbc741 100644 --- a/package.json +++ b/package.json @@ -9,6 +9,7 @@ "@parcel/watcher": "^2.4.1", "chokidar": "^4.0.0", "immutable": "^5.0.2", - "intercept-stdout": "^0.1.2" + "intercept-stdout": "^0.1.2", + "sync-message-port": "v1.1.1" } } diff --git a/package/package.json b/package/package.json index 5b096250a..4a1c0d772 100644 --- a/package/package.json +++ b/package/package.json @@ -19,7 +19,8 @@ "dependencies": { "chokidar": "^4.0.0", "immutable": "^5.0.2", - "source-map-js": ">=0.6.2 <2.0.0" + "source-map-js": ">=0.6.2 <2.0.0", + "sync-message-port": "^1.1.1" }, "optionalDependencies": { "@parcel/watcher": "^2.4.1" diff --git a/tool/grind.dart b/tool/grind.dart index 6265c803b..1165bb613 100644 --- a/tool/grind.dart +++ b/tool/grind.dart @@ -49,8 +49,11 @@ void main(List args) { target: pkg.JSRequireTarget.node, identifier: 'nodeModule', ), + pkg.JSRequire("os", target: pkg.JSRequireTarget.cli), pkg.JSRequire("stream", target: pkg.JSRequireTarget.node), pkg.JSRequire("util", target: pkg.JSRequireTarget.node), + pkg.JSRequire("worker_threads", target: pkg.JSRequireTarget.cli), + pkg.JSRequire("sync-message-port", target: pkg.JSRequireTarget.cli), ]; pkg.jsModuleMainLibrary.value = "lib/src/js.dart"; pkg.npmPackageJson.fn = () => From 85784c42c9206316797bd011216d964d47bdfb06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=AA=E3=81=A4=E3=81=8D?= Date: Mon, 28 Oct 2024 10:54:16 -0700 Subject: [PATCH 02/22] Run embedded test on both dart and node --- lib/src/embedded/js/concurrency.dart | 1 - test/embedded/dart/file_importer_test.dart | 16 ++++++++++ test/embedded/dart/function_test.dart | 16 ++++++++++ test/embedded/dart/importer_test.dart | 16 ++++++++++ test/embedded/dart/length_delimited_test.dart | 16 ++++++++++ test/embedded/dart/protocol_test.dart | 16 ++++++++++ test/embedded/dart_test.dart | 22 +++++++++++++ test/embedded/node/file_importer_test.dart | 17 ++++++++++ test/embedded/node/function_test.dart | 17 ++++++++++ test/embedded/node/importer_test.dart | 17 ++++++++++ test/embedded/node/length_delimited_test.dart | 17 ++++++++++ test/embedded/node/protocol_test.dart | 17 ++++++++++ test/embedded/node_test.dart | 24 ++++++++++++++ .../{ => shared}/embedded_process.dart | 31 +++++++------------ .../file_importer.dart} | 7 ++--- .../function.dart} | 7 ++--- .../importer.dart} | 7 ++--- .../length_delimited.dart} | 5 +-- .../protocol.dart} | 20 ++++++------ test/embedded/{ => shared}/utils.dart | 0 20 files changed, 241 insertions(+), 48 deletions(-) create mode 100644 test/embedded/dart/file_importer_test.dart create mode 100644 test/embedded/dart/function_test.dart create mode 100644 test/embedded/dart/importer_test.dart create mode 100644 test/embedded/dart/length_delimited_test.dart create mode 100644 test/embedded/dart/protocol_test.dart create mode 100644 test/embedded/dart_test.dart create mode 100644 test/embedded/node/file_importer_test.dart create mode 100644 test/embedded/node/function_test.dart create mode 100644 test/embedded/node/importer_test.dart create mode 100644 test/embedded/node/length_delimited_test.dart create mode 100644 test/embedded/node/protocol_test.dart create mode 100644 test/embedded/node_test.dart rename test/embedded/{ => shared}/embedded_process.dart (92%) rename test/embedded/{file_importer_test.dart => shared/file_importer.dart} (99%) rename test/embedded/{function_test.dart => shared/function.dart} (99%) rename test/embedded/{importer_test.dart => shared/importer.dart} (99%) rename test/embedded/{length_delimited_test.dart => shared/length_delimited.dart} (98%) rename test/embedded/{protocol_test.dart => shared/protocol.dart} (97%) rename test/embedded/{ => shared}/utils.dart (100%) diff --git a/lib/src/embedded/js/concurrency.dart b/lib/src/embedded/js/concurrency.dart index 4966120ec..57205479f 100644 --- a/lib/src/embedded/js/concurrency.dart +++ b/lib/src/embedded/js/concurrency.dart @@ -4,7 +4,6 @@ import 'dart:js_interop'; - @JS('os.cpus') external JSArray _cpus(); diff --git a/test/embedded/dart/file_importer_test.dart b/test/embedded/dart/file_importer_test.dart new file mode 100644 index 000000000..b9be558da --- /dev/null +++ b/test/embedded/dart/file_importer_test.dart @@ -0,0 +1,16 @@ +// Copyright 2024 Google LLC. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +@TestOn('vm') +library; + +import 'package:test/test.dart'; + +import '../shared/file_importer.dart'; +import '../dart_test.dart'; + +void main() { + setUpAll(ensureSnapshotUpToDate); + sharedTests(runSassEmbedded); +} diff --git a/test/embedded/dart/function_test.dart b/test/embedded/dart/function_test.dart new file mode 100644 index 000000000..4ea5fdb8e --- /dev/null +++ b/test/embedded/dart/function_test.dart @@ -0,0 +1,16 @@ +// Copyright 2024 Google LLC. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +@TestOn('vm') +library; + +import 'package:test/test.dart'; + +import '../shared/function.dart'; +import '../dart_test.dart'; + +void main() { + setUpAll(ensureSnapshotUpToDate); + sharedTests(runSassEmbedded); +} diff --git a/test/embedded/dart/importer_test.dart b/test/embedded/dart/importer_test.dart new file mode 100644 index 000000000..a3ce7a3e2 --- /dev/null +++ b/test/embedded/dart/importer_test.dart @@ -0,0 +1,16 @@ +// Copyright 2024 Google LLC. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +@TestOn('vm') +library; + +import 'package:test/test.dart'; + +import '../shared/importer.dart'; +import '../dart_test.dart'; + +void main() { + setUpAll(ensureSnapshotUpToDate); + sharedTests(runSassEmbedded); +} diff --git a/test/embedded/dart/length_delimited_test.dart b/test/embedded/dart/length_delimited_test.dart new file mode 100644 index 000000000..9c7991bc7 --- /dev/null +++ b/test/embedded/dart/length_delimited_test.dart @@ -0,0 +1,16 @@ +// Copyright 2024 Google LLC. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +@TestOn('vm') +library; + +import 'package:test/test.dart'; + +import '../shared/length_delimited.dart'; +import '../dart_test.dart'; + +void main() { + setUpAll(ensureSnapshotUpToDate); + sharedTests(); +} diff --git a/test/embedded/dart/protocol_test.dart b/test/embedded/dart/protocol_test.dart new file mode 100644 index 000000000..0223773ff --- /dev/null +++ b/test/embedded/dart/protocol_test.dart @@ -0,0 +1,16 @@ +// Copyright 2024 Google LLC. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +@TestOn('vm') +library; + +import 'package:test/test.dart'; + +import '../shared/protocol.dart'; +import '../dart_test.dart'; + +void main() { + setUpAll(ensureSnapshotUpToDate); + sharedTests(runSassEmbedded); +} diff --git a/test/embedded/dart_test.dart b/test/embedded/dart_test.dart new file mode 100644 index 000000000..bac02cfc7 --- /dev/null +++ b/test/embedded/dart_test.dart @@ -0,0 +1,22 @@ +// Copyright 2024 Google LLC. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +@TestOn('vm') +library; + +import 'package:cli_pkg/testing.dart' as pkg; +import 'package:test/test.dart'; + +import 'shared/embedded_process.dart'; + +void main() {} + +/// Ensures that the snapshot of the Dart executable used by [runSassEmbedded] is +/// up-to-date, if one has been generated. +void ensureSnapshotUpToDate() => pkg.ensureExecutableUpToDate("sass"); + +Future runSassEmbedded( + [Iterable args = const Iterable.empty()]) => + EmbeddedProcess.start(pkg.executableRunner("sass"), + [...pkg.executableArgs("sass"), "--embedded", ...args]); diff --git a/test/embedded/node/file_importer_test.dart b/test/embedded/node/file_importer_test.dart new file mode 100644 index 000000000..3723d2342 --- /dev/null +++ b/test/embedded/node/file_importer_test.dart @@ -0,0 +1,17 @@ +// Copyright 2024 Google LLC. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +@TestOn('vm') +@Tags(['node']) +library; + +import 'package:test/test.dart'; + +import '../shared/file_importer.dart'; +import '../node_test.dart'; + +void main() { + setUpAll(ensureSnapshotUpToDate); + sharedTests(runSassEmbedded); +} diff --git a/test/embedded/node/function_test.dart b/test/embedded/node/function_test.dart new file mode 100644 index 000000000..9e1daf692 --- /dev/null +++ b/test/embedded/node/function_test.dart @@ -0,0 +1,17 @@ +// Copyright 2024 Google LLC. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +@TestOn('vm') +@Tags(['node']) +library; + +import 'package:test/test.dart'; + +import '../shared/function.dart'; +import '../node_test.dart'; + +void main() { + setUpAll(ensureSnapshotUpToDate); + sharedTests(runSassEmbedded); +} diff --git a/test/embedded/node/importer_test.dart b/test/embedded/node/importer_test.dart new file mode 100644 index 000000000..cb6843bcc --- /dev/null +++ b/test/embedded/node/importer_test.dart @@ -0,0 +1,17 @@ +// Copyright 2024 Google LLC. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +@TestOn('vm') +@Tags(['node']) +library; + +import 'package:test/test.dart'; + +import '../shared/importer.dart'; +import '../node_test.dart'; + +void main() { + setUpAll(ensureSnapshotUpToDate); + sharedTests(runSassEmbedded); +} diff --git a/test/embedded/node/length_delimited_test.dart b/test/embedded/node/length_delimited_test.dart new file mode 100644 index 000000000..b3a425e8f --- /dev/null +++ b/test/embedded/node/length_delimited_test.dart @@ -0,0 +1,17 @@ +// Copyright 2024 Google LLC. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +@TestOn('vm') +@Tags(['node']) +library; + +import 'package:test/test.dart'; + +import '../shared/length_delimited.dart'; +import '../node_test.dart'; + +void main() { + setUpAll(ensureSnapshotUpToDate); + sharedTests(); +} diff --git a/test/embedded/node/protocol_test.dart b/test/embedded/node/protocol_test.dart new file mode 100644 index 000000000..f6c8c658b --- /dev/null +++ b/test/embedded/node/protocol_test.dart @@ -0,0 +1,17 @@ +// Copyright 2024 Google LLC. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +@TestOn('vm') +@Tags(['node']) +library; + +import 'package:test/test.dart'; + +import '../shared/protocol.dart'; +import '../node_test.dart'; + +void main() { + setUpAll(ensureSnapshotUpToDate); + sharedTests(runSassEmbedded); +} diff --git a/test/embedded/node_test.dart b/test/embedded/node_test.dart new file mode 100644 index 000000000..5af39b2d7 --- /dev/null +++ b/test/embedded/node_test.dart @@ -0,0 +1,24 @@ +// Copyright 2024 Google LLC. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +@TestOn('vm') +@Tags(['node']) +library; + +import 'package:cli_pkg/testing.dart' as pkg; +import 'package:test/test.dart'; + +import '../ensure_npm_package.dart'; +import 'shared/embedded_process.dart'; + +void main() {} + +/// Ensures that the snapshot of the npm package used by [runSassEmbedded] is +/// up-to-date, if one has been generated. +void ensureSnapshotUpToDate() => ensureNpmPackage; + +Future runSassEmbedded( + [Iterable args = const Iterable.empty()]) => + EmbeddedProcess.start(pkg.executableRunner("sass", node: true), + [...pkg.executableArgs("sass", node: true), "--embedded", ...args]); diff --git a/test/embedded/embedded_process.dart b/test/embedded/shared/embedded_process.dart similarity index 92% rename from test/embedded/embedded_process.dart rename to test/embedded/shared/embedded_process.dart index 6357566b5..5be965731 100644 --- a/test/embedded/embedded_process.dart +++ b/test/embedded/shared/embedded_process.dart @@ -2,15 +2,11 @@ // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. -@TestOn('vm') -library; - import 'dart:async'; import 'dart:convert'; import 'dart:io'; import 'package:async/async.dart'; -import 'package:cli_pkg/testing.dart' as pkg; import 'package:test/test.dart'; import 'package:sass/src/embedded/embedded_sass.pb.dart'; @@ -87,21 +83,18 @@ class EmbeddedProcess { /// If [forwardOutput] is `true`, the process's [outbound] messages and /// [stderr] will be printed to the console as they appear. This is only /// intended to be set temporarily to help when debugging test failures. - static Future start({ - String? workingDirectory, - Map? environment, - bool includeParentEnvironment = true, - bool runInShell = false, - bool forwardOutput = false, - }) async { - var process = await Process.start( - pkg.executableRunner("sass"), - [...pkg.executableArgs("sass"), "--embedded"], - workingDirectory: workingDirectory, - environment: environment, - includeParentEnvironment: includeParentEnvironment, - runInShell: runInShell, - ); + + static Future start(String command, List args, + {String? workingDirectory, + Map? environment, + bool includeParentEnvironment = true, + bool runInShell = false, + bool forwardOutput = false}) async { + var process = await Process.start(command, args, + workingDirectory: workingDirectory, + environment: environment, + includeParentEnvironment: includeParentEnvironment, + runInShell: runInShell); return EmbeddedProcess._(process, forwardOutput: forwardOutput); } diff --git a/test/embedded/file_importer_test.dart b/test/embedded/shared/file_importer.dart similarity index 99% rename from test/embedded/file_importer_test.dart rename to test/embedded/shared/file_importer.dart index 6d8e99079..c670c98c2 100644 --- a/test/embedded/file_importer_test.dart +++ b/test/embedded/shared/file_importer.dart @@ -2,9 +2,6 @@ // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. -@TestOn('vm') -library; - import 'package:path/path.dart' as p; import 'package:test/test.dart'; import 'package:test_descriptor/test_descriptor.dart' as d; @@ -15,10 +12,10 @@ import 'package:sass/src/embedded/utils.dart'; import 'embedded_process.dart'; import 'utils.dart'; -void main() { +void sharedTests(Future runSassEmbedded()) { late EmbeddedProcess process; setUp(() async { - process = await EmbeddedProcess.start(); + process = await runSassEmbedded(); }); group("emits a protocol error", () { diff --git a/test/embedded/function_test.dart b/test/embedded/shared/function.dart similarity index 99% rename from test/embedded/function_test.dart rename to test/embedded/shared/function.dart index bd0761cc6..68101e391 100644 --- a/test/embedded/function_test.dart +++ b/test/embedded/shared/function.dart @@ -2,9 +2,6 @@ // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. -@TestOn('vm') -library; - import 'package:test/test.dart'; import 'package:sass/src/embedded/embedded_sass.pb.dart'; @@ -19,9 +16,9 @@ final _null = Value()..singleton = SingletonValue.NULL; late EmbeddedProcess _process; -void main() { +void sharedTests(Future runSassEmbedded()) async { setUp(() async { - _process = await EmbeddedProcess.start(); + _process = await runSassEmbedded(); }); group("emits a compile failure for a custom function with a signature", () { diff --git a/test/embedded/importer_test.dart b/test/embedded/shared/importer.dart similarity index 99% rename from test/embedded/importer_test.dart rename to test/embedded/shared/importer.dart index de036c6d4..69a95f8b7 100644 --- a/test/embedded/importer_test.dart +++ b/test/embedded/shared/importer.dart @@ -2,9 +2,6 @@ // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. -@TestOn('vm') -library; - import 'package:source_maps/source_maps.dart' as source_maps; import 'package:test/test.dart'; import 'package:test_descriptor/test_descriptor.dart' as d; @@ -15,10 +12,10 @@ import 'package:sass/src/embedded/utils.dart'; import 'embedded_process.dart'; import 'utils.dart'; -void main() { +void sharedTests(Future runSassEmbedded()) { late EmbeddedProcess process; setUp(() async { - process = await EmbeddedProcess.start(); + process = await runSassEmbedded(); }); group("emits a protocol error", () { diff --git a/test/embedded/length_delimited_test.dart b/test/embedded/shared/length_delimited.dart similarity index 98% rename from test/embedded/length_delimited_test.dart rename to test/embedded/shared/length_delimited.dart index a26d35954..c5f518b63 100644 --- a/test/embedded/length_delimited_test.dart +++ b/test/embedded/shared/length_delimited.dart @@ -2,9 +2,6 @@ // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. -@TestOn('vm') -library; - import 'dart:async'; import 'dart:typed_data'; @@ -13,7 +10,7 @@ import 'package:sass/src/embedded/util/length_delimited_transformer.dart'; import 'package:async/async.dart'; import 'package:test/test.dart'; -void main() { +void sharedTests() { group("encoder", () { late Sink> sink; late Stream> stream; diff --git a/test/embedded/protocol_test.dart b/test/embedded/shared/protocol.dart similarity index 97% rename from test/embedded/protocol_test.dart rename to test/embedded/shared/protocol.dart index 5a0a41a12..2b5baf4de 100644 --- a/test/embedded/protocol_test.dart +++ b/test/embedded/shared/protocol.dart @@ -2,9 +2,6 @@ // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. -@TestOn('vm') -library; - import 'package:path/path.dart' as p; import 'package:pub_semver/pub_semver.dart'; import 'package:source_maps/source_maps.dart' as source_maps; @@ -17,10 +14,10 @@ import 'package:sass/src/embedded/utils.dart'; import 'embedded_process.dart'; import 'utils.dart'; -void main() { +void sharedTests(Future runSassEmbedded()) { late EmbeddedProcess process; setUp(() async { - process = await EmbeddedProcess.start(); + process = await runSassEmbedded(); }); group("exits upon protocol error", () { @@ -521,11 +518,16 @@ void main() { ); var failure = await getCompileFailure(process); - expect(failure.message, startsWith("Cannot open file: ")); expect( - failure.message.replaceFirst("Cannot open file: ", "").trim(), - equalsPath(d.path('test.scss')), - ); + failure.message, + anyOf(startsWith("Cannot open file: "), + startsWith("no such file or directory: "))); + expect( + failure.message + .replaceFirst("Cannot open file: ", "") + .replaceFirst("no such file or directory: ", "") + .trim(), + equalsPath(d.path('test.scss'))); expect(failure.span.text, equals('')); expect(failure.span.context, equals('')); expect(failure.span.start, equals(SourceSpan_SourceLocation())); diff --git a/test/embedded/utils.dart b/test/embedded/shared/utils.dart similarity index 100% rename from test/embedded/utils.dart rename to test/embedded/shared/utils.dart From f31245cc161c0b49523ff19179292e87d2e31cb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=AA=E3=81=A4=E3=81=8D?= Date: Mon, 28 Oct 2024 13:35:35 -0700 Subject: [PATCH 03/22] Run embedded JS API test on both dart and node compiler --- .github/workflows/test.yml | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 5e7cb5ec3..f9d78c62d 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -128,24 +128,40 @@ jobs: working-directory: sass-spec sass_spec_js_embedded: - name: 'JS API Tests | Embedded | Node ${{ matrix.node-version }} | ${{ matrix.os }}' + name: "JS API Tests | Embedded ${{ matrix.js && 'Pure JS' || 'Dart' }} | Node ${{ matrix.node-version }} | ${{ matrix.os }}" runs-on: ${{ matrix.os }} if: "github.event_name != 'pull_request' || !contains(github.event.pull_request.body, 'skip sass-embedded')" strategy: fail-fast: false matrix: + js: [true, false] os: [ubuntu-latest, windows-latest, macos-latest] node-version: ['lts/*'] include: # Test older LTS versions - - os: ubuntu-latest + - js: true + os: ubuntu-latest dart_channel: stable node-version: lts/-1 - - os: ubuntu-latest + - js: true + os: ubuntu-latest dart_channel: stable node-version: lts/-2 - - os: ubuntu-latest + - js: true + os: ubuntu-latest + dart_channel: stable + node-version: lts/-3 + - js: false + os: ubuntu-latest + dart_channel: stable + node-version: lts/-1 + - js: false + os: ubuntu-latest + dart_channel: stable + node-version: lts/-2 + - js: false + os: ubuntu-latest dart_channel: stable node-version: lts/-3 @@ -168,7 +184,7 @@ jobs: - name: Initialize embedded host run: | npm install - npm run init -- --compiler-path=.. --language-path=../build/language + npm run init -- --compiler-path=.. --language-path=../build/language --compiler-js=${{ matrix.js && 'true' || 'false' }} npm run compile working-directory: embedded-host-node From 496cb026fc7ee90a9a9124147de2c9773c00d0fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=AA=E3=81=A4=E3=81=8D?= Date: Tue, 29 Oct 2024 12:36:28 -0700 Subject: [PATCH 04/22] Update README.md --- README.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/README.md b/README.md index de3edaad5..c78cd282d 100644 --- a/README.md +++ b/README.md @@ -434,9 +434,7 @@ an API for users to invoke Sass and define custom functions and importers. * `sass --embedded --version` prints `versionResponse` with `id = 0` in JSON and exits. -The `--embedded` command-line flag is not available when you install Dart Sass -as an [npm package]. No other command-line flags are supported with -`--embedded`. +No other command-line flags are supported with `--embedded`. [npm package]: #from-npm From 1df6c06891bab50307a9b9904f61560b960c62b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=AA=E3=81=A4=E3=81=8D?= Date: Fri, 15 Nov 2024 16:34:16 -0800 Subject: [PATCH 05/22] Pass exitCode via message --- lib/src/embedded/compilation_dispatcher.dart | 11 ++++++----- lib/src/embedded/isolate_dispatcher.dart | 13 +++++++------ lib/src/embedded/js/reusable_isolate.dart | 9 --------- 3 files changed, 13 insertions(+), 20 deletions(-) diff --git a/lib/src/embedded/compilation_dispatcher.dart b/lib/src/embedded/compilation_dispatcher.dart index e6fa3c46f..01580d9ca 100644 --- a/lib/src/embedded/compilation_dispatcher.dart +++ b/lib/src/embedded/compilation_dispatcher.dart @@ -3,6 +3,7 @@ // https://opensource.org/licenses/MIT. import 'dart:convert'; +import 'dart:io' if (dart.library.js) 'js/io.dart'; import 'dart:isolate' if (dart.library.js) 'js/isolate.dart'; import 'dart:typed_data'; @@ -408,16 +409,16 @@ final class CompilationDispatcher { message.writeToCodedBufferWriter(protobufWriter); // Add one additional byte to the beginning to indicate whether or not the - // compilation has finished (1) or encountered a fatal error (2), so the - // [IsolateDispatcher] knows whether to treat this isolate as inactive or - // close out entirely. + // compilation has finished (1) or encountered a fatal error (exitCode), so + // the [IsolateDispatcher] knows whether to treat this isolate as inactive + // or close out entirely. var packet = Uint8List( 1 + _compilationIdVarint.length + protobufWriter.lengthInBytes, ); packet[0] = switch (message.whichMessage()) { OutboundMessage_Message.compileResponse => 1, - OutboundMessage_Message.error => 2, - _ => 0, + OutboundMessage_Message.error => exitCode, + _ => 0 }; packet.setAll(1, _compilationIdVarint); protobufWriter.writeTo(packet, 1 + _compilationIdVarint.length); diff --git a/lib/src/embedded/isolate_dispatcher.dart b/lib/src/embedded/isolate_dispatcher.dart index 6df9044d3..a4a2711ba 100644 --- a/lib/src/embedded/isolate_dispatcher.dart +++ b/lib/src/embedded/isolate_dispatcher.dart @@ -141,11 +141,11 @@ class IsolateDispatcher { var fullBuffer = message as Uint8List; // The first byte of messages from isolates indicates whether the entire - // compilation is finished (1) or if it encountered an error (2). Sending - // this as part of the message buffer rather than a separate message - // avoids a race condition where the host might send a new compilation - // request with the same ID as one that just finished before the - // [IsolateDispatcher] receives word that the isolate with that ID is + // compilation is finished (1) or if it encountered an error (exitCode). + // Sending this as part of the message buffer rather than a separate + // message avoids a race condition where the host might send a new + // compilation request with the same ID as one that just finished before + // the [IsolateDispatcher] receives word that the isolate with that ID is // done. See sass/dart-sass#2004. var category = fullBuffer[0]; var packet = Uint8List.sublistView(fullBuffer, 1); @@ -159,8 +159,9 @@ class IsolateDispatcher { _inactiveIsolates.add(isolate); resource.release(); _channel.sink.add(packet); - case 2: + default: _channel.sink.add(packet); + exitCode = category; if (_gracefulShutdown) { _channel.sink.close(); } else { diff --git a/lib/src/embedded/js/reusable_isolate.dart b/lib/src/embedded/js/reusable_isolate.dart index 6919d82e6..e80173c84 100644 --- a/lib/src/embedded/js/reusable_isolate.dart +++ b/lib/src/embedded/js/reusable_isolate.dart @@ -6,7 +6,6 @@ import 'dart:async'; import 'dart:js_interop'; import 'dart:typed_data'; -import 'io.dart'; import 'js.dart'; import 'sync_message_port.dart'; import 'worker_threads.dart'; @@ -46,14 +45,6 @@ class ReusableIsolate { workerData: channel.port2, transferList: [channel.port2].toJS, argv: argv)); - worker.once( - 'exit', - (int code) { - // Worker exit code 1 means it is killed by worker.terminate() - if (code > exitCode && code != 1) { - exitCode = code; - } - }.toJS); var controller = StreamController(sync: true); var sendPort = SyncMessagePort(channel.port1); var receivePort = channel.port1; From 6f4de3140850374cc71d1652570f5cfd41822e72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=AA=E3=81=A4=E3=81=8D?= Date: Tue, 1 Apr 2025 18:44:52 -0700 Subject: [PATCH 06/22] Address review feedbacks --- .github/workflows/test.yml | 2 +- lib/src/embedded/concurrency.dart | 3 ++- lib/src/embedded/isolate_main.dart | 2 +- lib/src/embedded/js/concurrency.dart | 2 +- lib/src/embedded/js/executable.dart | 2 +- lib/src/embedded/js/io.dart | 2 +- lib/src/embedded/js/isolate.dart | 2 +- lib/src/embedded/js/isolate_main.dart | 2 +- lib/src/embedded/js/js.dart | 2 +- lib/src/embedded/js/reusable_isolate.dart | 2 +- lib/src/embedded/js/sync_message_port.dart | 2 +- lib/src/embedded/js/sync_receive_port.dart | 2 +- lib/src/embedded/js/worker_threads.dart | 2 +- lib/src/embedded/options.dart | 2 +- lib/src/embedded/sync_receive_port.dart | 2 +- lib/src/embedded/vm/sync_receive_port.dart | 2 +- test/embedded/dart/file_importer_test.dart | 2 +- test/embedded/dart/function_test.dart | 2 +- test/embedded/dart/importer_test.dart | 2 +- test/embedded/dart/length_delimited_test.dart | 2 +- test/embedded/dart/protocol_test.dart | 2 +- test/embedded/dart_test.dart | 2 +- test/embedded/node/file_importer_test.dart | 2 +- test/embedded/node/function_test.dart | 2 +- test/embedded/node/importer_test.dart | 2 +- test/embedded/node/length_delimited_test.dart | 2 +- test/embedded/node/protocol_test.dart | 2 +- test/embedded/node_test.dart | 2 +- 28 files changed, 29 insertions(+), 28 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index f9d78c62d..f168c1602 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -184,7 +184,7 @@ jobs: - name: Initialize embedded host run: | npm install - npm run init -- --compiler-path=.. --language-path=../build/language --compiler-js=${{ matrix.js && 'true' || 'false' }} + npm run init -- --compiler-path=.. --language-path=../build/language ${{ matrix.js && '--compiler-js' || '' }} npm run compile working-directory: embedded-host-node diff --git a/lib/src/embedded/concurrency.dart b/lib/src/embedded/concurrency.dart index 66ca26081..03999baab 100644 --- a/lib/src/embedded/concurrency.dart +++ b/lib/src/embedded/concurrency.dart @@ -1,4 +1,4 @@ -// Copyright 2024 Google Inc. Use of this source code is governed by an +// Copyright 2025 Google Inc. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. @@ -6,5 +6,6 @@ import 'dart:ffi'; /// More than MaxMutatorThreadCount isolates in the same isolate group /// can deadlock the Dart VM. +/// /// See https://github.com/sass/dart-sass/pull/2019 int get concurrencyLimit => sizeOf() <= 4 ? 7 : 15; diff --git a/lib/src/embedded/isolate_main.dart b/lib/src/embedded/isolate_main.dart index 070992a14..660b6724b 100644 --- a/lib/src/embedded/isolate_main.dart +++ b/lib/src/embedded/isolate_main.dart @@ -1,4 +1,4 @@ -// Copyright 2024 Google Inc. Use of this source code is governed by an +// Copyright 2025 Google Inc. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. diff --git a/lib/src/embedded/js/concurrency.dart b/lib/src/embedded/js/concurrency.dart index 57205479f..fc6606fdb 100644 --- a/lib/src/embedded/js/concurrency.dart +++ b/lib/src/embedded/js/concurrency.dart @@ -1,4 +1,4 @@ -// Copyright 2024 Google Inc. Use of this source code is governed by an +// Copyright 2025 Google Inc. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. diff --git a/lib/src/embedded/js/executable.dart b/lib/src/embedded/js/executable.dart index 724d10c3c..d970e97e1 100644 --- a/lib/src/embedded/js/executable.dart +++ b/lib/src/embedded/js/executable.dart @@ -1,4 +1,4 @@ -// Copyright 2024 Google Inc. Use of this source code is governed by an +// Copyright 2025 Google Inc. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. diff --git a/lib/src/embedded/js/io.dart b/lib/src/embedded/js/io.dart index 981a0fb65..1e751d729 100644 --- a/lib/src/embedded/js/io.dart +++ b/lib/src/embedded/js/io.dart @@ -1,4 +1,4 @@ -// Copyright 2024 Google Inc. Use of this source code is governed by an +// Copyright 2025 Google Inc. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. diff --git a/lib/src/embedded/js/isolate.dart b/lib/src/embedded/js/isolate.dart index 1db935114..d4bdc8f0e 100644 --- a/lib/src/embedded/js/isolate.dart +++ b/lib/src/embedded/js/isolate.dart @@ -1,4 +1,4 @@ -// Copyright 2024 Google LLC. Use of this source code is governed by an +// Copyright 2025 Google LLC. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. diff --git a/lib/src/embedded/js/isolate_main.dart b/lib/src/embedded/js/isolate_main.dart index 1224877fd..010033425 100644 --- a/lib/src/embedded/js/isolate_main.dart +++ b/lib/src/embedded/js/isolate_main.dart @@ -1,4 +1,4 @@ -// Copyright 2024 Google Inc. Use of this source code is governed by an +// Copyright 2025 Google Inc. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. diff --git a/lib/src/embedded/js/js.dart b/lib/src/embedded/js/js.dart index 040cbf0b2..555da4985 100644 --- a/lib/src/embedded/js/js.dart +++ b/lib/src/embedded/js/js.dart @@ -1,4 +1,4 @@ -// Copyright 2024 Google Inc. Use of this source code is governed by an +// Copyright 2025 Google Inc. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. diff --git a/lib/src/embedded/js/reusable_isolate.dart b/lib/src/embedded/js/reusable_isolate.dart index e80173c84..1cf2fe915 100644 --- a/lib/src/embedded/js/reusable_isolate.dart +++ b/lib/src/embedded/js/reusable_isolate.dart @@ -1,4 +1,4 @@ -// Copyright 2024 Google Inc. Use of this source code is governed by an +// Copyright 2025 Google Inc. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. diff --git a/lib/src/embedded/js/sync_message_port.dart b/lib/src/embedded/js/sync_message_port.dart index 2630ec959..53a800bb7 100644 --- a/lib/src/embedded/js/sync_message_port.dart +++ b/lib/src/embedded/js/sync_message_port.dart @@ -1,4 +1,4 @@ -// Copyright 2024 Google LLC. Use of this source code is governed by an +// Copyright 2025 Google LLC. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. diff --git a/lib/src/embedded/js/sync_receive_port.dart b/lib/src/embedded/js/sync_receive_port.dart index 37b915e36..0f8097c35 100644 --- a/lib/src/embedded/js/sync_receive_port.dart +++ b/lib/src/embedded/js/sync_receive_port.dart @@ -1,4 +1,4 @@ -// Copyright 2024 Google Inc. Use of this source code is governed by an +// Copyright 2025 Google Inc. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. diff --git a/lib/src/embedded/js/worker_threads.dart b/lib/src/embedded/js/worker_threads.dart index 854e8c698..64bf064ee 100644 --- a/lib/src/embedded/js/worker_threads.dart +++ b/lib/src/embedded/js/worker_threads.dart @@ -1,4 +1,4 @@ -// Copyright 2024 Google Inc. Use of this source code is governed by an +// Copyright 2025 Google Inc. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. diff --git a/lib/src/embedded/options.dart b/lib/src/embedded/options.dart index e680a23dd..299237652 100644 --- a/lib/src/embedded/options.dart +++ b/lib/src/embedded/options.dart @@ -1,4 +1,4 @@ -// Copyright 2024 Google Inc. Use of this source code is governed by an +// Copyright 2025 Google Inc. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. diff --git a/lib/src/embedded/sync_receive_port.dart b/lib/src/embedded/sync_receive_port.dart index 2bc1e0060..9be0477c0 100644 --- a/lib/src/embedded/sync_receive_port.dart +++ b/lib/src/embedded/sync_receive_port.dart @@ -1,4 +1,4 @@ -// Copyright 2024 Google Inc. Use of this source code is governed by an +// Copyright 2025 Google Inc. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. diff --git a/lib/src/embedded/vm/sync_receive_port.dart b/lib/src/embedded/vm/sync_receive_port.dart index 0ad7b04d5..978bf2df5 100644 --- a/lib/src/embedded/vm/sync_receive_port.dart +++ b/lib/src/embedded/vm/sync_receive_port.dart @@ -1,4 +1,4 @@ -// Copyright 2024 Google Inc. Use of this source code is governed by an +// Copyright 2025 Google Inc. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. diff --git a/test/embedded/dart/file_importer_test.dart b/test/embedded/dart/file_importer_test.dart index b9be558da..caa81fcd8 100644 --- a/test/embedded/dart/file_importer_test.dart +++ b/test/embedded/dart/file_importer_test.dart @@ -1,4 +1,4 @@ -// Copyright 2024 Google LLC. Use of this source code is governed by an +// Copyright 2025 Google LLC. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. diff --git a/test/embedded/dart/function_test.dart b/test/embedded/dart/function_test.dart index 4ea5fdb8e..06849ed16 100644 --- a/test/embedded/dart/function_test.dart +++ b/test/embedded/dart/function_test.dart @@ -1,4 +1,4 @@ -// Copyright 2024 Google LLC. Use of this source code is governed by an +// Copyright 2025 Google LLC. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. diff --git a/test/embedded/dart/importer_test.dart b/test/embedded/dart/importer_test.dart index a3ce7a3e2..f7d0da951 100644 --- a/test/embedded/dart/importer_test.dart +++ b/test/embedded/dart/importer_test.dart @@ -1,4 +1,4 @@ -// Copyright 2024 Google LLC. Use of this source code is governed by an +// Copyright 2025 Google LLC. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. diff --git a/test/embedded/dart/length_delimited_test.dart b/test/embedded/dart/length_delimited_test.dart index 9c7991bc7..24cdb0f64 100644 --- a/test/embedded/dart/length_delimited_test.dart +++ b/test/embedded/dart/length_delimited_test.dart @@ -1,4 +1,4 @@ -// Copyright 2024 Google LLC. Use of this source code is governed by an +// Copyright 2025 Google LLC. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. diff --git a/test/embedded/dart/protocol_test.dart b/test/embedded/dart/protocol_test.dart index 0223773ff..d89860ab5 100644 --- a/test/embedded/dart/protocol_test.dart +++ b/test/embedded/dart/protocol_test.dart @@ -1,4 +1,4 @@ -// Copyright 2024 Google LLC. Use of this source code is governed by an +// Copyright 2025 Google LLC. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. diff --git a/test/embedded/dart_test.dart b/test/embedded/dart_test.dart index bac02cfc7..a3f480bb2 100644 --- a/test/embedded/dart_test.dart +++ b/test/embedded/dart_test.dart @@ -1,4 +1,4 @@ -// Copyright 2024 Google LLC. Use of this source code is governed by an +// Copyright 2025 Google LLC. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. diff --git a/test/embedded/node/file_importer_test.dart b/test/embedded/node/file_importer_test.dart index 3723d2342..076ab74a5 100644 --- a/test/embedded/node/file_importer_test.dart +++ b/test/embedded/node/file_importer_test.dart @@ -1,4 +1,4 @@ -// Copyright 2024 Google LLC. Use of this source code is governed by an +// Copyright 2025 Google LLC. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. diff --git a/test/embedded/node/function_test.dart b/test/embedded/node/function_test.dart index 9e1daf692..e9ed7bc18 100644 --- a/test/embedded/node/function_test.dart +++ b/test/embedded/node/function_test.dart @@ -1,4 +1,4 @@ -// Copyright 2024 Google LLC. Use of this source code is governed by an +// Copyright 2025 Google LLC. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. diff --git a/test/embedded/node/importer_test.dart b/test/embedded/node/importer_test.dart index cb6843bcc..d5a6a1ef1 100644 --- a/test/embedded/node/importer_test.dart +++ b/test/embedded/node/importer_test.dart @@ -1,4 +1,4 @@ -// Copyright 2024 Google LLC. Use of this source code is governed by an +// Copyright 2025 Google LLC. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. diff --git a/test/embedded/node/length_delimited_test.dart b/test/embedded/node/length_delimited_test.dart index b3a425e8f..9568c5ba9 100644 --- a/test/embedded/node/length_delimited_test.dart +++ b/test/embedded/node/length_delimited_test.dart @@ -1,4 +1,4 @@ -// Copyright 2024 Google LLC. Use of this source code is governed by an +// Copyright 2025 Google LLC. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. diff --git a/test/embedded/node/protocol_test.dart b/test/embedded/node/protocol_test.dart index f6c8c658b..4ea15966b 100644 --- a/test/embedded/node/protocol_test.dart +++ b/test/embedded/node/protocol_test.dart @@ -1,4 +1,4 @@ -// Copyright 2024 Google LLC. Use of this source code is governed by an +// Copyright 2025 Google LLC. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. diff --git a/test/embedded/node_test.dart b/test/embedded/node_test.dart index 5af39b2d7..ddc6eb801 100644 --- a/test/embedded/node_test.dart +++ b/test/embedded/node_test.dart @@ -1,4 +1,4 @@ -// Copyright 2024 Google LLC. Use of this source code is governed by an +// Copyright 2025 Google LLC. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. From 3d8b4f05e29353acd17f95d6163aef7b88de88f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=AA=E3=81=A4=E3=81=8D?= Date: Tue, 1 Apr 2025 19:57:11 -0700 Subject: [PATCH 07/22] Rename files and move things around --- bin/sass.dart | 4 +- lib/src/embedded/executable.dart | 20 +---- lib/src/embedded/js/executable.dart | 8 +- ...able_isolate.dart => reusable_worker.dart} | 30 +++---- ...olate_main.dart => worker_entrypoint.dart} | 2 +- lib/src/embedded/options.dart | 4 +- lib/src/embedded/{ => vm}/concurrency.dart | 0 lib/src/embedded/vm/executable.dart | 21 +++++ .../reusable_worker.dart} | 30 +++---- ...dispatcher.dart => worker_dispatcher.dart} | 81 ++++++++++--------- ...olate_main.dart => worker_entrypoint.dart} | 2 +- 11 files changed, 103 insertions(+), 99 deletions(-) rename lib/src/embedded/js/{reusable_isolate.dart => reusable_worker.dart} (72%) rename lib/src/embedded/js/{isolate_main.dart => worker_entrypoint.dart} (88%) rename lib/src/embedded/{ => vm}/concurrency.dart (100%) create mode 100644 lib/src/embedded/vm/executable.dart rename lib/src/embedded/{reusable_isolate.dart => vm/reusable_worker.dart} (79%) rename lib/src/embedded/{isolate_dispatcher.dart => worker_dispatcher.dart} (70%) rename lib/src/embedded/{isolate_main.dart => worker_entrypoint.dart} (82%) diff --git a/bin/sass.dart b/bin/sass.dart index 382724305..44ef98831 100644 --- a/bin/sass.dart +++ b/bin/sass.dart @@ -17,9 +17,7 @@ import 'package:sass/src/importer/filesystem.dart'; import 'package:sass/src/io.dart'; import 'package:sass/src/stylesheet_graph.dart'; import 'package:sass/src/utils.dart'; -import 'package:sass/src/embedded/executable.dart' - if (dart.library.js) 'package:sass/src/embedded/js/executable.dart' - as embedded; +import 'package:sass/src/embedded/executable.dart' as embedded; Future main(List args) async { if (args case ['--embedded', ...var rest]) { diff --git a/lib/src/embedded/executable.dart b/lib/src/embedded/executable.dart index 5c1ff52ca..89cc58ef8 100644 --- a/lib/src/embedded/executable.dart +++ b/lib/src/embedded/executable.dart @@ -1,21 +1,5 @@ -// Copyright 2019 Google Inc. Use of this source code is governed by an +// Copyright 2025 Google Inc. Use of this source code is governed by an // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. -import 'dart:io'; - -import 'package:stream_channel/stream_channel.dart'; - -import 'isolate_dispatcher.dart'; -import 'options.dart'; -import 'util/length_delimited_transformer.dart'; - -void main(List args) { - if (parseOptions(args)) { - IsolateDispatcher( - StreamChannel.withGuarantees(stdin, stdout, allowSinkErrors: false) - .transform(lengthDelimited), - gracefulShutdown: false) - .listen(); - } -} +export 'vm/executable.dart' if (dart.library.js) 'js/executable.dart'; diff --git a/lib/src/embedded/js/executable.dart b/lib/src/embedded/js/executable.dart index d970e97e1..363af460a 100644 --- a/lib/src/embedded/js/executable.dart +++ b/lib/src/embedded/js/executable.dart @@ -4,10 +4,10 @@ import 'package:stream_channel/stream_channel.dart'; -import '../isolate_dispatcher.dart'; -import '../isolate_main.dart'; import '../options.dart'; import '../util/length_delimited_transformer.dart'; +import '../worker_dispatcher.dart'; +import '../worker_entrypoint.dart'; import 'io.dart'; import 'sync_receive_port.dart'; import 'worker_threads.dart'; @@ -15,13 +15,13 @@ import 'worker_threads.dart'; void main(List args) { if (parseOptions(args)) { if (isMainThread) { - IsolateDispatcher(StreamChannel.withGuarantees(stdin, stdout, + WorkerDispatcher(StreamChannel.withGuarantees(stdin, stdout, allowSinkErrors: false) .transform(lengthDelimited)) .listen(); } else { var port = workerData! as MessagePort; - isolateMain(JSSyncReceivePort(port), JSSendPort(port)); + workerEntryPoint(JSSyncReceivePort(port), JSSendPort(port)); } } } diff --git a/lib/src/embedded/js/reusable_isolate.dart b/lib/src/embedded/js/reusable_worker.dart similarity index 72% rename from lib/src/embedded/js/reusable_isolate.dart rename to lib/src/embedded/js/reusable_worker.dart index 1cf2fe915..4d81deb3d 100644 --- a/lib/src/embedded/js/reusable_isolate.dart +++ b/lib/src/embedded/js/reusable_worker.dart @@ -10,12 +10,12 @@ import 'js.dart'; import 'sync_message_port.dart'; import 'worker_threads.dart'; -/// The entrypoint for a [ReusableIsolate]. +/// The entrypoint for a [ReusableWorker]. /// /// This must return a Record of filename and argv for creating the Worker. -typedef ReusableIsolateEntryPoint = (String, JSArray) Function(); +typedef ReusableWorkerEntryPoint = (String, JSArray) Function(); -class ReusableIsolate { +class ReusableWorker { /// The worker. final Worker _worker; @@ -28,14 +28,14 @@ class ReusableIsolate { /// The subscription to [_receivePort]. final StreamSubscription _subscription; - /// Whether the current isolate has been borrowed. + /// Whether the current worker has been borrowed. bool _borrowed = false; - ReusableIsolate._( + ReusableWorker._( this._worker, this._sendPort, this._receivePort, this._subscription); - /// Spawns a [ReusableIsolate] that runs the the entrypoint script. - static Future spawn(ReusableIsolateEntryPoint entryPoint, + /// Spawns a [ReusableWorker] that runs the the entrypoint script. + static Future spawn(ReusableWorkerEntryPoint entryPoint, {Function? onError}) async { var (filename, argv) = entryPoint(); var channel = SyncMessagePort.createChannel(); @@ -53,14 +53,14 @@ class ReusableIsolate { ((JSUint8Array buffer) { controller.add(buffer.toDart); }).toJS); - return ReusableIsolate._(worker, sendPort, receivePort, + return ReusableWorker._(worker, sendPort, receivePort, controller.stream.listen(_defaultOnData)); } /// Subscribe to messages from [_receivePort]. void borrow(void onData(dynamic event)?) { if (_borrowed) { - throw StateError('ReusableIsolate has already been borrowed.'); + throw StateError('ReusableWorker has already been borrowed.'); } _borrowed = true; _subscription.onData(onData); @@ -69,16 +69,16 @@ class ReusableIsolate { /// Unsubscribe to messages from [_receivePort]. void release() { if (!_borrowed) { - throw StateError('ReusableIsolate has not been borrowed.'); + throw StateError('ReusableWorker has not been borrowed.'); } _borrowed = false; _subscription.onData(_defaultOnData); } - /// Sends [message] to the isolate. + /// Sends [message] to the worker. /// - /// Throws a [StateError] if this is called while the isolate isn't borrowed, - /// or if a second message is sent before the isolate has processed the first + /// Throws a [StateError] if this is called while the worker isn't borrowed, + /// or if a second message is sent before the worker has processed the first /// one. void send(Uint8List message) { if (!_borrowed) { @@ -88,7 +88,7 @@ class ReusableIsolate { _sendPort.postMessage(array, [array.buffer].toJS); } - /// Shuts down the isolate. + /// Shuts down the worker. void kill() { _sendPort.close(); _worker.terminate(); @@ -96,7 +96,7 @@ class ReusableIsolate { } } -/// The default handler for data events from the wrapped isolate when it's not +/// The default handler for data events from the wrapped worker when it's not /// borrowed. void _defaultOnData(dynamic _) { throw StateError("Shouldn't receive a message before being borrowed."); diff --git a/lib/src/embedded/js/isolate_main.dart b/lib/src/embedded/js/worker_entrypoint.dart similarity index 88% rename from lib/src/embedded/js/isolate_main.dart rename to lib/src/embedded/js/worker_entrypoint.dart index 010033425..7541b59c7 100644 --- a/lib/src/embedded/js/isolate_main.dart +++ b/lib/src/embedded/js/worker_entrypoint.dart @@ -9,6 +9,6 @@ import 'js.dart'; @JS('process.argv') external JSArray get _argv; -(String, JSArray) isolateMain() { +(String, JSArray) workerEntryPoint() { return ((_argv[1]! as JSString).toDart, _argv.slice(2) as JSArray); } diff --git a/lib/src/embedded/options.dart b/lib/src/embedded/options.dart index 299237652..1c92b8037 100644 --- a/lib/src/embedded/options.dart +++ b/lib/src/embedded/options.dart @@ -5,14 +5,14 @@ import 'dart:convert'; import '../io.dart'; -import 'isolate_dispatcher.dart'; +import 'worker_dispatcher.dart'; /// Returns true if should start embedded compiler, /// and false if should exit. bool parseOptions(List args) { switch (args) { case ["--version", ...]: - var response = IsolateDispatcher.versionResponse(); + var response = WorkerDispatcher.versionResponse(); response.id = 0; safePrint(JsonEncoder.withIndent(" ").convert(response.toProto3Json())); return false; diff --git a/lib/src/embedded/concurrency.dart b/lib/src/embedded/vm/concurrency.dart similarity index 100% rename from lib/src/embedded/concurrency.dart rename to lib/src/embedded/vm/concurrency.dart diff --git a/lib/src/embedded/vm/executable.dart b/lib/src/embedded/vm/executable.dart new file mode 100644 index 000000000..c6c62bb35 --- /dev/null +++ b/lib/src/embedded/vm/executable.dart @@ -0,0 +1,21 @@ +// Copyright 2025 Google Inc. Use of this source code is governed by an +// MIT-style license that can be found in the LICENSE file or at +// https://opensource.org/licenses/MIT. + +import 'dart:io'; + +import 'package:stream_channel/stream_channel.dart'; + +import '../options.dart'; +import '../util/length_delimited_transformer.dart'; +import '../worker_dispatcher.dart'; + +void main(List args) { + if (parseOptions(args)) { + WorkerDispatcher( + StreamChannel.withGuarantees(stdin, stdout, allowSinkErrors: false) + .transform(lengthDelimited), + gracefulShutdown: false) + .listen(); + } +} diff --git a/lib/src/embedded/reusable_isolate.dart b/lib/src/embedded/vm/reusable_worker.dart similarity index 79% rename from lib/src/embedded/reusable_isolate.dart rename to lib/src/embedded/vm/reusable_worker.dart index ae9b0762f..510869acc 100644 --- a/lib/src/embedded/reusable_isolate.dart +++ b/lib/src/embedded/vm/reusable_worker.dart @@ -9,21 +9,21 @@ import 'dart:typed_data'; import 'package:native_synchronization/mailbox.dart'; import 'package:native_synchronization/sendable.dart'; -import 'sync_receive_port.dart'; +import '../sync_receive_port.dart'; -/// The entrypoint for a [ReusableIsolate]. +/// The entrypoint for a [ReusableWorker]. /// /// This must be a static global function. It's run when the isolate is spawned, -/// and is passed a [Mailbox] that receives messages from [ReusableIsolate.send] +/// and is passed a [Mailbox] that receives messages from [ReusableWorker.send] /// and a [SendPort] that sends messages to the [ReceivePort] listened by -/// [ReusableIsolate.borrow]. +/// [ReusableWorker.borrow]. /// -/// If the [sendPort] sends a message before [ReusableIsolate.borrow] is called, +/// If the [sendPort] sends a message before [ReusableWorker.borrow] is called, /// this will throw an unhandled [StateError]. -typedef ReusableIsolateEntryPoint = FutureOr Function( +typedef ReusableWorkerEntryPoint = FutureOr Function( SyncReceivePort receivePort, SendPort sendPort); -class ReusableIsolate { +class ReusableWorker { /// The wrapped isolate. final Isolate _isolate; @@ -39,16 +39,16 @@ class ReusableIsolate { /// Whether the current isolate has been borrowed. bool _borrowed = false; - ReusableIsolate._( + ReusableWorker._( this._isolate, this._mailbox, this._receivePort, { Function? onError, }) : _subscription = _receivePort.listen(_defaultOnData, onError: onError); - /// Spawns a [ReusableIsolate] that runs the given [entryPoint]. - static Future spawn( - ReusableIsolateEntryPoint entryPoint, { + /// Spawns a [ReusableWorker] that runs the given [entryPoint]. + static Future spawn( + ReusableWorkerEntryPoint entryPoint, { Function? onError, }) async { var mailbox = Mailbox(); @@ -58,13 +58,13 @@ class ReusableIsolate { mailbox.asSendable, receivePort.sendPort, )); - return ReusableIsolate._(isolate, mailbox, receivePort, onError: onError); + return ReusableWorker._(isolate, mailbox, receivePort, onError: onError); } /// Subscribe to messages from [_receivePort]. void borrow(void onData(dynamic event)?) { if (_borrowed) { - throw StateError('ReusableIsolate has already been borrowed.'); + throw StateError('ReusableWorker has already been borrowed.'); } _borrowed = true; _subscription.onData(onData); @@ -73,7 +73,7 @@ class ReusableIsolate { /// Unsubscribe to messages from [_receivePort]. void release() { if (!_borrowed) { - throw StateError('ReusableIsolate has not been borrowed.'); + throw StateError('ReusableWorker has not been borrowed.'); } _borrowed = false; _subscription.onData(_defaultOnData); @@ -108,7 +108,7 @@ void _defaultOnData(dynamic _) { } void _isolateMain( - (ReusableIsolateEntryPoint, Sendable, SendPort) message, + (ReusableWorkerEntryPoint, Sendable, SendPort) message, ) { var (entryPoint, sendableMailbox, sendPort) = message; entryPoint(MailboxSyncReceivePort(sendableMailbox.materialize()), sendPort); diff --git a/lib/src/embedded/isolate_dispatcher.dart b/lib/src/embedded/worker_dispatcher.dart similarity index 70% rename from lib/src/embedded/isolate_dispatcher.dart rename to lib/src/embedded/worker_dispatcher.dart index a4a2711ba..c96799dd4 100644 --- a/lib/src/embedded/isolate_dispatcher.dart +++ b/lib/src/embedded/worker_dispatcher.dart @@ -10,42 +10,43 @@ import 'package:pool/pool.dart'; import 'package:protobuf/protobuf.dart'; import 'package:stream_channel/stream_channel.dart'; -import 'concurrency.dart' if (dart.library.js) 'js/concurrency.dart'; import 'embedded_sass.pb.dart'; -import 'isolate_main.dart' if (dart.library.js) 'js/isolate_main.dart'; -import 'reusable_isolate.dart' if (dart.library.js) 'js/reusable_isolate.dart'; import 'util/proto_extensions.dart'; import 'utils.dart'; +import 'vm/concurrency.dart' if (dart.library.js) 'js/concurrency.dart'; +import 'vm/reusable_worker.dart' if (dart.library.js) 'js/reusable_worker.dart'; +import 'worker_entrypoint.dart' + if (dart.library.js) 'js/worker_entrypoint.dart'; -/// A class that dispatches messages between the host and various isolates that +/// A class that dispatches messages between the host and various workers that /// are each running an individual compilation. -class IsolateDispatcher { +class WorkerDispatcher { /// The channel of encoded protocol buffers, connected to the host. final StreamChannel _channel; - /// Whether to wait for all worker isolates to exit before exiting the main - /// isolate or not. + /// Whether to wait for all worker workers to exit before exiting the main + /// worker or not. final bool _gracefulShutdown; - /// All isolates that have been spawned to dispatch to. + /// All workers that have been spawned to dispatch to. /// /// Only used for cleaning up the process when the underlying channel closes. - final _allIsolates = StreamController(sync: true); + final _allWorkers = StreamController(sync: true); - /// The isolates that aren't currently running compilations - final _inactiveIsolates = {}; + /// The workers that aren't currently running compilations + final _inactiveWorkers = {}; - /// A map from active compilationIds to isolates running those compilations. - final _activeIsolates = >{}; + /// A map from active compilationIds to workers running those compilations. + final _activeWorkers = >{}; - /// A pool controlling how many isolates (and thus concurrent compilations) + /// A pool controlling how many workers (and thus concurrent compilations) /// may be live at once. - final _isolatePool = Pool(concurrencyLimit); + final _workerPool = Pool(concurrencyLimit); /// Whether [_channel] has been closed or not. var _closed = false; - IsolateDispatcher(this._channel, {bool gracefulShutdown = true}) + WorkerDispatcher(this._channel, {bool gracefulShutdown = true}) : _gracefulShutdown = gracefulShutdown; void listen() { @@ -58,16 +59,16 @@ class IsolateDispatcher { (compilationId, messageBuffer) = parsePacket(packet); if (compilationId != 0) { - var isolate = await _activeIsolates.putIfAbsent( + var worker = await _activeWorkers.putIfAbsent( compilationId, - () => _getIsolate(compilationId!), + () => _getWorker(compilationId!), ); - // The shutdown may have started by the time the isolate is spawned + // The shutdown may have started by the time the worker is spawned if (_closed) return; try { - isolate.send(packet); + worker.send(packet); return; } on StateError catch (_) { throw paramsError( @@ -108,7 +109,7 @@ class IsolateDispatcher { onDone: () { if (_gracefulShutdown) { _closed = true; - _allIsolates.stream.listen((isolate) => isolate.kill()); + _allWorkers.stream.listen((worker) => worker.kill()); } else { exit(exitCode); } @@ -116,36 +117,36 @@ class IsolateDispatcher { ); } - /// Returns an isolate that's ready to run a new compilation. + /// Returns an worker that's ready to run a new compilation. /// - /// This re-uses an existing isolate if possible, and spawns a new one + /// This re-uses an existing worker if possible, and spawns a new one /// otherwise. - Future _getIsolate(int compilationId) async { - var resource = await _isolatePool.request(); - ReusableIsolate isolate; - if (_inactiveIsolates.isNotEmpty) { - isolate = _inactiveIsolates.first; - _inactiveIsolates.remove(isolate); + Future _getWorker(int compilationId) async { + var resource = await _workerPool.request(); + ReusableWorker worker; + if (_inactiveWorkers.isNotEmpty) { + worker = _inactiveWorkers.first; + _inactiveWorkers.remove(worker); } else { - var future = ReusableIsolate.spawn( - isolateMain, + var future = ReusableWorker.spawn( + workerEntryPoint, onError: (Object error, StackTrace stackTrace) { _handleError(error, stackTrace); }, ); - isolate = await future; - _allIsolates.add(isolate); + worker = await future; + _allWorkers.add(worker); } - isolate.borrow((message) { + worker.borrow((message) { var fullBuffer = message as Uint8List; - // The first byte of messages from isolates indicates whether the entire + // The first byte of messages from workers indicates whether the entire // compilation is finished (1) or if it encountered an error (exitCode). // Sending this as part of the message buffer rather than a separate // message avoids a race condition where the host might send a new // compilation request with the same ID as one that just finished before - // the [IsolateDispatcher] receives word that the isolate with that ID is + // the [WorkerDispatcher] receives word that the worker with that ID is // done. See sass/dart-sass#2004. var category = fullBuffer[0]; var packet = Uint8List.sublistView(fullBuffer, 1); @@ -154,9 +155,9 @@ class IsolateDispatcher { case 0: _channel.sink.add(packet); case 1: - _activeIsolates.remove(compilationId); - isolate.release(); - _inactiveIsolates.add(isolate); + _activeWorkers.remove(compilationId); + worker.release(); + _inactiveWorkers.add(worker); resource.release(); _channel.sink.add(packet); default: @@ -170,7 +171,7 @@ class IsolateDispatcher { } }); - return isolate; + return worker; } /// Creates a [OutboundMessage_VersionResponse] diff --git a/lib/src/embedded/isolate_main.dart b/lib/src/embedded/worker_entrypoint.dart similarity index 82% rename from lib/src/embedded/isolate_main.dart rename to lib/src/embedded/worker_entrypoint.dart index 660b6724b..c79212910 100644 --- a/lib/src/embedded/isolate_main.dart +++ b/lib/src/embedded/worker_entrypoint.dart @@ -7,6 +7,6 @@ import 'dart:isolate' show SendPort; import 'compilation_dispatcher.dart'; import 'sync_receive_port.dart'; -void isolateMain(SyncReceivePort receivePort, SendPort sendPort) { +void workerEntryPoint(SyncReceivePort receivePort, SendPort sendPort) { CompilationDispatcher(receivePort, sendPort).listen(); } From fd74031f45f5940a5f93619bfc523b5692418b1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=AA=E3=81=A4=E3=81=8D?= Date: Tue, 1 Apr 2025 20:14:55 -0700 Subject: [PATCH 08/22] Add a comment for gracefulShutdown --- lib/src/embedded/vm/executable.dart | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/src/embedded/vm/executable.dart b/lib/src/embedded/vm/executable.dart index c6c62bb35..42a9e388f 100644 --- a/lib/src/embedded/vm/executable.dart +++ b/lib/src/embedded/vm/executable.dart @@ -12,6 +12,14 @@ import '../worker_dispatcher.dart'; void main(List args) { if (parseOptions(args)) { + // The option `gracefulShutdown: false` means exit the process immediately + // without waiting for child isolates to shutdown. Dart VM does not need to + // wait as it does not have race condition issues between communication and + // worker shutdown in NodeJS. + // + // Because `gracefulShutdown` only gracefully terminates the worker that it + // does not wait for inflight requests to finish, there is no behavior + // difference to the end users. WorkerDispatcher( StreamChannel.withGuarantees(stdin, stdout, allowSinkErrors: false) .transform(lengthDelimited), From 7514f298110ebc21c9e2da6f08a65882c45fcca0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=AA=E3=81=A4=E3=81=8D?= Date: Wed, 2 Apr 2025 10:37:59 -0700 Subject: [PATCH 09/22] Update README.md --- lib/src/embedded/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/src/embedded/README.md b/lib/src/embedded/README.md index b892e4818..7ce8b8188 100644 --- a/lib/src/embedded/README.md +++ b/lib/src/embedded/README.md @@ -11,12 +11,12 @@ protocol specification] for details. The embedded compiler has two different levels of dispatchers for handling incoming messages from the embedded host: -1. The [`IsolateDispatcher`] is the first recipient of each packet. It decodes +1. The [`WorkerDispatcher`] is the first recipient of each packet. It decodes the packets _just enough_ to determine which compilation they belong to, and forwards them to the appropriate compilation dispatcher. It also parses and handles messages that aren't compilation specific, such as `VersionRequest`. - [`IsolateDispatcher`]: isolate_dispatcher.dart + [`WorkerDispatcher`]: worker_dispatcher.dart 2. The [`CompilationDispatcher`] fully parses and handles messages for a single compilation. Each `CompilationDispatcher` runs in a separate isolate so that From 66437ca7c2d424b12bb0c758a8cf7723887bf623 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=AA=E3=81=A4=E3=81=8D?= Date: Wed, 2 Apr 2025 11:32:10 -0700 Subject: [PATCH 10/22] Add diagrams explaining DartVM and NodeJS worker management --- lib/src/embedded/README.md | 64 +++++++++++++++++++++++++++++++++++--- 1 file changed, 60 insertions(+), 4 deletions(-) diff --git a/lib/src/embedded/README.md b/lib/src/embedded/README.md index 7ce8b8188..c0f712d1d 100644 --- a/lib/src/embedded/README.md +++ b/lib/src/embedded/README.md @@ -1,9 +1,9 @@ # Embedded Sass Compiler This directory contains the Dart Sass embedded compiler. This is a special mode -of the Dart Sass command-line executable, only supported on the Dart VM, in -which it uses stdin and stdout to communicate with another endpoint, the -"embedded host", using a protocol buffer-based protocol. See [the embedded +of the Dart Sass command-line executable, only supported on the Dart VM and +Node.js, in which it uses stdin and stdout to communicate with another endpoint, +the "embedded host", using a protocol buffer-based protocol. See [the embedded protocol specification] for details. [the embedded protocol specification]: https://github.com/sass/sass/blob/main/spec/embedded-protocol.md @@ -24,5 +24,61 @@ incoming messages from the embedded host: [`CompilationDispatcher`]: compilation_dispatcher.dart -Otherwise, most of the code in this directory just wraps Dart APIs to +Otherwise, most of the code in this directory just wraps Dart APIs or JS APIs to communicate with their protocol buffer equivalents. + +## Worker Communication and Management + +The way Dart VM launches lightweight isolates versus Node.js launches worker +threads are very different. + +In Dart VM, the lightweight isolates shares program structures like loaded +libraries, classes, functions, etc., even including JIT optimized code. This +allows main isolate to spawn child isolate with a reference to the entry point +function. + +``` +┌─────────────────┐ ┌─────────────────┐ +│ Main Isolate │ Isolate.spawn(workerEntryPoint, mailbox, sendPort) │ Worker Isolate │ +│ ├───────────────────────────────────────────────────►│ │ +│ │ │ │ +│ ┌─────────────┐ │ Synchronous Messaging │ ┌─────────────┐ │ +│ │ Mailbox ├─┼────────────────────────────────────────────────────┼►│ Mailbox │ │ +│ └─────────────┘ │ │ └─────────────┘ │ +│ │ │ │ +│ ┌─────────────┐ │ Asynchronous Messaging │ ┌─────────────┐ │ +│ │ ReceivePort │◄┼────────────────────────────────────────────────────┼─┤ SendPort │ │ +│ └─────────────┘ │ │ └─────────────┘ │ +│ │ │ │ +└─────────────────┘ └─────────────────┘ +``` + +In Node.JS, the worker threads do not share program structures. In order to +launch a worker thread, it needs an entry point file, with the entry point +function effectly hard-coded in the entry point file. While it's possible +to have a separate entry point file for the worker threads, it's requires more +complex packaging changes with `cli_pkg`, therefore the main thread the worker +threads shares [the same entry point file](js/executable.dart), and the entry +point file will decide what to run depends on `worker_threads.isMainThread`. + +``` + if (worker_threads.isMainThread) { if (worker_threads.isMainThread) { + mainEntryPoint(); mainEntryPoint(); + } else { } else { + workerEntryPoint(); new Worker(process.argv[1], { workerEntryPoint(); + } argv: process.argv.slice(2), } + workerData: channel.port2, +┌────────────────────────────────────┐ transferList: [channel.port2] ┌────────────────────────────────────┐ +│ Main Thread │ }) │ Worker Thread │ +│ ├────────────────────────────────────────────────────────────►│ │ +│ │ │ │ +│ ┌────────────────────────────────┐ │ Synchronous Messaging │ ┌────────────────────────────────┐ │ +│ │ SyncMessagePort(channel.port1) ├─┼─────────────────────────────────────────────────────────────┼►│ SyncMessagePort(channel.port2) │ │ +│ └────────────────────────────────┘ │ │ └────────────────────────────────┘ │ +│ │ │ │ +│ ┌────────────────────────────────┐ │ Asynchronous Messaging │ ┌────────────────────┐ │ +│ │ channel.port1 │◄┼─────────────────────────────────────────────────────────────┼─┤ channel.port2 │ │ +│ └────────────────────────────────┘ │ │ └────────────────────┘ │ +│ │ │ │ +└────────────────────────────────────┘ └────────────────────────────────────┘ +``` From 931af8e5475a1aabb6f5eacc1625f60c9f13ca74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=AA=E3=81=A4=E3=81=8D?= Date: Wed, 2 Apr 2025 12:01:39 -0700 Subject: [PATCH 11/22] Update README.md --- lib/src/embedded/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/embedded/README.md b/lib/src/embedded/README.md index c0f712d1d..01fce995e 100644 --- a/lib/src/embedded/README.md +++ b/lib/src/embedded/README.md @@ -19,7 +19,7 @@ incoming messages from the embedded host: [`WorkerDispatcher`]: worker_dispatcher.dart 2. The [`CompilationDispatcher`] fully parses and handles messages for a single - compilation. Each `CompilationDispatcher` runs in a separate isolate so that + compilation. Each `CompilationDispatcher` runs in a separate worker so that the embedded compiler can run multiple compilations in parallel. [`CompilationDispatcher`]: compilation_dispatcher.dart From 3633df19f960686bfb43ccca87b8fd9855958aef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=AA=E3=81=A4=E3=81=8D?= Date: Wed, 2 Apr 2025 16:33:29 -0700 Subject: [PATCH 12/22] Use a separate byte to send exitCode --- lib/src/embedded/compilation_dispatcher.dart | 20 ++++++++++++-------- lib/src/embedded/worker_dispatcher.dart | 17 +++++++++++++---- 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/lib/src/embedded/compilation_dispatcher.dart b/lib/src/embedded/compilation_dispatcher.dart index 01580d9ca..86a641145 100644 --- a/lib/src/embedded/compilation_dispatcher.dart +++ b/lib/src/embedded/compilation_dispatcher.dart @@ -408,20 +408,24 @@ final class CompilationDispatcher { var protobufWriter = CodedBufferWriter(); message.writeToCodedBufferWriter(protobufWriter); - // Add one additional byte to the beginning to indicate whether or not the - // compilation has finished (1) or encountered a fatal error (exitCode), so - // the [IsolateDispatcher] knows whether to treat this isolate as inactive - // or close out entirely. + // Add two bytes to the beginning. + // + // The first byte indicates whether or not the compilation has finished (1) + // or encountered a fatal error (2), so the [WorkerDispatcher] knows + // whether to treat this isolate as inactive or close out entirely. + // + // The second byte is the exitCode when a fatal error occurs. var packet = Uint8List( - 1 + _compilationIdVarint.length + protobufWriter.lengthInBytes, + 2 + _compilationIdVarint.length + protobufWriter.lengthInBytes, ); packet[0] = switch (message.whichMessage()) { + OutboundMessage_Message.error => 2, OutboundMessage_Message.compileResponse => 1, - OutboundMessage_Message.error => exitCode, _ => 0 }; - packet.setAll(1, _compilationIdVarint); - protobufWriter.writeTo(packet, 1 + _compilationIdVarint.length); + packet[1] = exitCode; + packet.setAll(2, _compilationIdVarint); + protobufWriter.writeTo(packet, 2 + _compilationIdVarint.length); return packet; } diff --git a/lib/src/embedded/worker_dispatcher.dart b/lib/src/embedded/worker_dispatcher.dart index c96799dd4..c5ed9c997 100644 --- a/lib/src/embedded/worker_dispatcher.dart +++ b/lib/src/embedded/worker_dispatcher.dart @@ -142,14 +142,14 @@ class WorkerDispatcher { var fullBuffer = message as Uint8List; // The first byte of messages from workers indicates whether the entire - // compilation is finished (1) or if it encountered an error (exitCode). + // compilation is finished (1) or if it encountered an error (2). // Sending this as part of the message buffer rather than a separate // message avoids a race condition where the host might send a new // compilation request with the same ID as one that just finished before // the [WorkerDispatcher] receives word that the worker with that ID is // done. See sass/dart-sass#2004. var category = fullBuffer[0]; - var packet = Uint8List.sublistView(fullBuffer, 1); + var packet = Uint8List.sublistView(fullBuffer, 2); switch (category) { case 0: @@ -160,9 +160,18 @@ class WorkerDispatcher { _inactiveWorkers.add(worker); resource.release(); _channel.sink.add(packet); - default: + case 2: _channel.sink.add(packet); - exitCode = category; + // The second byte of message is the exitCode when fatal error + // occurs. This is needed because in Node.js process.exitCode + // is thread local, so that we need to pass it from the worker + // thread back to main thread. Using onexit event to retrieve + // the exitCode is unrelibale because worker.kill() might get + // triggered from main thread before the worker thread finish + // exit itself, in which case onexit event will recevie an exit + // code 1 regardless of actual process.exitCode value in worker + // thread. + exitCode = fullBuffer[1]; if (_gracefulShutdown) { _channel.sink.close(); } else { From cad7dfee5fc4d273fbef5d757aa1c42c8fb7b698 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=AA=E3=81=A4=E3=81=8D?= Date: Wed, 2 Apr 2025 16:43:51 -0700 Subject: [PATCH 13/22] Update README.md --- lib/src/embedded/README.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/src/embedded/README.md b/lib/src/embedded/README.md index 01fce995e..a0bf5b8a3 100644 --- a/lib/src/embedded/README.md +++ b/lib/src/embedded/README.md @@ -32,7 +32,7 @@ communicate with their protocol buffer equivalents. The way Dart VM launches lightweight isolates versus Node.js launches worker threads are very different. -In Dart VM, the lightweight isolates shares program structures like loaded +In Dart VM, the lightweight isolates share program structures like loaded libraries, classes, functions, etc., even including JIT optimized code. This allows main isolate to spawn child isolate with a reference to the entry point function. @@ -56,10 +56,10 @@ function. In Node.JS, the worker threads do not share program structures. In order to launch a worker thread, it needs an entry point file, with the entry point function effectly hard-coded in the entry point file. While it's possible -to have a separate entry point file for the worker threads, it's requires more -complex packaging changes with `cli_pkg`, therefore the main thread the worker -threads shares [the same entry point file](js/executable.dart), and the entry -point file will decide what to run depends on `worker_threads.isMainThread`. +to have a separate entry point file for the worker threads, it requires more +complex packaging changes with `cli_pkg`, therefore the main thread and the +worker threads share [the same entry point file](js/executable.dart), which +decides what to run based on `worker_threads.isMainThread`. ``` if (worker_threads.isMainThread) { if (worker_threads.isMainThread) { @@ -76,9 +76,9 @@ point file will decide what to run depends on `worker_threads.isMainThread`. │ │ SyncMessagePort(channel.port1) ├─┼─────────────────────────────────────────────────────────────┼►│ SyncMessagePort(channel.port2) │ │ │ └────────────────────────────────┘ │ │ └────────────────────────────────┘ │ │ │ │ │ -│ ┌────────────────────────────────┐ │ Asynchronous Messaging │ ┌────────────────────┐ │ -│ │ channel.port1 │◄┼─────────────────────────────────────────────────────────────┼─┤ channel.port2 │ │ -│ └────────────────────────────────┘ │ │ └────────────────────┘ │ +│ ┌────────────────────────────────┐ │ Asynchronous Messaging │ ┌────────────────────────────────┐ │ +│ │ channel.port1 │◄┼─────────────────────────────────────────────────────────────┼─┤ channel.port2 │ │ +│ └────────────────────────────────┘ │ │ └────────────────────────────────┘ │ │ │ │ │ └────────────────────────────────────┘ └────────────────────────────────────┘ ``` From 659e27e6799c49a322cfd2f06b32606b8c40d69e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=AA=E3=81=A4=E3=81=8D?= Date: Wed, 2 Apr 2025 17:57:21 -0700 Subject: [PATCH 14/22] Reflow a comment --- lib/src/embedded/worker_dispatcher.dart | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/src/embedded/worker_dispatcher.dart b/lib/src/embedded/worker_dispatcher.dart index c5ed9c997..2562303cd 100644 --- a/lib/src/embedded/worker_dispatcher.dart +++ b/lib/src/embedded/worker_dispatcher.dart @@ -142,12 +142,12 @@ class WorkerDispatcher { var fullBuffer = message as Uint8List; // The first byte of messages from workers indicates whether the entire - // compilation is finished (1) or if it encountered an error (2). - // Sending this as part of the message buffer rather than a separate - // message avoids a race condition where the host might send a new - // compilation request with the same ID as one that just finished before - // the [WorkerDispatcher] receives word that the worker with that ID is - // done. See sass/dart-sass#2004. + // compilation is finished (1) or if it encountered an error (2). Sending + // this as part of the message buffer rather than a separate message + // avoids a race condition where the host might send a new compilation + // request with the same ID as one that just finished before the + // [WorkerDispatcher] receives word that the worker with that ID is done. + // See sass/dart-sass#2004. var category = fullBuffer[0]; var packet = Uint8List.sublistView(fullBuffer, 2); From a8e2fa07e31f300e182657b6eb562ca75594c3cb Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Mon, 7 Apr 2025 15:05:52 -0700 Subject: [PATCH 15/22] Grammar/formatting changes to the embedded README --- lib/src/embedded/README.md | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/lib/src/embedded/README.md b/lib/src/embedded/README.md index a0bf5b8a3..8d82ba346 100644 --- a/lib/src/embedded/README.md +++ b/lib/src/embedded/README.md @@ -29,13 +29,11 @@ communicate with their protocol buffer equivalents. ## Worker Communication and Management -The way Dart VM launches lightweight isolates versus Node.js launches worker -threads are very different. - -In Dart VM, the lightweight isolates share program structures like loaded -libraries, classes, functions, etc., even including JIT optimized code. This -allows main isolate to spawn child isolate with a reference to the entry point -function. +The way the Dart VM launches lightweight isolates is very different from how +Node.js launches worker threads. In the Dart VM, the lightweight isolates share +program structures like loaded libraries, classes, functions, and so on, even +including JIT optimized code. This allows main isolate to spawn child isolate +with a reference to the entry point function. ``` ┌─────────────────┐ ┌─────────────────┐ @@ -55,21 +53,21 @@ function. In Node.JS, the worker threads do not share program structures. In order to launch a worker thread, it needs an entry point file, with the entry point -function effectly hard-coded in the entry point file. While it's possible -to have a separate entry point file for the worker threads, it requires more -complex packaging changes with `cli_pkg`, therefore the main thread and the -worker threads share [the same entry point file](js/executable.dart), which -decides what to run based on `worker_threads.isMainThread`. +function effectly hard-coded in that file. While it's possible to have a +separate entry point file for the worker threads, it would require complex +packaging changes within `cli_pkg`, so instead the main thread and the worker +threads share [the same entry point file](js/executable.dart), which decides +what to run based on `worker_threads.isMainThread`. ``` if (worker_threads.isMainThread) { if (worker_threads.isMainThread) { mainEntryPoint(); mainEntryPoint(); } else { } else { - workerEntryPoint(); new Worker(process.argv[1], { workerEntryPoint(); - } argv: process.argv.slice(2), } - workerData: channel.port2, -┌────────────────────────────────────┐ transferList: [channel.port2] ┌────────────────────────────────────┐ -│ Main Thread │ }) │ Worker Thread │ + workerEntryPoint(); new Worker(process.argv[1], { workerEntryPoint(); + } argv: process.argv.slice(2), } + workerData: channel.port2, +┌────────────────────────────────────┐ transferList: [channel.port2] ┌────────────────────────────────────┐ +│ Main Thread │ }) │ Worker Thread │ │ ├────────────────────────────────────────────────────────────►│ │ │ │ │ │ │ ┌────────────────────────────────┐ │ Synchronous Messaging │ ┌────────────────────────────────┐ │ From b27cbbb16f350ffa242a0f6ba5e81c62a96f0b40 Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Mon, 7 Apr 2025 15:28:20 -0700 Subject: [PATCH 16/22] Make the two ReusableWorker interfaces match --- lib/src/embedded/js/executable.dart | 4 +-- lib/src/embedded/js/reusable_worker.dart | 17 ++++++------- lib/src/embedded/js/worker_entrypoint.dart | 14 ----------- lib/src/embedded/vm/reusable_worker.dart | 29 ++++++---------------- lib/src/embedded/worker_dispatcher.dart | 3 --- lib/src/embedded/worker_entrypoint.dart | 12 --------- 6 files changed, 17 insertions(+), 62 deletions(-) delete mode 100644 lib/src/embedded/js/worker_entrypoint.dart delete mode 100644 lib/src/embedded/worker_entrypoint.dart diff --git a/lib/src/embedded/js/executable.dart b/lib/src/embedded/js/executable.dart index 363af460a..b55b231cc 100644 --- a/lib/src/embedded/js/executable.dart +++ b/lib/src/embedded/js/executable.dart @@ -4,10 +4,10 @@ import 'package:stream_channel/stream_channel.dart'; +import '../compilation_dispatcher.dart'; import '../options.dart'; import '../util/length_delimited_transformer.dart'; import '../worker_dispatcher.dart'; -import '../worker_entrypoint.dart'; import 'io.dart'; import 'sync_receive_port.dart'; import 'worker_threads.dart'; @@ -21,7 +21,7 @@ void main(List args) { .listen(); } else { var port = workerData! as MessagePort; - workerEntryPoint(JSSyncReceivePort(port), JSSendPort(port)); + CompilationDispatcher(JSSyncReceivePort(port), JSSendPort(port)).listen(); } } } diff --git a/lib/src/embedded/js/reusable_worker.dart b/lib/src/embedded/js/reusable_worker.dart index 4d81deb3d..1630ff9ec 100644 --- a/lib/src/embedded/js/reusable_worker.dart +++ b/lib/src/embedded/js/reusable_worker.dart @@ -6,15 +6,12 @@ import 'dart:async'; import 'dart:js_interop'; import 'dart:typed_data'; +import 'package:node_interop/node_interop.dart'; + import 'js.dart'; import 'sync_message_port.dart'; import 'worker_threads.dart'; -/// The entrypoint for a [ReusableWorker]. -/// -/// This must return a Record of filename and argv for creating the Worker. -typedef ReusableWorkerEntryPoint = (String, JSArray) Function(); - class ReusableWorker { /// The worker. final Worker _worker; @@ -34,17 +31,17 @@ class ReusableWorker { ReusableWorker._( this._worker, this._sendPort, this._receivePort, this._subscription); - /// Spawns a [ReusableWorker] that runs the the entrypoint script. - static Future spawn(ReusableWorkerEntryPoint entryPoint, - {Function? onError}) async { - var (filename, argv) = entryPoint(); + /// Spawns a [ReusableWorker]. + static Future spawn({Function? onError}) async { + var filename = process.argv[1] as String; + var argv = [for (var arg in process.argv.skip(2)) (arg as String).toJS]; var channel = SyncMessagePort.createChannel(); var worker = Worker( filename, WorkerOptions( workerData: channel.port2, transferList: [channel.port2].toJS, - argv: argv)); + argv: argv.toJS)); var controller = StreamController(sync: true); var sendPort = SyncMessagePort(channel.port1); var receivePort = channel.port1; diff --git a/lib/src/embedded/js/worker_entrypoint.dart b/lib/src/embedded/js/worker_entrypoint.dart deleted file mode 100644 index 7541b59c7..000000000 --- a/lib/src/embedded/js/worker_entrypoint.dart +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright 2025 Google Inc. Use of this source code is governed by an -// MIT-style license that can be found in the LICENSE file or at -// https://opensource.org/licenses/MIT. - -import 'dart:js_interop'; - -import 'js.dart'; - -@JS('process.argv') -external JSArray get _argv; - -(String, JSArray) workerEntryPoint() { - return ((_argv[1]! as JSString).toDart, _argv.slice(2) as JSArray); -} diff --git a/lib/src/embedded/vm/reusable_worker.dart b/lib/src/embedded/vm/reusable_worker.dart index 510869acc..afb753606 100644 --- a/lib/src/embedded/vm/reusable_worker.dart +++ b/lib/src/embedded/vm/reusable_worker.dart @@ -10,18 +10,7 @@ import 'package:native_synchronization/mailbox.dart'; import 'package:native_synchronization/sendable.dart'; import '../sync_receive_port.dart'; - -/// The entrypoint for a [ReusableWorker]. -/// -/// This must be a static global function. It's run when the isolate is spawned, -/// and is passed a [Mailbox] that receives messages from [ReusableWorker.send] -/// and a [SendPort] that sends messages to the [ReceivePort] listened by -/// [ReusableWorker.borrow]. -/// -/// If the [sendPort] sends a message before [ReusableWorker.borrow] is called, -/// this will throw an unhandled [StateError]. -typedef ReusableWorkerEntryPoint = FutureOr Function( - SyncReceivePort receivePort, SendPort sendPort); +import '../compilation_dispatcher.dart'; class ReusableWorker { /// The wrapped isolate. @@ -46,15 +35,13 @@ class ReusableWorker { Function? onError, }) : _subscription = _receivePort.listen(_defaultOnData, onError: onError); - /// Spawns a [ReusableWorker] that runs the given [entryPoint]. - static Future spawn( - ReusableWorkerEntryPoint entryPoint, { + /// Spawns a [ReusableWorker]. + static Future spawn({ Function? onError, }) async { var mailbox = Mailbox(); var receivePort = ReceivePort(); var isolate = await Isolate.spawn(_isolateMain, ( - entryPoint, mailbox.asSendable, receivePort.sendPort, )); @@ -107,9 +94,9 @@ void _defaultOnData(dynamic _) { throw StateError("Shouldn't receive a message before being borrowed."); } -void _isolateMain( - (ReusableWorkerEntryPoint, Sendable, SendPort) message, -) { - var (entryPoint, sendableMailbox, sendPort) = message; - entryPoint(MailboxSyncReceivePort(sendableMailbox.materialize()), sendPort); +void _isolateMain((Sendable, SendPort) message) { + var (sendableMailbox, sendPort) = message; + CompilationDispatcher( + MailboxSyncReceivePort(sendableMailbox.materialize()), sendPort) + .listen(); } diff --git a/lib/src/embedded/worker_dispatcher.dart b/lib/src/embedded/worker_dispatcher.dart index 2562303cd..3eda448b7 100644 --- a/lib/src/embedded/worker_dispatcher.dart +++ b/lib/src/embedded/worker_dispatcher.dart @@ -15,8 +15,6 @@ import 'util/proto_extensions.dart'; import 'utils.dart'; import 'vm/concurrency.dart' if (dart.library.js) 'js/concurrency.dart'; import 'vm/reusable_worker.dart' if (dart.library.js) 'js/reusable_worker.dart'; -import 'worker_entrypoint.dart' - if (dart.library.js) 'js/worker_entrypoint.dart'; /// A class that dispatches messages between the host and various workers that /// are each running an individual compilation. @@ -129,7 +127,6 @@ class WorkerDispatcher { _inactiveWorkers.remove(worker); } else { var future = ReusableWorker.spawn( - workerEntryPoint, onError: (Object error, StackTrace stackTrace) { _handleError(error, stackTrace); }, diff --git a/lib/src/embedded/worker_entrypoint.dart b/lib/src/embedded/worker_entrypoint.dart deleted file mode 100644 index c79212910..000000000 --- a/lib/src/embedded/worker_entrypoint.dart +++ /dev/null @@ -1,12 +0,0 @@ -// Copyright 2025 Google Inc. Use of this source code is governed by an -// MIT-style license that can be found in the LICENSE file or at -// https://opensource.org/licenses/MIT. - -import 'dart:isolate' show SendPort; - -import 'compilation_dispatcher.dart'; -import 'sync_receive_port.dart'; - -void workerEntryPoint(SyncReceivePort receivePort, SendPort sendPort) { - CompilationDispatcher(receivePort, sendPort).listen(); -} From d134ad48e13627b41dc96b4d0b946df1d0d35310 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=AA=E3=81=A4=E3=81=8D?= Date: Mon, 7 Apr 2025 17:09:25 -0700 Subject: [PATCH 17/22] Condense graph in README.md --- lib/src/embedded/README.md | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/lib/src/embedded/README.md b/lib/src/embedded/README.md index 8d82ba346..bb4b16310 100644 --- a/lib/src/embedded/README.md +++ b/lib/src/embedded/README.md @@ -60,23 +60,23 @@ threads share [the same entry point file](js/executable.dart), which decides what to run based on `worker_threads.isMainThread`. ``` - if (worker_threads.isMainThread) { if (worker_threads.isMainThread) { - mainEntryPoint(); mainEntryPoint(); - } else { } else { - workerEntryPoint(); new Worker(process.argv[1], { workerEntryPoint(); - } argv: process.argv.slice(2), } - workerData: channel.port2, -┌────────────────────────────────────┐ transferList: [channel.port2] ┌────────────────────────────────────┐ -│ Main Thread │ }) │ Worker Thread │ -│ ├────────────────────────────────────────────────────────────►│ │ -│ │ │ │ -│ ┌────────────────────────────────┐ │ Synchronous Messaging │ ┌────────────────────────────────┐ │ -│ │ SyncMessagePort(channel.port1) ├─┼─────────────────────────────────────────────────────────────┼►│ SyncMessagePort(channel.port2) │ │ -│ └────────────────────────────────┘ │ │ └────────────────────────────────┘ │ -│ │ │ │ -│ ┌────────────────────────────────┐ │ Asynchronous Messaging │ ┌────────────────────────────────┐ │ -│ │ channel.port1 │◄┼─────────────────────────────────────────────────────────────┼─┤ channel.port2 │ │ -│ └────────────────────────────────┘ │ │ └────────────────────────────────┘ │ -│ │ │ │ -└────────────────────────────────────┘ └────────────────────────────────────┘ + if (worker_threads.isMainThread) { if (worker_threads.isMainThread) { + mainEntryPoint(); mainEntryPoint(); + } else { } else { + workerEntryPoint(); new Worker(process.argv[1], { workerEntryPoint(); + } argv: process.argv.slice(2), } + workerData: channel.port2, +┌────────────────────────────────────┐ transferList: [channel.port2] ┌────────────────────────────────────┐ +│ Main Thread │ }) │ Worker Thread │ +│ ├───────────────────────────────────────────────►│ │ +│ │ │ │ +│ ┌────────────────────────────────┐ │ Synchronous Messaging │ ┌────────────────────────────────┐ │ +│ │ SyncMessagePort(channel.port1) ├─┼────────────────────────────────────────────────┼►│ SyncMessagePort(channel.port2) │ │ +│ └────────────────────────────────┘ │ │ └────────────────────────────────┘ │ +│ │ │ │ +│ ┌────────────────────────────────┐ │ Asynchronous Messaging │ ┌────────────────────────────────┐ │ +│ │ channel.port1 │◄┼────────────────────────────────────────────────┼─┤ channel.port2 │ │ +│ └────────────────────────────────┘ │ │ └────────────────────────────────┘ │ +│ │ │ │ +└────────────────────────────────────┘ └────────────────────────────────────┘ ``` From a0fc4404a485974ef92c3466c1beb139afa6d338 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=AA=E3=81=A4=E3=81=8D?= Date: Mon, 7 Apr 2025 17:12:50 -0700 Subject: [PATCH 18/22] Apply review feedbacks --- lib/src/embedded/js/executable.dart | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/src/embedded/js/executable.dart b/lib/src/embedded/js/executable.dart index b55b231cc..2e6244e40 100644 --- a/lib/src/embedded/js/executable.dart +++ b/lib/src/embedded/js/executable.dart @@ -13,15 +13,15 @@ import 'sync_receive_port.dart'; import 'worker_threads.dart'; void main(List args) { - if (parseOptions(args)) { - if (isMainThread) { + if (isMainThread) { + if (parseOptions(args)) { WorkerDispatcher(StreamChannel.withGuarantees(stdin, stdout, allowSinkErrors: false) .transform(lengthDelimited)) .listen(); - } else { - var port = workerData! as MessagePort; - CompilationDispatcher(JSSyncReceivePort(port), JSSendPort(port)).listen(); } + } else { + var port = workerData! as MessagePort; + CompilationDispatcher(JSSyncReceivePort(port), JSSendPort(port)).listen(); } } From 4729acab2c8f5455a3c47214b0e3d18cb1c0d35e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=AA=E3=81=A4=E3=81=8D?= Date: Mon, 7 Apr 2025 17:15:23 -0700 Subject: [PATCH 19/22] Apply review feedbacks --- lib/src/embedded/README.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/src/embedded/README.md b/lib/src/embedded/README.md index bb4b16310..2e1863c6a 100644 --- a/lib/src/embedded/README.md +++ b/lib/src/embedded/README.md @@ -1,10 +1,9 @@ # Embedded Sass Compiler This directory contains the Dart Sass embedded compiler. This is a special mode -of the Dart Sass command-line executable, only supported on the Dart VM and -Node.js, in which it uses stdin and stdout to communicate with another endpoint, -the "embedded host", using a protocol buffer-based protocol. See [the embedded -protocol specification] for details. +of the Dart Sass command-line executable, in which it uses stdin and stdout to +communicate with another endpoint, the "embedded host", using a protocol +buffer-based protocol. See [the embedded protocol specification] for details. [the embedded protocol specification]: https://github.com/sass/sass/blob/main/spec/embedded-protocol.md From 627e522ae05ab1cbcc84e5321fa891cd62d13066 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=AA=E3=81=A4=E3=81=8D?= Date: Mon, 7 Apr 2025 17:28:29 -0700 Subject: [PATCH 20/22] Apply review feedbacks --- lib/src/embedded/compilation_dispatcher.dart | 8 +++----- lib/src/embedded/js/isolate.dart | 17 ----------------- lib/src/embedded/js/sync_receive_port.dart | 2 +- lib/src/io/interface.dart | 6 ++++++ lib/src/io/js.dart | 8 ++++++++ lib/src/io/vm.dart | 5 +++++ 6 files changed, 23 insertions(+), 23 deletions(-) delete mode 100644 lib/src/embedded/js/isolate.dart diff --git a/lib/src/embedded/compilation_dispatcher.dart b/lib/src/embedded/compilation_dispatcher.dart index 86a641145..460de6d2b 100644 --- a/lib/src/embedded/compilation_dispatcher.dart +++ b/lib/src/embedded/compilation_dispatcher.dart @@ -3,8 +3,6 @@ // https://opensource.org/licenses/MIT. import 'dart:convert'; -import 'dart:io' if (dart.library.js) 'js/io.dart'; -import 'dart:isolate' if (dart.library.js) 'js/isolate.dart'; import 'dart:typed_data'; import 'package:path/path.dart' as p; @@ -13,7 +11,7 @@ import 'package:pub_semver/pub_semver.dart'; import 'package:sass/sass.dart' as sass; import 'package:sass/src/importer/node_package.dart' as npi; -import '../io.dart' show FileSystemException; +import '../io.dart'; import '../logger.dart'; import '../value/function.dart'; import '../value/mixin.dart'; @@ -306,7 +304,7 @@ final class CompilationDispatcher { /// /// This is used during compilation by other classes like host callable. Never sendError(ProtocolError error) { - Isolate.exit(_sendPort, _serializePacket(OutboundMessage()..error = error)); + exitWorker(_sendPort, _serializePacket(OutboundMessage()..error = error)); } InboundMessage_CanonicalizeResponse sendCanonicalizeRequest( @@ -436,7 +434,7 @@ final class CompilationDispatcher { } on StateError catch (_) { // The [SyncReceivePort] has been closed, exit the current isolate immediately // to avoid bubble the error up as [SassException] during [_sendRequest]. - Isolate.exit(); + exitWorker(); } } } diff --git a/lib/src/embedded/js/isolate.dart b/lib/src/embedded/js/isolate.dart deleted file mode 100644 index d4bdc8f0e..000000000 --- a/lib/src/embedded/js/isolate.dart +++ /dev/null @@ -1,17 +0,0 @@ -// Copyright 2025 Google LLC. Use of this source code is governed by an -// MIT-style license that can be found in the LICENSE file or at -// https://opensource.org/licenses/MIT. - -import 'dart:isolate' show SendPort; -export 'dart:isolate' show SendPort; - -import 'io.dart' as io; - -abstract class Isolate { - static Never exit([SendPort? finalMessagePort, Object? message]) { - if (message != null) { - finalMessagePort?.send(message); - } - io.exit(io.exitCode) as Never; - } -} diff --git a/lib/src/embedded/js/sync_receive_port.dart b/lib/src/embedded/js/sync_receive_port.dart index 0f8097c35..19a3d80f7 100644 --- a/lib/src/embedded/js/sync_receive_port.dart +++ b/lib/src/embedded/js/sync_receive_port.dart @@ -6,7 +6,7 @@ import 'dart:js_interop'; import 'dart:typed_data'; import '../sync_receive_port.dart'; -import 'isolate.dart'; +import '../../io.dart'; import 'js.dart'; import 'sync_message_port.dart'; import 'worker_threads.dart'; diff --git a/lib/src/io/interface.dart b/lib/src/io/interface.dart index 55754ddc3..104f58958 100644 --- a/lib/src/io/interface.dart +++ b/lib/src/io/interface.dart @@ -2,6 +2,9 @@ // MIT-style license that can be found in the LICENSE file or at // https://opensource.org/licenses/MIT. +import 'dart:isolate' show SendPort; +export 'dart:isolate' show SendPort; + import 'package:watcher/watcher.dart'; /// An error thrown by [readFile]. @@ -89,6 +92,9 @@ String? getEnvironmentVariable(String name) => throw ''; int get exitCode => throw ''; set exitCode(int value) => throw ''; +/// Exit the current dart isolate or nodejs thread +Never exitWorker([SendPort? finalMessagePort, Object? message]) => throw ''; + /// Recursively watches the directory at [path] for modifications. /// /// Returns a future that completes with a single-subscription stream once the diff --git a/lib/src/io/js.dart b/lib/src/io/js.dart index be850d871..cc29d66ae 100644 --- a/lib/src/io/js.dart +++ b/lib/src/io/js.dart @@ -4,6 +4,7 @@ import 'dart:async'; import 'dart:convert'; +import 'dart:isolate' show SendPort; import 'package:cli_pkg/js.dart'; import 'package:js/js.dart'; @@ -285,6 +286,13 @@ int get exitCode => _process?.exitCode ?? 0; set exitCode(int code) => _process?.exitCode = code; +Never exitWorker([SendPort? finalMessagePort, Object? message]) { + if (message != null) { + finalMessagePort?.send(message); + } + _process?.exit(exitCode) as Never; +} + Future> watchDir(String path, {bool poll = false}) async { if (!isNodeJs) { throw UnsupportedError("watchDir() is only supported on Node.js"); diff --git a/lib/src/io/vm.dart b/lib/src/io/vm.dart index b82a36ed8..b64158e26 100644 --- a/lib/src/io/vm.dart +++ b/lib/src/io/vm.dart @@ -5,6 +5,7 @@ import 'dart:async'; import 'dart:convert'; import 'dart:io' as io; +import 'dart:isolate'; import 'package:async/async.dart'; import 'package:path/path.dart' as p; @@ -103,6 +104,10 @@ DateTime modificationTime(String path) { String? getEnvironmentVariable(String name) => io.Platform.environment[name]; +Never exitWorker([SendPort? finalMessagePort, Object? message]) { + Isolate.exit(finalMessagePort, message); +} + Future> watchDir(String path, {bool poll = false}) async { var watcher = poll ? PollingDirectoryWatcher(path) : DirectoryWatcher(path); From c97645985f38711d724390a1b277686fe8730040 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=AA=E3=81=A4=E3=81=8D?= Date: Mon, 7 Apr 2025 17:35:43 -0700 Subject: [PATCH 21/22] Apply review feedbacks --- lib/src/io/js.dart | 1 + lib/src/io/vm.dart | 1 + 2 files changed, 2 insertions(+) diff --git a/lib/src/io/js.dart b/lib/src/io/js.dart index cc29d66ae..4b3d07b41 100644 --- a/lib/src/io/js.dart +++ b/lib/src/io/js.dart @@ -5,6 +5,7 @@ import 'dart:async'; import 'dart:convert'; import 'dart:isolate' show SendPort; +export 'dart:isolate' show SendPort; import 'package:cli_pkg/js.dart'; import 'package:js/js.dart'; diff --git a/lib/src/io/vm.dart b/lib/src/io/vm.dart index b64158e26..168f32545 100644 --- a/lib/src/io/vm.dart +++ b/lib/src/io/vm.dart @@ -6,6 +6,7 @@ import 'dart:async'; import 'dart:convert'; import 'dart:io' as io; import 'dart:isolate'; +export 'dart:isolate' show SendPort; import 'package:async/async.dart'; import 'package:path/path.dart' as p; From ff957ec9b4a3a7eb7007b0988eea82e255603909 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=AA=E3=81=A4=E3=81=8D?= Date: Mon, 7 Apr 2025 17:41:27 -0700 Subject: [PATCH 22/22] Apply review feedbacks --- lib/src/embedded/sync_receive_port.dart | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/src/embedded/sync_receive_port.dart b/lib/src/embedded/sync_receive_port.dart index 9be0477c0..c15b2a63a 100644 --- a/lib/src/embedded/sync_receive_port.dart +++ b/lib/src/embedded/sync_receive_port.dart @@ -6,8 +6,10 @@ import 'dart:typed_data'; export 'vm/sync_receive_port.dart' if (dart.library.js) 'js/sync_receive_port.dart'; -/// A common interface that is implemented by wrapping -/// Dart Mailbox or JS SyncMessagePort. +/// A port that receives message synchronously across workers. abstract interface class SyncReceivePort { + /// Receives a message from the port. + /// + /// Throws [StateError] if called after port has been closed. Uint8List receive(); }