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.

Sign up for GitHub

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

Fix handler when install script makes an incomplete install #2106

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Fix handler when install script makes an incomplete install #2106

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ba54094
Refactor locking logic into its own class so we can use it elsewhere
nagilson Jan 28, 2025
f320a62
Add a mutex for the local installation directory and then wipe it if …
nagilson Jan 28, 2025
14c75ab
Install node-cache
nagilson Jan 30, 2025
281c44d
Fix package lock
nagilson Jan 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4,264 changes: 2,132 additions & 2,132 deletions sample/yarn.lock
View file
Open in desktop

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions vscode-dotnet-runtime-extension/package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

413 changes: 313 additions & 100 deletions vscode-dotnet-runtime-extension/yarn.lock
View file
Open in desktop

Large diffs are not rendered by default.

22 changes: 22 additions & 0 deletions vscode-dotnet-runtime-library/package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions vscode-dotnet-runtime-library/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
"https-proxy-agent": "^7.0.4",
"lodash": "^4.17.21",
"mocha": "^9.1.3",
"node-cache": "^5.1.2",
"open": "^8.4.0",
"proper-lockfile": "^4.1.2",
"rimraf": "3.0.2",
Expand Down
119 changes: 65 additions & 54 deletions vscode-dotnet-runtime-library/src/Acquisition/AcquisitionInvoker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ import { IInstallScriptAcquisitionWorker } from './IInstallScriptAcquisitionWork
import { DotnetInstall } from './DotnetInstall';
import { DotnetInstallMode } from './DotnetInstallMode';
import { WebRequestWorker } from '../Utils/WebRequestWorker';
import { executeWithLock } from './LockUtilities';
import { getDotnetExecutable } from '../Utils/TypescriptUtilities';

export class AcquisitionInvoker extends IAcquisitionInvoker {
protected readonly scriptWorker: IInstallScriptAcquisitionWorker;
Expand All @@ -49,75 +51,84 @@ You will need to restart VS Code after these changes. If PowerShell is still not

public async installDotnet(installContext: IDotnetInstallationContext, install : DotnetInstall): Promise<void>
{
const winOS = os.platform() === 'win32';
const installCommand = await this.getInstallCommand(installContext.version, installContext.installDir, installContext.installMode, installContext.architecture);

return new Promise<void>(async (resolve, reject) =>
return executeWithLock(this.eventStream, false, installContext.installDir, async (installContext: IDotnetInstallationContext, install : DotnetInstall) =>
{
try
return new Promise<void>(async (resolve, reject) =>
{
let windowsFullCommand = `powershell.exe -NoProfile -NonInteractive -NoLogo -ExecutionPolicy unrestricted -Command "& { [Net.ServicePointManager]::SecurityProtocol = [Net.ServicePointManager]::SecurityProtocol -bor [Net.SecurityProtocolType]::Tls12; & ${installCommand} }"`;
if(winOS)
try
{
const powershellReference = await this.verifyPowershellCanRun(installContext, install);
windowsFullCommand = windowsFullCommand.replace('powershell.exe', powershellReference);
}
const winOS = os.platform() === 'win32';
const installCommand = await this.getInstallCommand(installContext.version, installContext.installDir, installContext.installMode, installContext.architecture);

cp.exec(winOS ? windowsFullCommand : installCommand,
{ cwd: process.cwd(), maxBuffer: 500 * 1024, timeout: 1000 * installContext.timeoutSeconds, killSignal: 'SIGKILL' },
async (error, stdout, stderr) =>
{
if (stdout)
let windowsFullCommand = `powershell.exe -NoProfile -NonInteractive -NoLogo -ExecutionPolicy unrestricted -Command "& { [Net.ServicePointManager]::SecurityProtocol = [Net.ServicePointManager]::SecurityProtocol -bor [Net.SecurityProtocolType]::Tls12; & ${installCommand} }"`;
if(winOS)
{
this.eventStream.post(new DotnetAcquisitionScriptOutput(install, TelemetryUtilities.HashAllPaths(stdout)));
const powershellReference = await this.verifyPowershellCanRun(installContext, install);
windowsFullCommand = windowsFullCommand.replace('powershell.exe', powershellReference);
}
if (stderr)

// The install script can leave behind a directory in an invalid install state. Make sure the executable is present at the very least.
if(this.fileUtilities.existsSync(installContext.installDir) && !this.fileUtilities.existsSync(path.join(installContext.installDir, getDotnetExecutable())))
{
this.eventStream.post(new DotnetAcquisitionScriptOutput(install, `STDERR: ${TelemetryUtilities.HashAllPaths(stderr)}`));
this.fileUtilities.wipeDirectory(installContext.installDir, this.eventStream);
}
if (error)

cp.exec(winOS ? windowsFullCommand : installCommand,
{ cwd: process.cwd(), maxBuffer: 500 * 1024, timeout: 1000 * installContext.timeoutSeconds, killSignal: 'SIGKILL' },
async (error, stdout, stderr) =>
{
if (!(await WebRequestWorker.isOnline(installContext.timeoutSeconds, this.eventStream)))
if (stdout)
{
const offlineError = new EventBasedError('DotnetOfflineFailure', 'No internet connection detected: Cannot install .NET');
this.eventStream.post(new DotnetOfflineFailure(offlineError, install));
reject(offlineError);
this.eventStream.post(new DotnetAcquisitionScriptOutput(install, TelemetryUtilities.HashAllPaths(stdout)));
}
else if (error.signal === 'SIGKILL') {
const newError = new EventBasedError('DotnetAcquisitionTimeoutError',
`${timeoutConstants.timeoutMessage}, MESSAGE: ${error.message}, CODE: ${error.code}, KILLED: ${error.killed}`, error.stack);
this.eventStream.post(new DotnetAcquisitionTimeoutError(error, install, installContext.timeoutSeconds));
reject(newError);
if (stderr)
{
this.eventStream.post(new DotnetAcquisitionScriptOutput(install, `STDERR: ${TelemetryUtilities.HashAllPaths(stderr)}`));
}
if (error)
{
if (!(await WebRequestWorker.isOnline(installContext.timeoutSeconds, this.eventStream)))
{
const offlineError = new EventBasedError('DotnetOfflineFailure', 'No internet connection detected: Cannot install .NET');
this.eventStream.post(new DotnetOfflineFailure(offlineError, install));
reject(offlineError);
}
else if (error.signal === 'SIGKILL') {
const newError = new EventBasedError('DotnetAcquisitionTimeoutError',
`${timeoutConstants.timeoutMessage}, MESSAGE: ${error.message}, CODE: ${error.code}, KILLED: ${error.killed}`, error.stack);
this.eventStream.post(new DotnetAcquisitionTimeoutError(error, install, installContext.timeoutSeconds));
reject(newError);
}
else
{
const newError = new EventBasedError('DotnetAcquisitionInstallError',
`${timeoutConstants.timeoutMessage}, MESSAGE: ${error.message}, CODE: ${error.code}, SIGNAL: ${error.signal}`, error.stack);
this.eventStream.post(new DotnetAcquisitionInstallError(newError, install));
reject(newError);
}
}
else if (stderr && stderr.length > 0)
{
this.eventStream.post(new DotnetAcquisitionCompleted(install, installContext.dotnetPath, installContext.version));
resolve();
}
else
{
const newError = new EventBasedError('DotnetAcquisitionInstallError',
`${timeoutConstants.timeoutMessage}, MESSAGE: ${error.message}, CODE: ${error.code}, SIGNAL: ${error.signal}`, error.stack);
this.eventStream.post(new DotnetAcquisitionInstallError(newError, install));
reject(newError);
this.eventStream.post(new DotnetAcquisitionCompleted(install, installContext.dotnetPath, installContext.version));
resolve();
}
}
else if (stderr && stderr.length > 0)
{
this.eventStream.post(new DotnetAcquisitionCompleted(install, installContext.dotnetPath, installContext.version));
resolve();
}
else
{
this.eventStream.post(new DotnetAcquisitionCompleted(install, installContext.dotnetPath, installContext.version));
resolve();
}
});
}
catch (error : any)
{
// Remove this when https://github.com/typescript-eslint/typescript-eslint/issues/2728 is done
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
const newError = new EventBasedError('DotnetAcquisitionUnexpectedError', error?.message, error?.stack)
this.eventStream.post(new DotnetAcquisitionUnexpectedError(newError, install));
reject(newError);
}
});
});
}
catch (error : any)
{
// Remove this when https://github.com/typescript-eslint/typescript-eslint/issues/2728 is done
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
const newError = new EventBasedError('DotnetAcquisitionUnexpectedError', error?.message, error?.stack)
this.eventStream.post(new DotnetAcquisitionUnexpectedError(newError, install));
reject(newError);
}
});
}, installContext, install );
}

private async getInstallCommand(version: string, dotnetInstallDir: string, installMode: DotnetInstallMode, architecture: string): Promise<string> {
Expand Down
63 changes: 8 additions & 55 deletions vscode-dotnet-runtime-library/src/Acquisition/InstallTrackerSingleton.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import { DotnetCoreAcquisitionWorker } from './DotnetCoreAcquisitionWorker';
import { IEventStream } from '../EventStream/EventStream';
import { IExtensionState } from '../IExtensionState';
import { IDotnetAcquireContext } from '..';
import { executeWithLock } from './LockUtilities';

interface InProgressInstall
{
Expand Down Expand Up @@ -77,54 +78,6 @@ export class InstallTrackerSingleton
InstallTrackerSingleton.instance.extensionState = extensionState;
}

protected executeWithLock = async <A extends any[], R>(alreadyHoldingLock : boolean, dataKey : string, f: (...args: A) => R, ...args: A): Promise<R> =>
{
const trackingLock = `${dataKey}.lock`;
const lockPath = path.join(__dirname, trackingLock);
fs.writeFileSync(lockPath, '', 'utf-8');

let returnResult : any;

try
{
if(alreadyHoldingLock)
{
// eslint-disable-next-line @typescript-eslint/await-thenable
return await f(...(args));
}
else
{
this.eventStream?.post(new DotnetLockAttemptingAcquireEvent(`Lock Acquisition request to begin.`, new Date().toISOString(), lockPath, lockPath));
await lockfile.lock(lockPath, { retries: { retries: 10, minTimeout: 5, maxTimeout: 10000 } })
.then(async (release) =>
{
// eslint-disable-next-line @typescript-eslint/await-thenable
returnResult = await f(...(args));
this.eventStream?.post(new DotnetLockReleasedEvent(`Lock about to be released.`, new Date().toISOString(), lockPath, lockPath));
return release();
})
.catch((e : Error) =>
{
// Either the lock could not be acquired or releasing it failed
this.eventStream?.post(new DotnetLockErrorEvent(e, e.message, new Date().toISOString(), lockPath, lockPath));
});
}
}
catch(e : any)
{
// Either the lock could not be acquired or releasing it failed

// Remove this when https://github.com/typescript-eslint/typescript-eslint/issues/2728 is done
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
this.eventStream.post(new DotnetLockErrorEvent(e, e?.message ?? 'Unable to acquire lock to update installation state', new Date().toISOString(), lockPath, lockPath));

// Remove this when https://github.com/typescript-eslint/typescript-eslint/issues/2728 is done
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
throw new EventBasedError('DotnetLockErrorEvent', e?.message, e?.stack);
}

return returnResult;
}

public clearPromises() : void
{
Expand Down Expand Up @@ -176,7 +129,7 @@ export class InstallTrackerSingleton

public async canUninstall(isFinishedInstall : boolean, dotnetInstall : DotnetInstall, allowUninstallUserOnlyInstall = false) : Promise<boolean>
{
return this.executeWithLock( false, this.installedVersionsId, async (id: string, install: DotnetInstall) =>
return executeWithLock( this.eventStream, false, this.installedVersionsId, async (id: string, install: DotnetInstall) =>
{
this.eventStream.post(new RemovingVersionFromExtensionState(`Removing ${JSON.stringify(install)} with id ${id} from the state.`));
const existingInstalls = await this.getExistingInstalls(id === this.installedVersionsId, true);
Expand All @@ -189,15 +142,15 @@ export class InstallTrackerSingleton

public async uninstallAllRecords() : Promise<void>
{
await this.executeWithLock( false, this.installingVersionsId, async () =>
await executeWithLock( this.eventStream, false, this.installingVersionsId, async () =>
{
// This does not uninstall global things yet, so don't remove their ids.
const installingVersions = await this.getExistingInstalls(false, true);
const remainingInstallingVersions = installingVersions.filter(x => x.dotnetInstall.isGlobal);
await this.extensionState.update(this.installingVersionsId, remainingInstallingVersions);
}, );

return this.executeWithLock( false, this.installedVersionsId, async () =>
return executeWithLock( this.eventStream, false, this.installedVersionsId, async () =>
{
const installedVersions = await this.getExistingInstalls(true, true);
const remainingInstalledVersions = installedVersions.filter(x => x.dotnetInstall.isGlobal);
Expand All @@ -211,7 +164,7 @@ export class InstallTrackerSingleton
*/
public async getExistingInstalls(getAlreadyInstalledVersion : boolean, alreadyHoldingLock = false) : Promise<InstallRecord[]>
{
return this.executeWithLock( alreadyHoldingLock, getAlreadyInstalledVersion ? this.installedVersionsId : this.installingVersionsId,
return executeWithLock( this.eventStream, alreadyHoldingLock, getAlreadyInstalledVersion ? this.installedVersionsId : this.installingVersionsId,
(getAlreadyInstalledVersions : boolean) =>
{
const extensionStateAccessor = getAlreadyInstalledVersions ? this.installedVersionsId : this.installingVersionsId;
Expand Down Expand Up @@ -284,7 +237,7 @@ ${convertedInstalls.map(x => `${JSON.stringify(x.dotnetInstall)} owned by ${x.in

protected async removeVersionFromExtensionState(context : IAcquisitionWorkerContext, idStr: string, installIdObj: DotnetInstall, forceUninstall = false)
{
return this.executeWithLock( false, idStr, async (id: string, install: DotnetInstall) =>
return executeWithLock( this.eventStream, false, idStr, async (id: string, install: DotnetInstall) =>
{
this.eventStream.post(new RemovingVersionFromExtensionState(`Removing ${JSON.stringify(install)} with id ${id} from the state.`));
const existingInstalls = await this.getExistingInstalls(id === this.installedVersionsId, true);
Expand Down Expand Up @@ -332,7 +285,7 @@ ${installRecord.map(x => `${x.installingExtensions.join(' ')} ${JSON.stringify(I

protected async addVersionToExtensionState(context : IAcquisitionWorkerContext, idStr: string, installObj: DotnetInstall, alreadyHoldingLock = false)
{
return this.executeWithLock( alreadyHoldingLock, idStr, async (id: string, install: DotnetInstall) =>
return executeWithLock( this.eventStream, alreadyHoldingLock, idStr, async (id: string, install: DotnetInstall) =>
{
this.eventStream.post(new RemovingVersionFromExtensionState(`Adding ${JSON.stringify(install)} with id ${id} from the state.`));

Expand Down Expand Up @@ -369,7 +322,7 @@ ${existingVersions.map(x => `${JSON.stringify(x.dotnetInstall)} owned by ${x.ins

public async checkForUnrecordedLocalSDKSuccessfulInstall(context : IAcquisitionWorkerContext, dotnetInstallDirectory: string, installedInstallIdsList: InstallRecord[]): Promise<InstallRecord[]>
{
return this.executeWithLock( false, this.installedVersionsId, async (dotnetInstallDir: string, installedInstallIds: InstallRecord[]) =>
return executeWithLock( this.eventStream, false, this.installedVersionsId, async (dotnetInstallDir: string, installedInstallIds: InstallRecord[]) =>
{
let localSDKDirectoryIdIter = '';
try
Expand Down
Loading