Skip to content

Commit 902bb39

Browse files
nagilsonCopilot
andauthored
Fix Linux Mutex Utilization (#2229)
* Protect against stdout.txt not existing It seems like it dropped the execution lock (maybe it threw an error) when executing The command apt-get -o DPkg::Lock::Timeout=180 upgrade -y dotnet-sdk-8.0 was forwarded to the master process to run. It waited about 2 seconds then freed the server before it was done. It freed before it threw but free happens before throw, so it might have failed for some reason, then failed again since stdout.txt didn't exist because the process wasn't done running. then freed. When it freed, it was still going but then it allowed the sudo master process killer to run which caused this to fail later.... Still not sure why that'd happen, it might be if the locking was not implemented correctly. * Fix lint * Add timeout to the logging * Make each sudo process instance get its own directory. This will help prevent an existing process from colliding with another. * Dont delay anymore since the chance of this is very low this is basically like random int collision at this point, could probably remove. * Fix lint * Throw an error if stderr DNE Co-authored-by: Copilot <[email protected]> * Fix support matrix * Dont echo env in linux * Fix copilot change * Fix lint * Fix env var logging * Fix env var logging * Consider that EEXIST may be thrown instead of EADDRINUSE with certain kernels EACCESS as well. This may be something that will not resolve, though if some admin thing is blocking and holding the file, perhaps this is a good resolution. I never encountered this when trying different permissions on my demos but it seems that some people are. * Remove extraneous check * Use the old dir as a fallback if we cant create dirs and throw * Lint * Delete the directory might as well do it in this PR --------- Co-authored-by: Copilot <[email protected]>
1 parent bc53846 commit 902bb39

File tree

8 files changed

+93
-26
lines changed

8 files changed

+93
-26
lines changed

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,8 @@ export class DotnetPathFinder implements IDotnetPathFinder
133133

134134
else
135135
{
136-
this.executor?.execute(CommandExecutor.makeCommand('env', []), { dotnetInstallToolCacheTtlMs: DOTNET_INFORMATION_CACHE_DURATION_MS }, false)
136+
this.executor?.execute(CommandExecutor.makeCommand('echo', ['$PATH']), { dotnetInstallToolCacheTtlMs: DOTNET_INFORMATION_CACHE_DURATION_MS }, false)
137+
137138
.then((result) =>
138139
{
139140
// Log the default shell state
@@ -143,7 +144,8 @@ export class DotnetPathFinder implements IDotnetPathFinder
143144

144145
if (!(options.shell === '/bin/bash'))
145146
{
146-
this.executor?.execute(CommandExecutor.makeCommand('env', []), { shell: 'bin/bash', dotnetInstallToolCacheTtlMs: SYS_CMD_SEARCH_CACHE_DURATION_MS }, false)
147+
this.executor?.execute(CommandExecutor.makeCommand('echo', ['$PATH']), { shell: 'bin/bash', dotnetInstallToolCacheTtlMs: SYS_CMD_SEARCH_CACHE_DURATION_MS }, false)
148+
147149
.then((result) =>
148150
{
149151
this.workerContext.eventStream.post(new DotnetFindPathLookupPATH(`Execution Path (Unix Bash): ${result?.stdout}`));

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1168,6 +1168,16 @@ export class DotnetFindPathNoHostOnRegistry extends DotnetCustomMessageEvent
11681168
public readonly eventName = 'DotnetFindPathNoHostOnRegistry';
11691169
}
11701170

1171+
export class SudoDirCreationFailed extends DotnetCustomMessageEvent
1172+
{
1173+
public readonly eventName = 'SudoDirCreationFailed';
1174+
}
1175+
1176+
export class SudoDirDeletionFailed extends DotnetCustomMessageEvent
1177+
{
1178+
public readonly eventName = 'SudoDirDeletionFailed';
1179+
}
1180+
11711181
export class DotnetFindPathOnFileSystem extends DotnetCustomMessageEvent
11721182
{
11731183
public readonly eventName = 'DotnetFindPathOnFileSystem';
@@ -1535,6 +1545,10 @@ export class NetInstallerEndExecutionEvent extends DotnetCustomMessageEvent
15351545
public readonly eventName = 'NetInstallerEndExecutionEvent';
15361546
}
15371547

1548+
export class FailedToRunSudoCommand extends DotnetCustomMessageEvent
1549+
{
1550+
public readonly eventName = 'FailedToRunSudoCommand';
1551+
}
15381552

15391553
export class DotnetInstallLinuxChecks extends DotnetCustomMessageEvent
15401554
{

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

Lines changed: 56 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ import
3131
DotnetWSLSecurityError,
3232
EventBasedError,
3333
EventCancellationError,
34+
FailedToRunSudoCommand,
35+
SudoDirCreationFailed,
3436
SudoProcAliveCheckBegin,
3537
SudoProcAliveCheckEnd,
3638
SudoProcCommandExchangeBegin,
@@ -68,12 +70,15 @@ export class CommandExecutor extends ICommandExecutor
6870
LANGUAGE: 'en',
6971
DOTNET_CLI_UI_LANGUAGE: 'en-US',
7072
}; // Not all systems have english installed -- not sure if it's safe to use this.
71-
private sudoProcessCommunicationDir = path.join(__dirname, 'install scripts');
73+
private sudoProcessScript = path.join(__dirname, 'install scripts', 'interprocess-communicator.sh');
74+
private sudoProcessCommunicationDir: string;
7275
private fileUtil: IFileUtilities;
7376

7477
constructor(context: IAcquisitionWorkerContext, utilContext: IUtilityContext, protected readonly validSudoCommands?: string[])
7578
{
7679
super(context, utilContext);
80+
81+
this.sudoProcessCommunicationDir = path.join(__dirname, LockUsedByThisInstanceSingleton.SUDO_SESSION_ID);
7782
this.fileUtil = new FileUtilities();
7883
}
7984

@@ -85,7 +90,31 @@ export class CommandExecutor extends ICommandExecutor
8590
{
8691
const fullCommandString = CommandExecutor.prettifyCommandExecutorCommand(command, false);
8792
this.context?.eventStream.post(new CommandExecutionUnderSudoEvent(`The command ${fullCommandString} is being ran under sudo.`));
88-
const shellScript = path.join(this.sudoProcessCommunicationDir, 'interprocess-communicator.sh');
93+
const shellScript = this.sudoProcessScript;
94+
95+
try
96+
{
97+
await fs.promises.mkdir(this.sudoProcessCommunicationDir, { recursive: true });
98+
}
99+
catch (error: any)
100+
{
101+
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
102+
error.message = error?.message + `\nFailed to create ${this.sudoProcessCommunicationDir}. Please check your permissions or install dotnet manually.`;
103+
this.context?.eventStream.post(new SudoDirCreationFailed(`The command ${fullCommandString} failed, as no directory could be made: ${JSON.stringify(error)}`));
104+
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
105+
if (error?.code !== 'EEXIST')
106+
{
107+
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
108+
if (error?.code === 'EPERM' || error?.code === 'EACCES')
109+
{
110+
this.sudoProcessCommunicationDir = path.dirname(this.sudoProcessScript);
111+
}
112+
else
113+
{
114+
throw error;
115+
}
116+
}
117+
}
89118

90119
if (await isRunningUnderWSL(this.context, this.utilityContext, this))
91120
{
@@ -104,7 +133,7 @@ Please install the .NET SDK manually by following https://learn.microsoft.com/en
104133

105134
const waitForLockTimeMs = this.context?.timeoutSeconds ? (this.context?.timeoutSeconds * 1000 / 3) : 180000;
106135
// @ts-expect-error We want to hold the lock and sometimes return a bool, sometimes a CommandExecutorResult. The bool will never be returned if runCommand is true, so this makes the compiler accept this (its bad ik).
107-
return executeWithLock(this.context.eventStream, false, RUN_UNDER_SUDO_LOCK(this.sudoProcessCommunicationDir), SUDO_LOCK_PING_DURATION_MS, waitForLockTimeMs,
136+
return executeWithLock(this.context.eventStream, false, RUN_UNDER_SUDO_LOCK(this.sudoProcessScript), SUDO_LOCK_PING_DURATION_MS, waitForLockTimeMs,
108137
async () =>
109138
{
110139
this.startupSudoProc(fullCommandString, shellScript, terminalFailure).catch(() => {});
@@ -128,14 +157,6 @@ Please install the .NET SDK manually by following https://learn.microsoft.com/en
128157
return '0';
129158
}
130159
}
131-
else
132-
{
133-
if (await this.sudoProcIsLive(false, fullCommandString, 1000)) // If the sudo process was spawned by another instance of code, we do not want to have 2 at once but also do not waste a lot of time checking
134-
// As it should not be in the middle of an operation which may cause it to take a while.
135-
{
136-
return '0';
137-
}
138-
}
139160

140161
// Launch the process under sudo
141162
this.context?.eventStream.post(new CommandExecutionUserAskDialogueEvent(`Prompting user for command ${fullCommandString} under sudo.`));
@@ -187,7 +208,7 @@ ${stderr}`));
187208
private async sudoProcIsLive(errorIfDead: boolean, fullCommandString: string, maxTimeoutTimeMs?: number, runCommand = false): Promise<boolean | CommandExecutorResult>
188209
{
189210
const processAliveOkSentinelFile = path.join(this.sudoProcessCommunicationDir, 'ok.txt');
190-
const waitForLockTimeMs = maxTimeoutTimeMs ? maxTimeoutTimeMs : this.context?.timeoutSeconds ? (this.context?.timeoutSeconds * 1000 / 5) : 180000;
211+
const waitForLockTimeMs = maxTimeoutTimeMs ? maxTimeoutTimeMs : (this.context?.timeoutSeconds !== undefined ? (Math.max(this.context.timeoutSeconds * 1000 / 5, 100)) : 180000);
191212
const waitForSudoResponseTimeMs = waitForLockTimeMs * 0.75; // Arbitrary, but this should be less than the time to get the lock.
192213

193214
await (this.fileUtil as FileUtilities).wipeDirectory(this.sudoProcessCommunicationDir, this.context?.eventStream, ['.txt']);
@@ -219,7 +240,7 @@ ${stderr}`));
219240

220241
const isLive = LockUsedByThisInstanceSingleton.getInstance().isCurrentSudoProcCheckAlive();
221242
this.context?.eventStream.post(new SudoProcAliveCheckEnd(`Finished Sudo Process Master: Is Alive? ${isLive}. ${new Date().toISOString()}
222-
maxTimeoutTimeMs: ${maxTimeoutTimeMs} with lockTime ${waitForLockTimeMs} and responseTime ${waitForSudoResponseTimeMs}`));
243+
waitForLockTimeMs: ${waitForLockTimeMs} with lockTime ${waitForLockTimeMs} and responseTime ${waitForSudoResponseTimeMs}`));
223244

224245
// The sudo process spawned by vscode does not exit unless it fails or times out after an hour. We can't await it as we need it to persist.
225246
// If someone cancels the install, we store that error here since this gets awaited to prevent further code statement control flow from executing.
@@ -279,22 +300,24 @@ ${stderr}`));
279300
this.context?.eventStream.post(new CommandProcessorExecutionBegin(`The command ${commandToExecuteString} was forwarded to the master process to run.`));
280301

281302

282-
const waitTimeMs = this.context?.timeoutSeconds ? (this.context?.timeoutSeconds * 1000) : 600000;
283-
await loopWithTimeoutOnCond(100, waitTimeMs,
303+
const waitTimeMs = this.context?.timeoutSeconds ? (Math.max(this.context?.timeoutSeconds * 1000, 1000)) : 600000;
304+
const sampleRateMs = 100;
305+
await loopWithTimeoutOnCond(sampleRateMs, waitTimeMs,
284306
function ProcessFinishedExecutingAndWroteOutput(): boolean { return fs.existsSync(outputFile) },
285307
function doNothing(): void { ; },
286308
this.context.eventStream,
287-
new SudoProcCommandExchangePing(`Ping : Waiting. ${new Date().toISOString()}`)
309+
new SudoProcCommandExchangePing(`Ping : Waiting, at rate ${sampleRateMs} with timeout ${waitTimeMs} ${new Date().toISOString()}`)
288310
)
289311
.catch(error =>
290312
{
313+
this.context?.eventStream.post(new FailedToRunSudoCommand(`The command ${commandToExecuteString} failed to run: ${JSON.stringify(error ?? '')}.`));
291314
// Let the rejected promise get handled below. This is required to not make an error from the checking if this promise is alive
292315
});
293316

294317
commandOutputJson = {
295-
stdout: (fs.readFileSync(stdoutFile, 'utf8')).trim(),
296-
stderr: (fs.readFileSync(stderrFile, 'utf8')).trim(),
297-
status: (fs.readFileSync(statusFile, 'utf8')).trim()
318+
stdout: (await (this.fileUtil as FileUtilities).read(stdoutFile)).trim(),
319+
stderr: (await (this.fileUtil as FileUtilities).read(stderrFile)).trim(),
320+
status: (await (this.fileUtil as FileUtilities).read(statusFile)).trim()
298321
} as CommandExecutorResult;
299322

300323
this.context?.eventStream.post(new SudoProcCommandExchangeEnd(`Finished or timed out with master process. ${new Date().toISOString()}`));
@@ -347,6 +370,7 @@ ${stderr}`));
347370
await executeWithLock(this.context.eventStream, false, RUN_UNDER_SUDO_LOCK(this.sudoProcessCommunicationDir), SUDO_LOCK_PING_DURATION_MS, this.context.timeoutSeconds * 1000 / 5,
348371
async () =>
349372
{
373+
await (this.fileUtil as FileUtilities).wipeDirectory(this.sudoProcessCommunicationDir, this.context?.eventStream, ['.txt']);
350374
const processExitFile = path.join(this.sudoProcessCommunicationDir, 'exit.txt');
351375
await (this.fileUtil as FileUtilities).writeFileOntoDisk('', processExitFile, this.context?.eventStream);
352376

@@ -369,6 +393,19 @@ ${stderr}`));
369393
eventStream.post(new TriedToExitMasterSudoProcess(`Tried to exit sudo master process: exit code ${LockUsedByThisInstanceSingleton.getInstance().hasSpawnedSudoSuccessfullyWithoutDeath()}`));
370394
});
371395

396+
try
397+
{
398+
fs.rmdirSync(this.sudoProcessCommunicationDir, { recursive: true });
399+
}
400+
catch (error: any)
401+
{
402+
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
403+
if (error?.code !== 'ENOENT')
404+
{
405+
eventStream.post(new SudoDirCreationFailed(`The command ${this.sudoProcessCommunicationDir} failed to rm the sudo directory: ${JSON.stringify(error)}`));
406+
}
407+
}
408+
372409
return LockUsedByThisInstanceSingleton.getInstance().hasSpawnedSudoSuccessfullyWithoutDeath() ? 1 : 0;
373410
}
374411

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,16 @@ export class FileUtilities extends IFileUtilities
9696

9797
public async read(filePath: string): Promise<string>
9898
{
99-
const output = await fs.promises.readFile(filePath, 'utf8');
100-
return output;
99+
try
100+
{
101+
const output = await fs.promises.readFile(filePath, 'utf8');
102+
return output;
103+
}
104+
catch (error: any)
105+
{
106+
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
107+
throw new Error(`Failed to read file ${filePath}: ${error?.message}`);
108+
}
101109
}
102110

103111
public async exists(filePath: string): Promise<boolean>

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ export class LockUsedByThisInstanceSingleton
1111
private currentAliveStatus = false;
1212
private sudoError: any = null;
1313

14+
public static readonly SUDO_SESSION_ID = crypto.randomUUID().substring(0, 8);
15+
1416
protected constructor(protected lockStringAndThisVsCodeInstanceOwnsIt: { [lockString: string]: boolean } = {})
1517
{
1618

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ export class NodeIPCMutex
108108
catch (error: any)
109109
{
110110
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
111-
if (error?.code === 'EADDRINUSE') // We couldn't acquire the lock, even though nobody else is using it.
111+
if (error?.code === 'EADDRINUSE' || error?.code === 'EEXIST' || error?.code === 'EACCESS')
112112
{
113113
if (retries >= maxRetryCountToEndAtRoughlyTimeoutTime)
114114
{

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,12 @@ export async function loopWithTimeoutOnCond(sampleRatePerMs: number, durationToW
2525
if (conditionToStop())
2626
{
2727
doAfterStop();
28-
return;
28+
return Promise.resolve();
2929
}
3030
eventStream?.post(waitEvent);
3131
await new Promise(waitAndResolve => setTimeout(waitAndResolve, sampleRatePerMs));
3232
}
33-
throw new Error('The promise timed out.');
33+
throw new Error(`The promise timed out at ${durationToWaitBeforeTimeoutMs}.`);
3434
}
3535

3636
/**

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,10 @@ export async function getLinuxSupportedDotnetSDKVersion(context: IAcquisitionWor
149149
{
150150
return '6.0.100';
151151
}
152+
if (distroInfo.version < '22.06')
153+
{
154+
return '9.0.100';
155+
}
152156
if (distroInfo.version < '24.04')
153157
{
154158
return '8.0.100';

0 commit comments

Comments
 (0)