sass --embedded race condition in deactivating isolates

[ntkme]

There is a race condition due to the deactivation of isolation (free up a wire id) being an async callback.

In a normal flow this is what should happen.

1. Host send compilation request id=1
2. Compiler activate isolate id=1
3. Compiler send compilation response id=1
4. Compiler deactivate isolate id=1
5. Host send compilation request id=1
6. ...

However, in the test I discovered that because the deactivation happens after sending response on a event loop rather than immediately after, there is a race condition:

1. Host send compilation request id=1
2. Compiler activate isolate id=1
3. Compiler send compilation response id=1
4. Host send compilation request id=1
5. Compiler deactivate isolate id=1
6. ???

It is completely legit for host to reuse id=1 as soon as it receives the response. However, a new request with the same id may land before the hook to deactivate isolate finishes, which causes host side lockup because it will never get a response.

[ntkme]

A workaround for now is to keep increasing id and not reuse at all.

[ntkme]

It was very difficult to reproduce on good hardware. However, by adding a sleep for 1 second like below, this race condition is consistently reproducible on any hardware.

diff --git a/lib/src/embedded/dispatcher.dart b/lib/src/embedded/dispatcher.dart
index 0e9c6899..8c0d6fb8 100644
--- a/lib/src/embedded/dispatcher.dart
+++ b/lib/src/embedded/dispatcher.dart
@@ -94,6 +94,9 @@ class Dispatcher {
             var response = await _compile(request);
             _send(OutboundMessage()..compileResponse = response);
             success = true;
+
+            sleep(Duration(seconds:1));
+
             // Each Dispatcher runs a single compilation and then closes.
             _channel.sink.close();
 

[nex3]

Thanks for tracking this down! I'll take a look at it tomorrow.

[nex3]

This is deceptively tricky: because the isolate dispatcher doesn't have any visibility into what the isolates are saying, it doesn't know that it just sent a compile response until it receives the follow-up message from the isolate channel indicating that it's ready to close. We could choose to always queue up messages for active isolates, but then we wouldn't be able to validate when hosts are actually setting an incorrect wire ID.

We could handle this by having each outbound message from the dispatcher have a "close this channel" boolean as well, but it's possible that we could also refactor the isolate dispatcher to keep a single channel alive and give the inner isolate responsibility for determining when it's alive or not.

[ntkme]

Logically what needs to happen in order is:

1. Child isolate send the compilation response to main isolate.
2. Main isolate hold the message and deactivate the child isolate.
3. Main isolate send the compilation response to the host.