Fix an issue parsing promises (#1927)

* Fix an issue parsing promises

We changed the function `getPromise` to not use a lock in version 2.1.2. This was a mistake. I thought it would return the unresolved `Promise` object if it was a sync function. That is not the behavior, instead it will skip that line of code and return null even if the promise exists. This causes multiple processes to try to grab the installer file handle.

When one fails to grab the file handle, it causes a chain-reaction of cascading failures for concurrent requests, because that is supposed to be blocked from happening.

In addition, the check I added to kill the sudo process once it is finished runs on all platforms despite the master sudo process only being present on Linux, which has resulted in some higher timeout rates. We need to not do this on other platforms.

* Fix tests

we need to clear the promise out after each test is done

* dont call uninstall all in sdk test, its the wrong command

* reset event stream in the test

sample/package-lock.json

@@ -33,7 +33,7 @@
 		},
 		"../vscode-dotnet-runtime-extension": {
 			"name": "vscode-dotnet-runtime",
-			"version": "2.1.2",
+			"version": "2.1.3",
 			"license": "MIT",
 			"dependencies": {
 				"@types/chai-as-promised": "^7.1.8",

vscode-dotnet-runtime-extension/CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning].
 
 ## [Unreleased]
 
+## [2.1.3] - 2024-08-19
+
+Fixes an issue with concurrent SDK installs introduced in 2.1.2.
+
 ## [2.1.2] - 2024-08-01
 
 Adds the ability for users to uninstall things themselves.

vscode-dotnet-runtime-extension/package-lock.json

@@ -1,12 +1,12 @@
 {
 	"name": "vscode-dotnet-runtime",
-	"version": "2.1.2",
+	"version": "2.1.3",
 	"lockfileVersion": 3,
 	"requires": true,
 	"packages": {
 		"": {
 			"name": "vscode-dotnet-runtime",
-			"version": "2.1.2",
+			"version": "2.1.3",
 			"license": "MIT",
 			"dependencies": {
 				"@types/chai-as-promised": "^7.1.8",

vscode-dotnet-runtime-extension/package.json

@@ -13,7 +13,7 @@
 	"description": "This extension installs and manages different versions of the .NET SDK and Runtime.",
 	"appInsightsKey": "02dc18e0-7494-43b2-b2a3-18ada5fcb522",
 	"icon": "images/dotnetIcon.png",
-	"version": "2.1.2",
+	"version": "2.1.3",
 	"publisher": "ms-dotnettools",
 	"engines": {
 		"vscode": "^1.81.1"

vscode-dotnet-runtime-extension/src/test/functional/DotnetCoreAcquisitionExtension.test.ts

@@ -24,11 +24,13 @@ import {
   MockWindowDisplayWorker,
   getMockAcquisitionContext,
   DotnetInstallMode,
-  DotnetInstallType
+  DotnetInstallType,
+  MockEventStream
 } from 'vscode-dotnet-runtime-library';
 import * as extension from '../../extension';
 import { warn } from 'console';
 import { json } from 'stream/consumers';
+import { InstallTrackerSingleton } from 'vscode-dotnet-runtime-library/dist/Acquisition/InstallTrackerSingleton';
 /* tslint:disable:no-any */
 /* tslint:disable:no-unsafe-finally */
 
@@ -97,6 +99,7 @@ suite('DotnetCoreAcquisitionExtension End to End', function()
     mockState.clear();
     MockTelemetryReporter.telemetryEvents = [];
     rimraf.sync(storagePath);
+    InstallTrackerSingleton.getInstance(new MockEventStream(), new MockExtensionContext()).clearPromises();
   });
 
   test('Activate', async () => {

vscode-dotnet-runtime-library/src/Acquisition/DotnetCoreAcquisitionWorker.ts

@@ -187,12 +187,10 @@ To keep your .NET version up to date, please reconnect to the internet at your s
         const install = GetDotnetInstallInfo(version, installMode, 'local',
             architecture ? architecture : context.acquisitionContext.architecture ?? this.getDefaultInternalArchitecture(context.acquisitionContext.architecture))
 
-        const existingAcquisitionPromise = InstallTrackerSingleton.getInstance(context.eventStream, context.extensionState).getPromise(install);
+        const existingAcquisitionPromise = await InstallTrackerSingleton.getInstance(context.eventStream, context.extensionState).getPromise(install, context.acquisitionContext, true);
         if (existingAcquisitionPromise)
         {
-            // Requested version is being acquired
-            context.eventStream.post(new DotnetAcquisitionStatusResolved(install, version));
-            return existingAcquisitionPromise.then((res) => ({ dotnetPath: res }));
+            return { dotnetPath: existingAcquisitionPromise } as IDotnetAcquireResult;
         }
 
         const dotnetInstallDir = context.installDirectoryProvider.getInstallDir(install.installId);
@@ -256,14 +254,10 @@ To keep your .NET version up to date, please reconnect to the internet at your s
         }
 
         context.eventStream.post(new DotnetInstallIdCreatedEvent(`The requested version ${version} is now marked under the install: ${JSON.stringify(install)}.`));
-        const existingAcquisitionPromise = InstallTrackerSingleton.getInstance(context.eventStream, context.extensionState).getPromise(install);
+        const existingAcquisitionPromise = await InstallTrackerSingleton.getInstance(context.eventStream, context.extensionState).getPromise(install, context.acquisitionContext);
         if (existingAcquisitionPromise)
         {
-            // This version of dotnet is already being acquired. Memoize the promise.
-            context.eventStream.post(new DotnetAcquisitionInProgress(install,
-                    (context.acquisitionContext && context.acquisitionContext.requestingExtensionId)
-                    ? context.acquisitionContext.requestingExtensionId : null));
-            return existingAcquisitionPromise.then((res) => ({ dotnetPath: res }));
+            return { dotnetPath: existingAcquisitionPromise } as IDotnetAcquireResult;
         }
         else
         {
@@ -408,7 +402,7 @@ To keep your .NET version up to date, please reconnect to the internet at your s
 
         // Don't count it as partial if the promise is still being resolved.
         // The promises get wiped out upon reload, so we can check this.
-        if (partialInstall && InstallTrackerSingleton.getInstance(context.eventStream, context.extensionState).getPromise(installId) === null)
+        if (partialInstall && await InstallTrackerSingleton.getInstance(context.eventStream, context.extensionState).getPromise(installId, context.acquisitionContext, true) === null)
         {
             // Partial install, we never updated our extension to no longer be 'installing'. Maybe someone killed the vscode process or we failed in an unexpected way.
             context.eventStream.post(new DotnetAcquisitionPartialInstallation(installId));

vscode-dotnet-runtime-library/src/Acquisition/InstallTrackerSingleton.ts

@@ -9,6 +9,7 @@ import { IAcquisitionWorkerContext } from './IAcquisitionWorkerContext';
 import {
     AddTrackingVersions,
     ConvertingLegacyInstallRecord,
+    DotnetAcquisitionInProgress,
     DotnetAcquisitionStatusResolved,
     DotnetLockAttemptingAcquireEvent,
     DotnetLockErrorEvent,
@@ -37,6 +38,7 @@ import { InstallRecord, InstallRecordOrStr } from './InstallRecord';
 import { DotnetCoreAcquisitionWorker } from './DotnetCoreAcquisitionWorker';
 import { IEventStream } from '../EventStream/EventStream';
 import { IExtensionState } from '../IExtensionState';
+import { IDotnetAcquireContext } from '..';
 /* tslint:disable:no-any */
 
 
@@ -127,18 +129,26 @@ export class InstallTrackerSingleton
      *
      * @param install the install id to get a working install promise for.
      */
-    public getPromise(install : DotnetInstall) : Promise<string> | null
+    public async getPromise(install : DotnetInstall, acquisitionContext : IDotnetAcquireContext, disableOutput = false) : Promise<string | null>
     {
-            this.inProgressInstalls.forEach(x =>
+        for(const x of this.inProgressInstalls)
+        {
+            const xAsId = x.dotnetInstall as DotnetInstall;
+            if(IsEquivalentInstallationFile(xAsId, install))
             {
-                const xAsId = x.dotnetInstall as DotnetInstall;
-                if(IsEquivalentInstallationFile(xAsId, install))
+                this.eventStream.post(new DotnetAcquisitionStatusResolved(install, install.version));
+
+                if(!disableOutput)
                 {
-                    this.eventStream.post(new DotnetAcquisitionStatusResolved(install, install.version));
-                    return x.installingPromise;
+                    this.eventStream.post(new DotnetAcquisitionInProgress(install,
+                        (acquisitionContext && acquisitionContext.requestingExtensionId)
+                        ? acquisitionContext.requestingExtensionId : 'unknown'));
                 }
-            })
-            return null;
+                const result = await x.installingPromise;
+                return result;
+            }
+        }
+        return null;
     }
 
     public addPromise(install : DotnetInstall, installPromise : Promise<string>) : void

vscode-dotnet-runtime-library/src/EventStream/EventStreamEvents.ts

@@ -1081,12 +1081,12 @@ export class DotnetAcquisitionInProgress extends IEvent {
     public readonly type = EventType.DotnetAcquisitionInProgress;
 
     public readonly eventName = 'DotnetAcquisitionInProgress';
-    constructor(public readonly installId: DotnetInstall, public readonly requestingExtensionId: string | null) { super(); }
+    constructor(public readonly install: DotnetInstall, public readonly requestingExtensionId: string | null) { super(); }
 
     public getProperties() {
         return {
-            InProgressInstallationInstallId : this.installId.installId,
-            ...InstallToStrings(this.installId!),
+            InProgressInstallationInstallId : this.install.installId,
+            ...InstallToStrings(this.install!),
             extensionId : TelemetryUtilities.HashData(this.requestingExtensionId)};
     }
 }

vscode-dotnet-runtime-library/src/EventStream/OutputChannelObserver.ts

@@ -99,7 +99,7 @@ export class OutputChannelObserver implements IEventStreamObserver {
                     this.outputChannel.append(`${
                         (event as DotnetAcquisitionInProgress).requestingExtensionId
                     } tried to install .NET ${
-                        (event as DotnetAcquisitionInProgress).installId
+                        (event as DotnetAcquisitionInProgress).install.installId
                     } but that install had already been requested. No downloads or changes were made.\n`);
                 }
                 break;

vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts

@@ -330,6 +330,11 @@ ${(commandOutputJson as CommandExecutorResult).stderr}.`),
      */
     public async endSudoProcessMaster(eventStream : IEventStream) : Promise<number>
     {
+        if(os.platform() !== 'linux')
+        {
+            return 0;
+        }
+
         let didDelete = 1;
         const processExitFile = path.join(this.sudoProcessCommunicationDir, 'exit.txt');
         await (this.fileUtil as FileUtilities).writeFileOntoDisk('', processExitFile, true, this.context?.eventStream);

vscode-dotnet-runtime-library/src/Utils/WebRequestWorker.ts

@@ -225,7 +225,7 @@ export class WebRequestWorker
         }
         catch(error : any)
         {
-            if(error?.message?.contains('ENOSPC'))
+            if(error && error?.message && (error?.message as string)?.includes('ENOSPC'))
             {
                 const err = new DiskIsFullError(new EventBasedError('DiskIsFullError',
 `You don't have enough space left on your disk to install the .NET SDK. Please clean up some space.`), getInstallFromContext(this.context));

vscode-dotnet-runtime-library/src/test/unit/DotnetCoreAcquisitionWorker.test.ts

@@ -40,6 +40,7 @@ import { IAcquisitionWorkerContext } from '../../Acquisition/IAcquisitionWorkerC
 import { IEventStream } from '../../EventStream/EventStream';
 import { DotnetInstallType} from '../../IDotnetAcquireContext';
 import { getInstallIdCustomArchitecture } from '../../Utils/InstallIdUtilities';
+import { InstallTrackerSingleton } from '../../Acquisition/InstallTrackerSingleton';
 
 const assert = chai.assert;
 chai.use(chaiAsPromised);
@@ -50,6 +51,11 @@ suite('DotnetCoreAcquisitionWorker Unit Tests', function () {
     const installedVersionsKey = 'installed';
     const dotnetFolderName = `.dotnet O'Hare O'Donald`;
 
+    this.afterEach(async () => {
+        // Tear down tmp storage for fresh run
+        InstallTrackerSingleton.getInstance(new MockEventStream(), new MockExtensionContext()).clearPromises();
+    });
+
     function setupStates(): [MockEventStream, MockExtensionContext]
     {
         const extContext = new MockExtensionContext();

vscode-dotnet-sdk-extension/src/test/functional/DotnetCoreAcquisitionExtension.test.ts

@@ -32,10 +32,13 @@ import {
   IExistingPaths,
   getMockAcquisitionContext,
   getMockAcquisitionWorker,
+  MockInstallTracker,
 } from 'vscode-dotnet-runtime-library';
 import * as extension from '../../extension';
 import { uninstallSDKExtension } from '../../ExtensionUninstall';
 import { warn } from 'console';
+import { InstallTrackerSingleton } from 'vscode-dotnet-runtime-library/dist/Acquisition/InstallTrackerSingleton';
+import { mock } from 'node:test';
 
 const standardTimeoutTime = 100000;
 const assert = chai.assert;
@@ -76,6 +79,14 @@ suite('DotnetCoreAcquisitionExtension End to End', function()
     });
   });
 
+  this.afterEach(async () => {
+    // Tear down tmp storage for fresh run
+    mockState.clear();
+    MockTelemetryReporter.telemetryEvents = [];
+    rimraf.sync(storagePath);
+    InstallTrackerSingleton.getInstance(new MockEventStream(), new MockExtensionContext()).clearPromises();
+  });
+
 
   test('Activate', async () => {
     // Commands should now be registered
@@ -107,6 +118,9 @@ suite('DotnetCoreAcquisitionExtension End to End', function()
     fs.mkdirSync(sdkDirEarlier, { recursive: true });
     fs.writeFileSync(dotnetExePath, '');
 
+    // set the event stream of the singleton since this is normally not needed per run
+    const _ = new MockInstallTracker(eventStream, mockContext.extensionState);
+
     // Assert preinstalled SDKs are detected
     const acquisitionInvoker = new NoInstallAcquisitionInvoker(eventStream, acquisitionWorker);
     const result = await acquisitionWorker.acquireLocalSDK(mockContext, acquisitionInvoker);
@@ -135,6 +149,7 @@ suite('DotnetCoreAcquisitionExtension End to End', function()
     const acquisitionWorker = getMockAcquisitionWorker(mockContext);
 
     const currentVersionInstallId =  getInstallIdCustomArchitecture(version, os.arch(), 'sdk', 'local');
+    InstallTrackerSingleton.getInstance(mockContext.eventStream, mockContext.extensionState).clearPromises();
     // Ensure nothing is returned when there is no preinstalled SDK
     const noPreinstallResult = await acquisitionWorker.acquireStatus(mockContext, 'sdk');
     assert.isUndefined(noPreinstallResult);