Skip to content

Commit

Permalink
[Message scheduler] Implement handling one message at a time.
Browse files Browse the repository at this point in the history
Change-Id: I2294a7f209414a499fce05c0180358ec271349fe
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/386581
Commit-Queue: Keerti Parthasarathy <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
  • Loading branch information
keertip authored and Commit Queue committed Nov 22, 2024
1 parent 2d8aec0 commit f7c9899
Show file tree
Hide file tree
Showing 16 changed files with 812 additions and 142 deletions.
5 changes: 4 additions & 1 deletion pkg/analysis_server/lib/src/analysis_server.dart
Original file line number Diff line number Diff line change
Expand Up @@ -282,14 +282,17 @@ abstract class AnalysisServer {
DartFixPromptManager? dartFixPromptManager,
this.providedByteStore,
PluginManager? pluginManager,
bool retainDataForTesting = false,
}) : resourceProvider = OverlayResourceProvider(baseResourceProvider),
pubApi = PubApi(
instrumentationService,
httpClient,
Platform.environment['PUB_HOSTED_URL'],
),
producerGeneratorsForLintRules = AssistProcessor.computeLintRuleMap(),
messageScheduler = MessageScheduler() {
messageScheduler = MessageScheduler(
testView:
retainDataForTesting ? MessageSchedulerTestView() : null) {
messageScheduler.setServer(this);
// Set the default URI converter. This uses the resource providers path
// context (unlike the initialized value) which allows tests to override it.
Expand Down
14 changes: 11 additions & 3 deletions pkg/analysis_server/lib/src/legacy_analysis_server.dart
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,7 @@ class LegacyAnalysisServer extends AnalysisServer {
DartFixPromptManager? dartFixPromptManager,
super.providedByteStore,
super.pluginManager,
bool retainDataForTesting = false,
}) : lspClientConfiguration = lsp.LspClientConfiguration(
baseResourceProvider.pathContext,
),
Expand All @@ -404,6 +405,7 @@ class LegacyAnalysisServer extends AnalysisServer {
requestStatistics: requestStatistics,
enableBlazeWatcher: enableBlazeWatcher,
dartFixPromptManager: dartFixPromptManager,
retainDataForTesting: retainDataForTesting,
) {
var contextManagerCallbacks = ServerContextManagerCallbacks(
this,
Expand Down Expand Up @@ -548,8 +550,13 @@ class LegacyAnalysisServer extends AnalysisServer {
sendStatusNotificationNew(status);
}

/// Handle a [request] that was read from the communication channel.
void handleRequest(Request request, CancelableToken? cancellationToken) {
/// Handle a [request] that was read from the communication channel. The completer
/// is used to indicate when the request handling is done.
void handleRequest(
Request request,
Completer<void> completer,
CancelableToken? cancellationToken,
) {
var startTime = DateTime.now();
performance.logRequestTiming(request.clientRequestTime);

Expand Down Expand Up @@ -594,6 +601,7 @@ class LegacyAnalysisServer extends AnalysisServer {
ServerRecentPerformance.slowRequestsThreshold) {
recentPerformance.slowRequests.add(requestPerformance!);
}
completer.complete();
},
(exception, stackTrace) {
if (exception is InconsistentAnalysisException) {
Expand All @@ -620,6 +628,7 @@ class LegacyAnalysisServer extends AnalysisServer {
var response = Response(request.id, error: error);
sendResponse(response);
}
completer.complete();
},
);
}
Expand All @@ -635,7 +644,6 @@ class LegacyAnalysisServer extends AnalysisServer {
cancellationToken: cancellationToken,
),
);
messageScheduler.notify();
} else if (requestOrResponse is Response) {
handleResponse(requestOrResponse);
}
Expand Down
5 changes: 5 additions & 0 deletions pkg/analysis_server/lib/src/lsp/handlers/handler_rename.dart
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,11 @@ class RenameHandler extends LspMessageHandler<RenameParams, WorkspaceEdit?> {
return error(ServerErrorCodes.RenameNotValid, finalStatus.message!);
}

// Set the completer to complete to show that request is paused, and
// that processing of incoming messages can continue while we wait
// for the user's response.
message.completer?.complete();

// Otherwise, ask the user whether to proceed with the rename.
var userChoice = await prompt(
MessageType.warning,
Expand Down
9 changes: 9 additions & 0 deletions pkg/analysis_server/lib/src/lsp/handlers/handlers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,11 @@ class MessageInfo {
/// May be null for messages sent prior to initialization.
final LspClientCapabilities? clientCapabilities;

/// The completer used to indicate that the handler is paused waiting on user
/// response. The completer, if any, is set to complete before a dialog is
/// shown.
final Completer<void>? completer;

MessageInfo({
required this.performance,
// TODO(dantup): Consider a version of this that has a non-nullable
Expand All @@ -448,6 +453,7 @@ class MessageInfo {
// to run without, so it would remove a bunch of boilerplate in the others.
required this.clientCapabilities,
this.timeSinceRequest,
this.completer,
});
}

Expand Down Expand Up @@ -496,6 +502,9 @@ abstract class ServerStateMessageHandler {
// will need to specifically check the token after `await`s.
cancellationToken ??= cancelHandler.createToken(message);
try {
if (cancellationToken.isCancellationRequested) {
return cancelled(cancellationToken);
}
var result = await handler.handleMessage(
message,
messageInfo,
Expand Down
24 changes: 21 additions & 3 deletions pkg/analysis_server/lib/src/lsp/lsp_analysis_server.dart
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ class LspAnalysisServer extends AnalysisServer {
// Disable to avoid using this in unit tests.
bool enableBlazeWatcher = false,
DartFixPromptManager? dartFixPromptManager,
bool retainDataForTesting = false,
}) : lspClientConfiguration = LspClientConfiguration(
baseResourceProvider.pathContext,
),
Expand All @@ -179,6 +180,7 @@ class LspAnalysisServer extends AnalysisServer {
LspNotificationManager(baseResourceProvider.pathContext),
enableBlazeWatcher: enableBlazeWatcher,
dartFixPromptManager: dartFixPromptManager,
retainDataForTesting: retainDataForTesting,
) {
notificationManager.server = this;
messageHandler = UninitializedStateMessageHandler(this);
Expand Down Expand Up @@ -495,8 +497,15 @@ class LspAnalysisServer extends AnalysisServer {
);
}

/// Handle a [message] that was read from the communication channel.
void handleMessage(Message message, {CancelableToken? cancellationToken}) {
/// Handle a [message] that was read from the communication channel. The
/// [completer] is used to indicate when the [RequestMessage] handling is
/// done. [ResponseMessage] and [NotificationMessage]'s are not waited on
/// till finish.
void handleMessage(
Message message,
Completer<void>? completer, {
CancelableToken? cancellationToken,
}) {
var startTime = DateTime.now();
performance.logRequestTiming(message.clientRequestTime);
runZonedGuarded(() async {
Expand All @@ -520,6 +529,7 @@ class LspAnalysisServer extends AnalysisServer {
performance: performance,
clientCapabilities: editorClientCapabilities,
timeSinceRequest: message.timeSinceRequest,
completer: completer,
);

if (message is RequestMessage) {
Expand Down Expand Up @@ -551,6 +561,9 @@ class LspAnalysisServer extends AnalysisServer {
} else {
showErrorMessageToUser('Unknown message type');
}
if (completer != null && !completer.isCompleted) {
completer.complete();
}
} on InconsistentAnalysisException {
sendErrorResponse(
message,
Expand All @@ -559,6 +572,9 @@ class LspAnalysisServer extends AnalysisServer {
message: 'Document was modified before operation completed',
),
);
if (completer != null && !completer.isCompleted) {
completer.complete();
}
} catch (error, stackTrace) {
var errorMessage =
message is ResponseMessage
Expand All @@ -576,6 +592,9 @@ class LspAnalysisServer extends AnalysisServer {
),
);
logException(errorMessage, error, stackTrace);
if (completer != null && !completer.isCompleted) {
completer.complete();
}
}
}, socketError);
}
Expand Down Expand Up @@ -815,7 +834,6 @@ class LspAnalysisServer extends AnalysisServer {
messageScheduler.add(
LspMessage(message: message, cancellationToken: cancellationToken),
);
messageScheduler.notify();
}

void sendErrorResponse(Message message, ResponseError error) {
Expand Down
Loading

0 comments on commit f7c9899

Please sign in to comment.