zowe / zowe-explorer-vscode

Visual Studio Code Extension for Zowe, which lets users interact with z/OS Data Sets, Unix System Services, and Jobs on a remote mainframe instance. Powered by Zowe SDKs.
Eclipse Public License 2.0
173 stars 92 forks source link

Fix infinite loop when fetching USS resources with stat() #3321

Closed benjamin-t-santos closed 1 week ago

benjamin-t-santos commented 1 week ago

Proposed changes

Extenders can fetch a USS resource on demand using vscode.workspace.fs.stat() with the fetch=true query. When done with a USS file that has not expanded in the USS tree view, Zowe Explorer gets stuck in an infinite loop of calling stat() over and over again. Here's how it happens:

  1. vscode.workspace.fs.stat() is called for a USS resource with the fetch query set to true. Example: vscode.workspace.fs.stat(zowe-uss:/zosmf/user/test.txt)
  2. UssFSProvider.remoteLookupForResource() is called to fetch the file from the host.
  3. This eventually calls UssFSProvider.fetchEntries(). fetchEntries() checks if the parent directory of the requested file is in the file system. If it's not, await vscode.workspace.fs.createDirectory(parentUri) is called on line 191 to create it.
  4. stat() is called again for the parentUri with a fetch=true query as a result of calling vscode.workspace.fs.createDirectory(). (I believe this is due to the VS Code file system base implementation. It surprised me but looking at the call stack that is what I see)
  5. UssFSProvider.remoteLookupForResource() is called to fetch the directory from the host. This eventually calls UssFSProvider.fetchEntries()
  6. When listFiles() is called in fetchEntries(), if the resource requested is a directory that does not already exist in the file system, we call vscode.workspace.fs.createDirectory() with the directory's URI. This is the same exact URI that is used when calling createDirectory() in step 3.

Basically, Zowe Explorer keeps calling vscode.workspace.fs.createDirectory() for the parent of the requested file over and over again without ever resolving. This does not happen when using tree views because of how directories are expanded from the top-down. In this case, I as an extender am requesting a file from a parent directory that has not been created.

There are improvements we can make to improve the UssFSProvider for extenders, but with the goal of trying to get a fix for this issue into the v3.0.3 release, I tried to keep my changes minimal. I added code that sets the query of the parentUri to an empty string when calling vscode.workspace.fs.createDirectory() in fetchEntries(). This prevents it from entering the infinite loop by not trying to fetch the requested resource's parent.

This change will not impact the tree view, as those views do not make use of the fetch query. This change will only fix functionality for extenders.

Release Notes

Milestone:

Changelog:

Types of changes

Checklist

General

Code coverage

Deployment

Further comments

codecov[bot] commented 1 week ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 92.99%. Comparing base (f5ca8cc) to head (de97307). Report is 2 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #3321 +/- ## ======================================= Coverage 92.99% 92.99% ======================================= Files 116 116 Lines 12063 12069 +6 Branches 2776 2777 +1 ======================================= + Hits 11218 11224 +6 Misses 843 843 Partials 2 2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

benjamin-t-santos commented 1 week ago

@traeok Thanks for the review. Is it possible to include this fix in the v3.0.3 release?

JillieBeanSim commented 1 week ago

@traeok Thanks for the review. Is it possible to include this fix in the v3.0.3 release?

@zFernand0 @t1m0thyj @adam-wolfe what do y'all think too? I believe we are including a similar fix for another tree.

t1m0thyj commented 1 week ago

@zFernand0 @t1m0thyj @adam-wolfe what do y'all think too? I believe we are including a similar fix for another tree.

@JillieBeanSim This fix looks like a simple one that would be good to include 👍

JillieBeanSim commented 1 week ago

thanks I will get it ported