Use separate thread to handle didChangeWatchedFiles events #2643
Conversation
|
Looks like we are now using a consumer thread handling the event and make it 'non-blocking'. Compare to the previous blocking strategy, will this change impact on the result of the following arriving LSP request? |
The main task in didChangeWatchedFiles is to sync the resource tree with the local file system and update the project configuration if a build file is updated. The project update is already in a separate workspace job, so it is not affected by this PR. The remaining question is the impact of delaying the resource tree refresh. I will assess its impact case by case. The didChangeWatchedFiles events can come from two typical use cases. The first use case is when you edit and save a file in VS Code. In this case, the Java model has been updated in the document lifecycle handler and its contents are visible to other actions. So delaying the response to the didChangeWatchedFiles event should not be a problem. The second use case is when the file events are from external operations, such as git checkout branch. This does not affect the document lifecycle operations like didOpen/didChange, because the client will always send the latest contents to the server in these document notifications. For other operations like code completion, you may get some outdated proposals if the related resource is not refreshed yet. The question we need to consider is the balance between responsiveness and reliability. For me, it is better to respond with something than nothing. |
See #2518 (comment), executing the file event handler in the default dispatch thread may block the jsonrpc reader. To avoid the deadlock, use a separate thread to handle didChangeWatchedFiles events.