sass / dart-sass

The reference implementation of Sass, written in Dart.
https://sass-lang.com/dart-sass
MIT License
3.87k stars 350 forks source link

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

Closed bep closed 2 weeks ago

bep commented 1 month ago

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

This looks like a regression as the tests

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

See https://github.com/bep/godartsass/issues/26

ntkme commented 1 month ago

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:

https://github.com/sass/dart-sass/blob/d4b19397c493adb6e518ed5344913aa2615c11f6/test/embedded/importer_test.dart#L24-L40

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 commented 1 month ago

We have regression test for https://github.com/sass/dart-sass/issues/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 commented 1 month ago

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

https://github.com/sass/dart-sass/blob/d4b19397c493adb6e518ed5344913aa2615c11f6/test/embedded/protocol_test.dart#L237-L248

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 commented 1 month ago

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?