Fix a race condition preventing embedded compiler to shutdown after a protocol error

[ntkme]

There is an extreme rare race condition during compiler shutdown after a protocol error is send from host callable here:

dart-sass/lib/src/embedded/host_callable.dart

Line 50 in 16b8512

dispatcher.sendError(handleError(error, stackTrace));

A reproduction looks like this:

require 'sass-embedded'

def try
  puts yield
rescue StandardError => e
  puts e
end

fn = lambda do |_args|
  # this is an invalid calculation that will trigger a protocol error
  Sass::Value::Calculation.send(:new, 'foo', [Sass::Value::Number.new(1)])
end

try do
  Sass.compile_string('a {b: foo()}',
                      functions: { 'foo()': fn }).css
end

try do
  Sass.compile_string('a {b: c}').css
end

The fundamental problem here is that StreamSink#close() is async operation which can take time if the system is slow. This gives us a chance to send a second message before compiler has been shutdown.

The sequence of events looks like:

1. Host send first request that triggers a protocol error in host callable response.
2. Compiler send ProtocolError to host, however it forget to set _requestError = true in this case.
3. Host send another request, and this request would be accepted because nothing has been cleaned up yet.
4. Because _requestError is not set, the thrown ProtocolError bubbles up as a CompileFailure.
5. Compiler send CompileFailure for the first request, and this would set _checkedOut = false on the isolate.
6. Compiler send CompilerSuccess for the second request, and get Unhandled exception: Bad state: Shouldn't receive a message before being checked out.

The compiler would get stuck after the unhandled exception. The fix here is to make sure _requestError = true is set to that it does not send an extra CompilerFailure later which would reset isolate state at incorrect timing.

Slow sink close can be emulated with the following patch to make it easy to reproduce:

diff --git a/lib/src/embedded/isolate_dispatcher.dart b/lib/src/embedded/isolate_dispatcher.dart
index 044e2b9c..b58732f6 100644
--- a/lib/src/embedded/isolate_dispatcher.dart
+++ b/lib/src/embedded/isolate_dispatcher.dart
@@ -118,7 +118,9 @@ class IsolateDispatcher {
         // isolate, so we just send them as-is and close out the underlying
         // channel.
         sendError(compilationId, error);
-        _channel.sink.close();
+        Timer(Duration(milliseconds: 100), () =>
+          _channel.sink.close()
+        );
       } else {
         _handleError(error, stackTrace);
       }

[nex3]

Thanks!