sass --embedded doesn't reliably exit when closing stdin on Windows

[bep]

This looks like the same issue as #2001, but now on Windows.

This looks like a regression as the tests

• Pass in 1.63.2
• Fails in 1.77.5

Note that I don't have av Windows development environment, so I cannot easily Git bisect.

See bep/godartsass#26

[ntkme]

We have regression test for #2001 in CI and it has never failed. Can you describe the exactly test case you're running?

The only known flaky test in dart-sass CI test is this one:

dart-sass/test/embedded/importer_test.dart

Lines 24 to 40 in d4b1939

	test("for a response without a corresponding request ID", () async {
	process.send(compileString("@import 'other'", importers: [
	InboundMessage_CompileRequest_Importer()..importerId = 1
	]));
	var request = await getCanonicalizeRequest(process);
	process.send(InboundMessage()
	..canonicalizeResponse =
	(InboundMessage_CanonicalizeResponse()..id = request.id + 1));
	await expectParamsError(
	process,
	errorId,
	"Response ID ${request.id + 1} doesn't match any outstanding "
	"requests in compilation $defaultCompilationId.");
	await process.shouldExit(76);
	});

That we found only on windows it rarely fails to shutdown after a protocol error.

❌ test\embedded\importer_test.dart: emits a protocol error for a response without a corresponding request ID (failed)
  
  Process `dart_sass_embedded` was killed with SIGKILL in a tear-down. Output:
      canonicalizeRequest: {
        id: 0
        importerId: 1
        url: other
        fromImport: true
      }
      
  [e] Host caused params error: Response ID 1 doesn't match any outstanding requests in compilation 4321.
      error: {
        type: PARAMS
        id: 4294967295
        message: Response ID 1 doesn't match any outstanding requests in compilation 4321.
      }
  TimeoutException after 0:00:30.000000: Test timed out after 30 seconds. See https://pub.dev/packages/test#timeouts
  dart:isolate  _RawReceivePort._handleMessage
❌ test\embedded\importer_test.dart: emits a protocol error for a response without a corresponding request ID (failed after test completion)
  Expected: <76>
    Actual: <-1>
  Process `dart_sass_embedded` had an unexpected exit code.
  
  package:matcher                            expect
  test\embedded\embedded_process.dart 215:5  EmbeddedProcess.shouldExit
  ===== asynchronous gap ===========================
  test\embedded\importer_test.dart 39:7      main.<fn>.<fn>

[bep]

We have regression test for #2001 in CI and it has never failed.

Note that this only fails on Windows this time. Last time it was the other way around, *nix only.

Can you describe the exactly test case you're running?

This is the test that fails (on Windows only):

https://github.com/bep/godartsass/blob/2c31e1a7b9a697799c2a0a312ac6095fb9203404/transpiler_test.go#L286

I remember we had a discussion the last time if the above was a realistic test (closing with inflight Sass compilations), and that may be a valid objection. This is mainly a robustness test of my RPC implementation, and I would expect the above to behave sensibly.

[ntkme]

We have a similar test on dart-sass but I've never seen this one failing:

dart-sass/test/embedded/protocol_test.dart

Lines 237 to 248 in d4b1939

	test("closes gracefully with many in-flight compilations", () async {
	// This should always be equal to the size of
	// [IsolateDispatcher._isolatePool], since that's as many concurrent
	// compilations as we can realistically have anyway.
	var totalRequests = 15;
	for (var i = 1; i <= totalRequests; i++) {
	process.inbound
	.add((i, compileString("a {b: foo() + 2px}", functions: [r"foo()"])));
	}
	await process.close();
	});

This is slightly different than your test: it submits multiple requests with a fake function call that never get a response so that these inflight requests would just hang waiting on the function call response. Then, we close the compiler by close stdin.

In your test it looks like you're just sending normal requests without letting them hang on the dart side, I think this might have caused the state of isolate to be slightly different.

[ntkme]

I tried this on the ruby host, and there is a high chance dart vm would get stuck during the shutdown on macos. This is not necessarily a "windows-only" issue.

# frozen_string_literal: true

require 'bundler/inline'

gemfile do
  source 'https://rubygems.org'
  gem 'sass-embedded'
end

sass = Sass::Compiler.new

puts sass.info

threads = []
10.times do |i|
  4.times do |j|
    threads << Thread.new do
      sass.close if i + j == 10
      sass.compile_string("$primary-color: ##{format('%03d', i + j)}; div { color: $primary-color; }").css
    rescue IOError
      # expected after close
    end
  end
end
threads.each(&:join)

If it works, this script should just exits with in one second, however, lots of times it hangs for a few seconds, and then dart vm start to print the message about non-responding isolates.

Note: .../connection.rb:58: warning: is just a prefix added by ruby host to identify logs from dart process' stderr.

sass-embedded	1.77.5	(Embedded Host)	[Ruby]
dart-sass	1.77.5	(Sass Compiler)	[Dart]
/opt/homebrew/lib/ruby/gems/3.3.0/gems/sass-embedded-1.77.5-arm64-darwin/lib/sass/compiler/connection.rb:58: warning: Attempt:11 waiting for isolate vm-isolate to check in
/opt/homebrew/lib/ruby/gems/3.3.0/gems/sass-embedded-1.77.5-arm64-darwin/lib/sass/compiler/connection.rb:58: warning: Attempt:11 waiting for isolate _isolateMain to check in
/opt/homebrew/lib/ruby/gems/3.3.0/gems/sass-embedded-1.77.5-arm64-darwin/lib/sass/compiler/connection.rb:58: warning: Attempt:11 waiting for isolate _isolateMain to check in
/opt/homebrew/lib/ruby/gems/3.3.0/gems/sass-embedded-1.77.5-arm64-darwin/lib/sass/compiler/connection.rb:58: warning: Attempt:12 waiting for isolate vm-isolate to check in
/opt/homebrew/lib/ruby/gems/3.3.0/gems/sass-embedded-1.77.5-arm64-darwin/lib/sass/compiler/connection.rb:58: warning: Attempt:12 waiting for isolate _isolateMain to check in
/opt/homebrew/lib/ruby/gems/3.3.0/gems/sass-embedded-1.77.5-arm64-darwin/lib/sass/compiler/connection.rb:58: warning: Attempt:12 waiting for isolate _isolateMain to check in
/opt/homebrew/lib/ruby/gems/3.3.0/gems/sass-embedded-1.77.5-arm64-darwin/lib/sass/compiler/connection.rb:58: warning: Attempt:13 waiting for isolate vm-isolate to check in
/opt/homebrew/lib/ruby/gems/3.3.0/gems/sass-embedded-1.77.5-arm64-darwin/lib/sass/compiler/connection.rb:58: warning: Attempt:13 waiting for isolate _isolateMain to check in
/opt/homebrew/lib/ruby/gems/3.3.0/gems/sass-embedded-1.77.5-arm64-darwin/lib/sass/compiler/connection.rb:58: warning: Attempt:13 waiting for isolate _isolateMain to check in
/opt/homebrew/lib/ruby/gems/3.3.0/gems/sass-embedded-1.77.5-arm64-darwin/lib/sass/compiler/connection.rb:58: warning: Attempt:14 waiting for isolate vm-isolate to check in
/opt/homebrew/lib/ruby/gems/3.3.0/gems/sass-embedded-1.77.5-arm64-darwin/lib/sass/compiler/connection.rb:58: warning: Attempt:14 waiting for isolate _isolateMain to check in
/opt/homebrew/lib/ruby/gems/3.3.0/gems/sass-embedded-1.77.5-arm64-darwin/lib/sass/compiler/connection.rb:58: warning: Attempt:14 waiting for isolate _isolateMain to check in
/opt/homebrew/lib/ruby/gems/3.3.0/gems/sass-embedded-1.77.5-arm64-darwin/lib/sass/compiler/connection.rb:58: warning: Attempt:15 waiting for isolate vm-isolate to check in
/opt/homebrew/lib/ruby/gems/3.3.0/gems/sass-embedded-1.77.5-arm64-darwin/lib/sass/compiler/connection.rb:58: warning: Attempt:15 waiting for isolate _isolateMain to check in
/opt/homebrew/lib/ruby/gems/3.3.0/gems/sass-embedded-1.77.5-arm64-darwin/lib/sass/compiler/connection.rb:58: warning: Attempt:15 waiting for isolate _isolateMain to check in
/opt/homebrew/lib/ruby/gems/3.3.0/gems/sass-embedded-1.77.5-arm64-darwin/lib/sass/compiler/connection.rb:58: warning: Attempt:16 waiting for isolate vm-isolate to check in
/opt/homebrew/lib/ruby/gems/3.3.0/gems/sass-embedded-1.77.5-arm64-darwin/lib/sass/compiler/connection.rb:58: warning: Attempt:16 waiting for isolate _isolateMain to check in
/opt/homebrew/lib/ruby/gems/3.3.0/gems/sass-embedded-1.77.5-arm64-darwin/lib/sass/compiler/connection.rb:58: warning: Attempt:16 waiting for isolate _isolateMain to check in
/opt/homebrew/lib/ruby/gems/3.3.0/gems/sass-embedded-1.77.5-arm64-darwin/lib/sass/compiler/connection.rb:58: warning: Attempt:17 waiting for isolate vm-isolate to check in
/opt/homebrew/lib/ruby/gems/3.3.0/gems/sass-embedded-1.77.5-arm64-darwin/lib/sass/compiler/connection.rb:58: warning: Attempt:17 waiting for isolate _isolateMain to check in
/opt/homebrew/lib/ruby/gems/3.3.0/gems/sass-embedded-1.77.5-arm64-darwin/lib/sass/compiler/connection.rb:58: warning: Attempt:17 waiting for isolate _isolateMain to check in

@bep Any chance you can get the stderr of dart process piped and printed out to the console, to see if it looks like what I'm getting?