Skip to content

Commit 5d3ed11

Browse files
authored
Merge pull request #2646 from dotnet/copilot/fix-old-runtime-display-issue
Fix race condition: outdated runtimes persist in Uninstall list after force update
2 parents eb555eb + ca1388e commit 5d3ed11

4 files changed

Lines changed: 82 additions & 5 deletions

File tree

vscode-dotnet-runtime-extension/yarn.lock

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ export class LocalInstallUpdateService extends IInstallManagementService
210210
errorConfiguration: AcquireErrorConfiguration.DisableErrorPopups
211211
};
212212

213-
this.uninstallAction(uninstallContext, false, true).catch((e: any) => {});
213+
await this.uninstallAction(uninstallContext, false, true).catch((e: any) => {});
214214
}
215215

216216
processedGroup = true;

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

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -620,4 +620,81 @@ suite('LocalInstallUpdateService Unit Tests', function ()
620620
assert.lengthOf(ownersAdded, 1, 'Owners should move to the newest runtime install');
621621
assert.strictEqual(ownersAdded[0].install.installId, latestInstall.dotnetInstall.installId, 'Highest patch runtime should receive owners');
622622
});
623+
624+
test('It fully completes uninstalls before ManageInstalls returns so the uninstall list is accurate', async () =>
625+
{
626+
const onlineStub = {
627+
isOnline: async () => true
628+
} as unknown as WebRequestWorkerSingleton;
629+
630+
(WebRequestWorkerSingleton as unknown as { getInstance: () => WebRequestWorkerSingleton }).getInstance = () => onlineStub;
631+
632+
const eventStream = new MockEventStream();
633+
const extensionState = new MockExtensionContext();
634+
extensionState.update('dotnet.latestUpdateDate', new Date(0));
635+
636+
const directoryProvider = new TestInstallationDirectoryProvider('/tmp');
637+
638+
const legacyInstall = createInstallRecord('10.0.1', 'x64', 'runtime', ['ms-dotnettools.sample-extension']);
639+
const latestInstall = createInstallRecord('10.0.5', 'x64', 'runtime', []);
640+
641+
extensionState.update('installed', [legacyInstall]);
642+
643+
const tracker = RealLocalUpdateServiceTracker.getInstance(eventStream, extensionState);
644+
645+
const acquireStub = async (context: IDotnetAcquireContext) =>
646+
{
647+
const workerContext = getMockAcquisitionContext(context.mode ?? 'runtime', latestInstall.dotnetInstall.version, 5000, eventStream, extensionState, context.architecture ?? DotnetCoreAcquisitionWorker.defaultArchitecture(), directoryProvider);
648+
workerContext.acquisitionContext.requestingExtensionId = context.requestingExtensionId;
649+
650+
const installPath = path.join(directoryProvider.getInstallDir(latestInstall.dotnetInstall.installId), getDotnetExecutable());
651+
await tracker.trackInstalledVersion(workerContext, latestInstall.dotnetInstall, installPath);
652+
return undefined;
653+
};
654+
655+
const uninstallStub = async (context: IDotnetAcquireContext, force: boolean, onlyCheckLiveDependents: boolean) =>
656+
{
657+
const uninstallTracker = RealLocalUpdateServiceTracker.getInstance(eventStream, extensionState);
658+
const architecture = context.architecture ?? DotnetCoreAcquisitionWorker.defaultArchitecture();
659+
const dotnetInstall: DotnetInstall = {
660+
version: context.version!,
661+
architecture,
662+
installId: `${context.version}~${architecture}~${context.mode}`,
663+
installMode: context.mode!,
664+
isGlobal: false
665+
};
666+
667+
const workerContext = getMockAcquisitionContext(context.mode ?? 'runtime', context.version!, 5000, eventStream, extensionState, architecture, directoryProvider);
668+
workerContext.acquisitionContext.requestingExtensionId = context.requestingExtensionId;
669+
670+
const installExePath = path.join(directoryProvider.getInstallDir(dotnetInstall.installId), getDotnetExecutable());
671+
672+
await uninstallTracker.untrackInstalledVersion(workerContext, dotnetInstall, force);
673+
674+
const noDependents = force ? true : onlyCheckLiveDependents ?
675+
await uninstallTracker.installHasNoLiveDependentsBesidesId(installExePath, directoryProvider, context.requestingExtensionId ?? '', dotnetInstall) :
676+
await uninstallTracker.installHasNoRegisteredDependentsBesidesId(dotnetInstall, directoryProvider, false, context.requestingExtensionId ?? '');
677+
678+
if (force || noDependents)
679+
{
680+
await uninstallTracker.reportSuccessfulUninstall(workerContext, dotnetInstall, force);
681+
}
682+
683+
return '0';
684+
};
685+
686+
const updateService = new LocalInstallUpdateService(eventStream, extensionState, directoryProvider, acquireStub, uninstallStub, new MockLoggingObserver(), RealLocalUpdateServiceTracker);
687+
688+
await updateService.ManageInstalls(0);
689+
690+
// Immediately after ManageInstalls returns, the outdated install must already be removed from state.
691+
// Previously (before the fix), the uninstall was fire-and-forget so the old version would still appear
692+
// in the Uninstall .NET list when queried right after ManageInstalls returned.
693+
const remainingInstalls = await tracker.getExistingInstalls(directoryProvider, false);
694+
assert.deepEqual(
695+
remainingInstalls.map(install => install.dotnetInstall.installId),
696+
[latestInstall.dotnetInstall.installId],
697+
'Outdated installs must be fully removed from state by the time ManageInstalls returns, not fire-and-forget'
698+
);
699+
}).timeout(10000);
623700
});

vscode-dotnet-runtime-library/yarn.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)