Skip to content

Commit f7c9899

Browse files
keertipCommit Queue
authored andcommitted
[Message scheduler] Implement handling one message at a time.
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]>
1 parent 2d8aec0 commit f7c9899

16 files changed

+812
-142
lines changed

pkg/analysis_server/lib/src/analysis_server.dart

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,14 +282,17 @@ abstract class AnalysisServer {
282282
DartFixPromptManager? dartFixPromptManager,
283283
this.providedByteStore,
284284
PluginManager? pluginManager,
285+
bool retainDataForTesting = false,
285286
}) : resourceProvider = OverlayResourceProvider(baseResourceProvider),
286287
pubApi = PubApi(
287288
instrumentationService,
288289
httpClient,
289290
Platform.environment['PUB_HOSTED_URL'],
290291
),
291292
producerGeneratorsForLintRules = AssistProcessor.computeLintRuleMap(),
292-
messageScheduler = MessageScheduler() {
293+
messageScheduler = MessageScheduler(
294+
testView:
295+
retainDataForTesting ? MessageSchedulerTestView() : null) {
293296
messageScheduler.setServer(this);
294297
// Set the default URI converter. This uses the resource providers path
295298
// context (unlike the initialized value) which allows tests to override it.

pkg/analysis_server/lib/src/legacy_analysis_server.dart

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,7 @@ class LegacyAnalysisServer extends AnalysisServer {
387387
DartFixPromptManager? dartFixPromptManager,
388388
super.providedByteStore,
389389
super.pluginManager,
390+
bool retainDataForTesting = false,
390391
}) : lspClientConfiguration = lsp.LspClientConfiguration(
391392
baseResourceProvider.pathContext,
392393
),
@@ -404,6 +405,7 @@ class LegacyAnalysisServer extends AnalysisServer {
404405
requestStatistics: requestStatistics,
405406
enableBlazeWatcher: enableBlazeWatcher,
406407
dartFixPromptManager: dartFixPromptManager,
408+
retainDataForTesting: retainDataForTesting,
407409
) {
408410
var contextManagerCallbacks = ServerContextManagerCallbacks(
409411
this,
@@ -548,8 +550,13 @@ class LegacyAnalysisServer extends AnalysisServer {
548550
sendStatusNotificationNew(status);
549551
}
550552

551-
/// Handle a [request] that was read from the communication channel.
552-
void handleRequest(Request request, CancelableToken? cancellationToken) {
553+
/// Handle a [request] that was read from the communication channel. The completer
554+
/// is used to indicate when the request handling is done.
555+
void handleRequest(
556+
Request request,
557+
Completer<void> completer,
558+
CancelableToken? cancellationToken,
559+
) {
553560
var startTime = DateTime.now();
554561
performance.logRequestTiming(request.clientRequestTime);
555562

@@ -594,6 +601,7 @@ class LegacyAnalysisServer extends AnalysisServer {
594601
ServerRecentPerformance.slowRequestsThreshold) {
595602
recentPerformance.slowRequests.add(requestPerformance!);
596603
}
604+
completer.complete();
597605
},
598606
(exception, stackTrace) {
599607
if (exception is InconsistentAnalysisException) {
@@ -620,6 +628,7 @@ class LegacyAnalysisServer extends AnalysisServer {
620628
var response = Response(request.id, error: error);
621629
sendResponse(response);
622630
}
631+
completer.complete();
623632
},
624633
);
625634
}
@@ -635,7 +644,6 @@ class LegacyAnalysisServer extends AnalysisServer {
635644
cancellationToken: cancellationToken,
636645
),
637646
);
638-
messageScheduler.notify();
639647
} else if (requestOrResponse is Response) {
640648
handleResponse(requestOrResponse);
641649
}

pkg/analysis_server/lib/src/lsp/handlers/handler_rename.dart

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,11 @@ class RenameHandler extends LspMessageHandler<RenameParams, WorkspaceEdit?> {
219219
return error(ServerErrorCodes.RenameNotValid, finalStatus.message!);
220220
}
221221

222+
// Set the completer to complete to show that request is paused, and
223+
// that processing of incoming messages can continue while we wait
224+
// for the user's response.
225+
message.completer?.complete();
226+
222227
// Otherwise, ask the user whether to proceed with the rename.
223228
var userChoice = await prompt(
224229
MessageType.warning,

pkg/analysis_server/lib/src/lsp/handlers/handlers.dart

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,11 @@ class MessageInfo {
440440
/// May be null for messages sent prior to initialization.
441441
final LspClientCapabilities? clientCapabilities;
442442

443+
/// The completer used to indicate that the handler is paused waiting on user
444+
/// response. The completer, if any, is set to complete before a dialog is
445+
/// shown.
446+
final Completer<void>? completer;
447+
443448
MessageInfo({
444449
required this.performance,
445450
// TODO(dantup): Consider a version of this that has a non-nullable
@@ -448,6 +453,7 @@ class MessageInfo {
448453
// to run without, so it would remove a bunch of boilerplate in the others.
449454
required this.clientCapabilities,
450455
this.timeSinceRequest,
456+
this.completer,
451457
});
452458
}
453459

@@ -496,6 +502,9 @@ abstract class ServerStateMessageHandler {
496502
// will need to specifically check the token after `await`s.
497503
cancellationToken ??= cancelHandler.createToken(message);
498504
try {
505+
if (cancellationToken.isCancellationRequested) {
506+
return cancelled(cancellationToken);
507+
}
499508
var result = await handler.handleMessage(
500509
message,
501510
messageInfo,

pkg/analysis_server/lib/src/lsp/lsp_analysis_server.dart

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,7 @@ class LspAnalysisServer extends AnalysisServer {
163163
// Disable to avoid using this in unit tests.
164164
bool enableBlazeWatcher = false,
165165
DartFixPromptManager? dartFixPromptManager,
166+
bool retainDataForTesting = false,
166167
}) : lspClientConfiguration = LspClientConfiguration(
167168
baseResourceProvider.pathContext,
168169
),
@@ -179,6 +180,7 @@ class LspAnalysisServer extends AnalysisServer {
179180
LspNotificationManager(baseResourceProvider.pathContext),
180181
enableBlazeWatcher: enableBlazeWatcher,
181182
dartFixPromptManager: dartFixPromptManager,
183+
retainDataForTesting: retainDataForTesting,
182184
) {
183185
notificationManager.server = this;
184186
messageHandler = UninitializedStateMessageHandler(this);
@@ -495,8 +497,15 @@ class LspAnalysisServer extends AnalysisServer {
495497
);
496498
}
497499

498-
/// Handle a [message] that was read from the communication channel.
499-
void handleMessage(Message message, {CancelableToken? cancellationToken}) {
500+
/// Handle a [message] that was read from the communication channel. The
501+
/// [completer] is used to indicate when the [RequestMessage] handling is
502+
/// done. [ResponseMessage] and [NotificationMessage]'s are not waited on
503+
/// till finish.
504+
void handleMessage(
505+
Message message,
506+
Completer<void>? completer, {
507+
CancelableToken? cancellationToken,
508+
}) {
500509
var startTime = DateTime.now();
501510
performance.logRequestTiming(message.clientRequestTime);
502511
runZonedGuarded(() async {
@@ -520,6 +529,7 @@ class LspAnalysisServer extends AnalysisServer {
520529
performance: performance,
521530
clientCapabilities: editorClientCapabilities,
522531
timeSinceRequest: message.timeSinceRequest,
532+
completer: completer,
523533
);
524534

525535
if (message is RequestMessage) {
@@ -551,6 +561,9 @@ class LspAnalysisServer extends AnalysisServer {
551561
} else {
552562
showErrorMessageToUser('Unknown message type');
553563
}
564+
if (completer != null && !completer.isCompleted) {
565+
completer.complete();
566+
}
554567
} on InconsistentAnalysisException {
555568
sendErrorResponse(
556569
message,
@@ -559,6 +572,9 @@ class LspAnalysisServer extends AnalysisServer {
559572
message: 'Document was modified before operation completed',
560573
),
561574
);
575+
if (completer != null && !completer.isCompleted) {
576+
completer.complete();
577+
}
562578
} catch (error, stackTrace) {
563579
var errorMessage =
564580
message is ResponseMessage
@@ -576,6 +592,9 @@ class LspAnalysisServer extends AnalysisServer {
576592
),
577593
);
578594
logException(errorMessage, error, stackTrace);
595+
if (completer != null && !completer.isCompleted) {
596+
completer.complete();
597+
}
579598
}
580599
}, socketError);
581600
}
@@ -815,7 +834,6 @@ class LspAnalysisServer extends AnalysisServer {
815834
messageScheduler.add(
816835
LspMessage(message: message, cancellationToken: cancellationToken),
817836
);
818-
messageScheduler.notify();
819837
}
820838

821839
void sendErrorResponse(Message message, ResponseError error) {

0 commit comments

Comments
 (0)