Fix race condition in ProfilesCache refresh

[t1m0thyj]

Proposed changes

Fixes #2831

Release Notes

Milestone: 2.15.3

Changelog: Fixed an issue where ProfilesCache may return missing or incorrect profile values when multiple extensions call it during activation

Types of changes

What types of changes does your code introduce to Zowe Explorer? Put an x in the boxes that apply

• [x] Bugfix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
• [ ] Updates to Documentation or Tests (if none of the other choices apply)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This checklist will be used as reference for both the contributor and the reviewer

• [ ] I have read the CONTRIBUTOR GUIDANCE wiki
• [ ] PR title follows Conventional Commits Guidelines
• [ ] PR Description is included
• [ ] gif or screenshot is included if visual changes are made
• [ ] yarn workspace vscode-extension-for-zowe vscode:prepublish has been executed
• [ ] All checks have passed (DCO, Jenkins and Code Coverage)
• [ ] I have added unit test and it is passing
• [ ] I have added integration test and it is passing
• [ ] There is coverage for the code that I have added
• [ ] I have tested it manually and there are no regressions found
• [ ] I have added necessary documentation (if appropriate)
• [ ] Any PR dependencies have been merged and published (if appropriate)

Further comments

[codecov[bot]]

Codecov Report

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

Project coverage is 93.44%. Comparing base (4c08160) to head (93270d0).

Additional details and impacted files

```diff @@ Coverage Diff @@ ## maintenance #2838 +/- ## ============================================ Coverage 93.44% 93.44% ============================================ Files 103 103 Lines 10673 10675 +2 Branches 2299 2324 +25 ============================================ + Hits 9973 9975 +2 Misses 699 699 Partials 1 1 ```

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

[roman-kupriyanov]

Hey @t1m0thyj! Thanks for a quick fix. We tested it with both our extensions activated at the same time and it works better in terms of now it recognizes at least one of our profile types. For the second one it returns nothing after multiple reloads 🙁 So I assume there is still racing somewhere between the extensions.

We also played with the init calls and this what we found. It actually starts to work as expected if you add extra reload after the init process:

      const cache = zoweExplorerExtenderApi.getProfilesCache();
      for (const profileSchema of desiredProfileSchemas) {
        await zoweExplorerExtenderApi.initForZowe(
          profileSchema.type,
          profileSchema.schema ? [profileSchema] : undefined
        );
        cache.registerCustomProfilesType(profileSchema.type);
      }
      await zoweExplorerExtenderApi.reloadProfiles();

We removed extra reloadProfiles() because previously it made it work better for us and to avoid triggering multiple refreshes, but now it does not work properly without it.

On the other hand, we tried to exchange the order of registerCustomProfilesType() and initForZowe() calls:

      const cache = zoweExplorerExtenderApi.getProfilesCache();
      for (const profileSchema of desiredProfileSchemas) {
        cache.registerCustomProfilesType(profileSchema.type);
        await zoweExplorerExtenderApi.initForZowe(
          profileSchema.type,
          profileSchema.schema ? [profileSchema] : undefined
        );
      }

And it also work now without even extra reloadProfiles() call.

What would be the proper order of calling all of those methods from the API and which ones of those are actually necessary/not necessary?

[sonarcloud[bot]]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

[traeok]

Hi @roman-kupriyanov,

I believe the second block of code you posted would be the right approach with the current state of the ProfilesCache. In initForZowe we do a refresh of the profiles cache, and during that refresh, we get all the types using ProfilesCache.getAllProfileTypes. If you do not call registerCustomProfilesType beforehand, it will not consider that type unless:

• it has been registered (using the same function) by another extender
• it has been registered with one of the APIs (MVS, USS, or JES).

Ideally, I'd like to register those custom types as a part of the logic in initForZowe so that extenders don't have to call registerCustomProfilesType - rather, we can do it ourselves during initialization if the type doesn't already exist. I don't think this would be breaking since allExternalTypes is already a set, so continuing to call the registerCustomProfilesType function from an extender's perspective shouldn't affect anything.

I don't want to speak for @t1m0thyj, but those are my thoughts regarding extender initialization/preparing custom profile types 😋

[roman-kupriyanov]

Hi @traeok,

Thanks for the quick response!

I believe the second block of code you posted would be the right approach with the current state of the ProfilesCache. In initForZowe we do a refresh of the profiles cache, and during that refresh, we get all the types using ProfilesCache.getAllProfileTypes. If you do not call registerCustomProfilesType beforehand, it will not consider that type unless it has already been registered under one of the APIs (MVS, USS, or JES).

That was my assumption too since it makes total sense to have an external profile type registered before the attempt of refreshing the cache. I think, before that extra reloadProfiles() call was actually protecting us from the problems by refreshing one more time, when all the parameters are actually set.

Ideally, I'd like to register those custom types as a part of the logic in initForZowe so that extenders don't have to call registerCustomProfilesType - rather, we can do it ourselves during initialization if the type doesn't already exist. I don't think this would be breaking since allExternalTypes is already a set, so continuing to call the registerCustomProfilesType function from an extender's perspective shouldn't affect anything.

Those were my thoughts exactly! Moreover, if the order of those calls is crucial to make it work right, I would expect it to be called internally by initForZowe() and at the right time to avoid making a mistake as the extender.

But I am not going to force any decision here as well and leave it up to @t1m0thyj and the team.

Thanks for the confirmation and investigation, cheers!

[JillieBeanSim]

Thanks for testing the different scenarios of registering with ZE @roman-kupriyanov. We will want to make sure our documentation matches the best solution for registering with ZE in zowe.org and our wiki if it's different than what we already have so other extenders can benefit from it. ❤️