Skip to content

Commit

Permalink
Fix flaky isolate shutdown
Browse files Browse the repository at this point in the history
  • Loading branch information
ntkme committed Aug 17, 2024
1 parent 9d45fad commit 564ea71
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 24 deletions.
32 changes: 11 additions & 21 deletions lib/src/embedded/compilation_dispatcher.dart
Original file line number Diff line number Diff line change
Expand Up @@ -51,24 +51,18 @@ final class CompilationDispatcher {
/// This is used in outgoing messages.
late Uint8List _compilationIdVarint;

/// Whether we detected a [ProtocolError] while parsing an incoming response.
///
/// If we have, we don't want to send the final compilation result because
/// it'll just be a wrapper around the error.
var _requestError = false;

/// Creates a [CompilationDispatcher] that receives encoded protocol buffers
/// through [_mailbox] and sends them through [_sendPort].
CompilationDispatcher(this._mailbox, this._sendPort);

/// Listens for incoming `CompileRequests` and runs their compilations.
void listen() {
do {
while (true) {
Uint8List packet;
try {
packet = _mailbox.take();
} on StateError catch (_) {
break;
Isolate.exit();
}

try {
Expand All @@ -88,9 +82,7 @@ final class CompilationDispatcher {
case InboundMessage_Message.compileRequest:
var request = message.compileRequest;
var response = _compile(request);
if (!_requestError) {
_send(OutboundMessage()..compileResponse = response);
}
_send(OutboundMessage()..compileResponse = response);

case InboundMessage_Message.versionRequest:
throw paramsError("VersionRequest must have compilation ID 0.");
Expand All @@ -112,8 +104,10 @@ final class CompilationDispatcher {
}
} catch (error, stackTrace) {
_handleError(error, stackTrace);
_mailbox.close();
Isolate.exit();
}
} while (!_requestError);
}
}

OutboundMessage_CompileResponse _compile(
Expand Down Expand Up @@ -290,11 +284,10 @@ final class CompilationDispatcher {
/// Sends [error] to the host.
///
/// This is used during compilation by other classes like host callable.
/// Therefore it must set _requestError = true to prevent sending a CompileFailure after
/// sending a ProtocolError.
void sendError(ProtocolError error) {
_sendError(error);
_requestError = true;
_mailbox.close();
Isolate.exit();
}

/// Sends [error] to the host.
Expand Down Expand Up @@ -330,10 +323,7 @@ final class CompilationDispatcher {
try {
packet = _mailbox.take();
} on StateError catch (_) {
// Compiler is shutting down, throw without calling `_handleError` as we
// don't want to report this as an actual error.
_requestError = true;
rethrow;
Isolate.exit();
}

try {
Expand Down Expand Up @@ -376,8 +366,8 @@ final class CompilationDispatcher {
return response;
} catch (error, stackTrace) {
_handleError(error, stackTrace);
_requestError = true;
rethrow;
_mailbox.close();
Isolate.exit();
}
}

Expand Down
5 changes: 2 additions & 3 deletions lib/src/embedded/reusable_isolate.dart
Original file line number Diff line number Diff line change
Expand Up @@ -118,12 +118,11 @@ class ReusableIsolate {

/// Shuts down the isolate.
void kill() {
_isolate.kill();
_receivePort.close();

// If the isolate is blocking on [Mailbox.take], it won't even process a
// kill event, so we closed the mailbox to nofity and wake it up.
_mailbox.close();
_receivePort.close();
_isolate.kill(priority: Isolate.immediate);
}
}

Expand Down

0 comments on commit 564ea71

Please sign in to comment.