Skip to content

Remove deferring from protocolFilter#10257

Closed
Colengms wants to merge 2 commits intomainfrom
coleng/remove_queuing_from_protocol_filter
Closed

Remove deferring from protocolFilter#10257
Colengms wants to merge 2 commits intomainfrom
coleng/remove_queuing_from_protocol_filter

Conversation

@Colengms
Copy link
Contributor

@Colengms Colengms commented Dec 8, 2022

A more complete fix for: #10227

An issue had occurred with didChange, as the contentChanges field of the TextDocumentChangeEvent can become inconsistent with the document field when re-evaluated late.

This change removes most deferring from protocolFilter. There are still await's that occur prior to processing didOpen, but that seems to be OK - it should be valid to process the document passed to didOpen at any point. (The native code will discard any edits that are received for versions prior to the one received from didOpen, or when the file is not yet known to be open).

This fix is based on the assumption that there are no otherwise unhandled order dependencies that must be enforced for these messages. The original implementation was designed to fundamental ensure all messages are processed serially, and this is removing code that ensured that. But, so far, I've been unable to identify any issues with these messages potentially occurring out of order relative to internal messages. These messages all represent actions the user has already performed, operations they want to initiate, etc., that could occur at any time, and therefore their ordering relative to internal messages is already indeterminate. When edits are involved or could invalidate an operation, we should also already be using the document version to verify the operation and discarding the results of the operation if versions do not match.

@michelleangela
Copy link
Contributor

Does protocalFilter.ts file need comments to explain certain conditions or special cases to consider the next time any logic in this file needs to be changed in the future?

@Colengms
Copy link
Contributor Author

@Colengms Colengms closed this Dec 21, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Feb 4, 2023
@Colengms Colengms deleted the coleng/remove_queuing_from_protocol_filter branch July 2, 2024 01:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants