Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Race condition in resolving workspace configuration #189138

Closed
vaclavHala opened this issue Jul 28, 2023 · 6 comments · Fixed by #191051
Closed

Race condition in resolving workspace configuration #189138

vaclavHala opened this issue Jul 28, 2023 · 6 comments · Fixed by #191051
Assignees
Labels
api author-verification-requested Issues potentially verifiable by issue author bug Issue identified by VS Code Team member as probable bug config VS Code configuration, set up issues confirmed Issue has been confirmed by VS Code Team member file-io File I/O insiders-released Patch has been released in VS Code Insiders verified Verification succeeded workbench-multiroot Multi-root (multiple folders) issues
Milestone

Comments

@vaclavHala
Copy link

Sometimes in multi-folder workspace the vscode.workspace.workspaceFolders will return empty array even though there are some folders in the workspace.

Does this issue occur when all extensions are disabled?: Yes

  • VS Code Version:
    Version: 1.79.2
    Commit: 695af09
    Date: 2023-06-14T08:59:55.818Z
    Electron: 22.5.7
    Chromium: 108.0.5359.215
    Node.js: 16.17.1
    V8: 10.8.168.25-electron.0
    OS: Linux x64 4.19.0-18-amd64

happens with latest insiders too

  • OS Version:
    Operating System: Debian GNU/Linux 10 (buster)
    Kernel: Linux 4.19.0-18-amd64
    Architecture: x86-64

Steps to Reproduce:

  1. clone this extension demonstrating the problem https://github.com/vaclavHala/vscode-workspace-race/tree/master/workspace-race
  2. run the extension from vscode
  3. add exactly 3 folders to the workspace (the bug manifests for any number of folders as long as you already have a multifolder workspace, 3 are required for the repro just for simplicity)
  4. run command Run workspace race
  5. after some time error will appear meaning the bug has manifested, see debugger log in the development vscode from which you launched the repro or in developer tools directly in the runtime vscode
  6. in the log you will see some sequence of expected messages alternating between 2/3 folders in the workspace, the last message will be an empty array

Sometimes the bug manifests after a few seconds, sometimes it may take up to a few minutes.

Note I'm always waiting for vscode.workspace.onDidChangeWorkspaceFolders to fire before touching the workspace again after any update, as demonstrated in the repro

@Tyriar Tyriar assigned bpasero and unassigned Tyriar Jul 28, 2023
@bpasero bpasero added api workbench-multiroot Multi-root (multiple folders) issues confirmed Issue has been confirmed by VS Code Team member bug Issue identified by VS Code Team member as probable bug labels Jul 28, 2023
@bpasero bpasero added this to the August 2023 milestone Jul 31, 2023
@bpasero bpasero added the config VS Code configuration, set up issues label Jul 31, 2023
@bpasero
Copy link
Member

bpasero commented Jul 31, 2023

@vaclavHala thanks for the issue, I can confirm.

@sandy081 this seems to be a race condition in how the configuration service reacts to changes to the config file. There is a scheduler that reads delayed after a change:

this.reloadConfigurationScheduler = this._register(new RunOnceScheduler(() => this.reloadConfiguration(), 50));

I think what happens here is a race condition where you manage to read the file when it has been truncated to an empty file but before writing the new contents. As such you consider the workspace as empty.

For this reason, we have atomic option for reading files, I had thought this was adopted in configuration land, but I think only for vscode-userdata schemes but not file scheme:

return this.fileSystemProvider.readFile(this.toFileSystemResource(resource), { atomic: true });

Here the configuration comes from a workspace file, thus scheme is likely file.

Bottom line: all reads from configuration service should pass atomic: true

@bpasero bpasero changed the title Sometimes in multi-folder workspace workspace.workspaceFolders returns empty array Race condition in resolving workspace configuration Jul 31, 2023
@bpasero bpasero added the file-io File I/O label Jul 31, 2023
sandy081 added a commit that referenced this issue Aug 23, 2023
sandy081 added a commit that referenced this issue Aug 23, 2023
* fix #189138

* feedback
@VSCodeTriageBot VSCodeTriageBot added unreleased Patch has not yet been released in VS Code Insiders insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Aug 23, 2023
@bpasero bpasero added the author-verification-requested Issues potentially verifiable by issue author label Aug 28, 2023
@VSCodeTriageBot
Copy link
Collaborator

This bug has been fixed in the latest release of VS Code Insiders!

@vaclavHala, you can help us out by commenting /verified if things are now working as expected.

If things still don't seem right, please ensure you're on version f7973f3 of Insiders (today's or later - you can use Help: About in the command palette to check), and leave a comment letting us know what isn't working as expected.

Happy Coding!

@roblourens roblourens added the verified Verification succeeded label Aug 31, 2023
@vaclavHala
Copy link
Author

Should this fix be present in insiders build from rev f1302be1e67e3af5fbeb8bbb2ea784de7bc96150?
It is what I got when I downloaded from the URL https://code.visualstudio.com/insiders/# and the bug is still present.
If there is some way to download build with the exact rev mentioned in above comment please let me know and I'll try with that

@sandy081
Copy link
Member

sandy081 commented Sep 4, 2023

@bpasero Since you were able to reproduce this, can you please confirm if it is fixed for you?

@bpasero
Copy link
Member

bpasero commented Sep 4, 2023

I am not seeing this issue anymore, also @roblourens verified it. I used above steps with an untitled workspace and with a saved workspace.

Does that actually make a difference for you @vaclavHala ?

@vaclavHala
Copy link
Author

Hmm the repro steps of this issue indeed seem to pass but when I tried running my original test (from which I derived the steps and code here as smallest possible repro) using the insiders build it still sometimes fails in the part where I setup workspace. It is likely that is caused by another problem though as I'm also calling vscode.workspace.fs.delete and vscode.workspace.fs.copy in the real code. I will try to create another repro demonstrating that and new issue with it if I succeed

@github-actions github-actions bot locked and limited conversation to collaborators Oct 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api author-verification-requested Issues potentially verifiable by issue author bug Issue identified by VS Code Team member as probable bug config VS Code configuration, set up issues confirmed Issue has been confirmed by VS Code Team member file-io File I/O insiders-released Patch has been released in VS Code Insiders verified Verification succeeded workbench-multiroot Multi-root (multiple folders) issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants