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

Leaving duplicate workspace nodes When refreshing #171458

Open
yiliang114 opened this issue Jan 17, 2023 · 7 comments
Open

Leaving duplicate workspace nodes When refreshing #171458

yiliang114 opened this issue Jan 17, 2023 · 7 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug file-explorer Explorer widget issues web Issues related to running VSCode in the web

Comments

@yiliang114
Copy link
Contributor

Reproduction step:

  1. Open https://vscode.dev/, then open any project from github, such as https://vscode.dev/github/yiliang114/vscode
  2. After the file tree is loaded, refresh the page directly without any other operations.
  3. You can see that the data of the file tree has disappeared, leaving only a temporary folder named workspace name, but in fact, there is already a workspace name on tree item.

bug15

image

Version: 1.74.3
Commit: 97dec17
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/108.0.0.0 Safari/537.36
Embedder: vscode.dev

@lramos15
Copy link
Member

@jrieken I think this is FS provider related. Let me know if you disagree, it also might be a lifecycle thing cc @bpasero

@jrieken jrieken assigned lramos15 and unassigned jrieken Jan 20, 2023
@jrieken
Copy link
Member

jrieken commented Jan 20, 2023

I don't think so, I remember that the explorer always had issues with that. Since it's the explorer showing the error I suggest that the investigation starts there. It's likely that it gives us to restore a persisted state before the actual extension registers the FS

@bpasero
Copy link
Member

bpasero commented Jan 20, 2023

cc @sandy081, I remember some tricks with how extension activation works with custom schemes, but I forgot.

@bpasero bpasero added file-explorer Explorer widget issues web Issues related to running VSCode in the web labels Jan 20, 2023
@lramos15 lramos15 added the bug Issue identified by VS Code Team member as probable bug label Jan 20, 2023
@sandy081
Copy link
Member

In configuration land I wait until the FSP is registered like this

await whenProviderRegistered(workspaceIdentifier.configPath, this.fileService);

@joyceerhl
Copy link
Contributor

This scenario happens when the workbench has already loaded and the page is being refreshed.

  1. The page reload triggers stopExtensionHosts in abstractExtensionHost.ts
  2. In this case the FS provider is coming from an extension, so stopping the extension host also disposes the means the FS provider gets disposed
  3. The explorer service listens for onDidChangeFileSystemProviderRegistrations and resets the tree input
  4. This leads to the explorer tree view getting rerendered
  5. The explorer has a decorations provider which runs after the rerender and it calls stat on the resource. At this point there is no filesystem capable of handling the stat anymore, so you get the ! Unable to resolve workspace folder decoration, just before the page reloads

This seems like mostly a visual problem rather than missing/broken functionality, maybe the explorer view can avoid setting the tree input and triggering the layout if the workbench is about to go down anyway?

@yiliang114
Copy link
Contributor Author

This scenario happens when the workbench has already loaded and the page is being refreshed.

  1. The page reload triggers stopExtensionHosts in abstractExtensionHost.ts
  2. In this case the FS provider is coming from an extension, so stopping the extension host also disposes the means the FS provider gets disposed
  3. The explorer service listens for onDidChangeFileSystemProviderRegistrations and resets the tree input
  4. This leads to the explorer tree view getting rerendered
  5. The explorer has a decorations provider which runs after the rerender and it calls stat on the resource. At this point there is no filesystem capable of handling the stat anymore, so you get the ! Unable to resolve workspace folder decoration, just before the page reloads

This seems like mostly a visual problem rather than missing/broken functionality, maybe the explorer view can avoid setting the tree input and triggering the layout if the workbench is about to go down anyway?

Before refreshing the page, is the setTreeInput called in onDidChangeFileSystemProviderRegistrations callback to reduce memory leaks? If not, I think the data of the file tree is not reset, will look better to experience it when the onDidChangeFileSystemProviderRegistrations triggered by refresh. It is a bit strange that the data disappears faster than refresh behavior.

@yiliang114
Copy link
Contributor Author

onDidChangeFileSystemProviderRegistrations Triggered by events here

this._onDidChangeFileSystemProviderRegistrations.fire({ added: false, scheme, provider });

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue identified by VS Code Team member as probable bug file-explorer Explorer widget issues web Issues related to running VSCode in the web
Projects
None yet
Development

No branches or pull requests

6 participants