Skip to content

Commit 5831d64

Browse files
herbenderblerherbenderbler
committed
fix: tighten process invoker disposal tests
1 parent b06c585 commit 5831d64

5 files changed

Lines changed: 315 additions & 134 deletions

File tree

‎src/Runner.Common/Util/UnixUtil.cs‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -42,21 +42,24 @@ public async Task ExecAsync(string workingDirectory, string toolName, string arg
4242
string toolPath = WhichUtil.Which(toolName, trace: Trace);
4343
Trace.Info($"Running {toolPath} {argLine}");
4444

45-
var processInvoker = HostContext.CreateService<IProcessInvoker>();
46-
processInvoker.OutputDataReceived += OnOutputDataReceived;
47-
processInvoker.ErrorDataReceived += OnErrorDataReceived;
48-
49-
try
45+
// Dispose invoker per call to release process/CTS resources promptly.
46+
using (var processInvoker = HostContext.CreateService<IProcessInvoker>())
5047
{
51-
using (var cs = new CancellationTokenSource(TimeSpan.FromSeconds(45)))
48+
processInvoker.OutputDataReceived += OnOutputDataReceived;
49+
processInvoker.ErrorDataReceived += OnErrorDataReceived;
50+
51+
try
5252
{
53-
await processInvoker.ExecuteAsync(workingDirectory, toolPath, argLine, null, true, cs.Token);
53+
using (var cs = new CancellationTokenSource(TimeSpan.FromSeconds(45)))
54+
{
55+
await processInvoker.ExecuteAsync(workingDirectory, toolPath, argLine, null, true, cs.Token);
56+
}
57+
}
58+
finally
59+
{
60+
processInvoker.OutputDataReceived -= OnOutputDataReceived;
61+
processInvoker.ErrorDataReceived -= OnErrorDataReceived;
5462
}
55-
}
56-
finally
57-
{
58-
processInvoker.OutputDataReceived -= OnOutputDataReceived;
59-
processInvoker.ErrorDataReceived -= OnErrorDataReceived;
6063
}
6164
}
6265

‎src/Runner.Worker/Container/DockerCommandManager.cs‎

Lines changed: 110 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -309,45 +309,48 @@ public async Task<int> DockerExec(IExecutionContext context, string containerId,
309309
{
310310
ArgUtil.NotNull(output, nameof(output));
311311

312+
if (!Constants.Runner.Platform.Equals(Constants.OSPlatform.Linux))
313+
{
314+
throw new NotSupportedException("Container operations are only supported on Linux runners");
315+
}
316+
312317
string arg = $"exec {options} {containerId} {command}".Trim();
313318
context.Command($"{DockerPath} {arg}");
314319

315320
object outputLock = new();
316-
var processInvoker = HostContext.CreateService<IProcessInvoker>();
317-
processInvoker.OutputDataReceived += delegate (object sender, ProcessDataReceivedEventArgs message)
321+
using (var processInvoker = HostContext.CreateService<IProcessInvoker>())
318322
{
319-
if (!string.IsNullOrEmpty(message.Data))
323+
processInvoker.OutputDataReceived += delegate (object sender, ProcessDataReceivedEventArgs message)
320324
{
321-
lock (outputLock)
325+
if (!string.IsNullOrEmpty(message.Data))
322326
{
323-
output.Add(message.Data);
327+
lock (outputLock)
328+
{
329+
output.Add(message.Data);
330+
}
324331
}
325-
}
326-
};
332+
};
327333

328-
processInvoker.ErrorDataReceived += delegate (object sender, ProcessDataReceivedEventArgs message)
329-
{
330-
if (!string.IsNullOrEmpty(message.Data))
334+
processInvoker.ErrorDataReceived += delegate (object sender, ProcessDataReceivedEventArgs message)
331335
{
332-
lock (outputLock)
336+
if (!string.IsNullOrEmpty(message.Data))
333337
{
334-
output.Add(message.Data);
338+
lock (outputLock)
339+
{
340+
output.Add(message.Data);
341+
}
335342
}
336-
}
337-
};
338-
339-
if (!Constants.Runner.Platform.Equals(Constants.OSPlatform.Linux))
340-
{
341-
throw new NotSupportedException("Container operations are only supported on Linux runners");
343+
};
344+
345+
return await processInvoker.ExecuteAsync(
346+
workingDirectory: HostContext.GetDirectory(WellKnownDirectory.Work),
347+
fileName: DockerPath,
348+
arguments: arg,
349+
environment: null,
350+
requireExitCodeZero: false,
351+
outputEncoding: null,
352+
cancellationToken: CancellationToken.None);
342353
}
343-
return await processInvoker.ExecuteAsync(
344-
workingDirectory: HostContext.GetDirectory(WellKnownDirectory.Work),
345-
fileName: DockerPath,
346-
arguments: arg,
347-
environment: null,
348-
requireExitCodeZero: false,
349-
outputEncoding: null,
350-
cancellationToken: CancellationToken.None);
351354
}
352355

353356
public async Task<List<string>> DockerInspect(IExecutionContext context, string dockerObject, string options)
@@ -361,26 +364,27 @@ public async Task<List<PortMapping>> DockerPort(IExecutionContext context, strin
361364
return DockerUtil.ParseDockerPort(portMappingLines);
362365
}
363366

364-
public Task<int> DockerLogin(IExecutionContext context, string configFileDirectory, string registry, string username, string password)
367+
public async Task<int> DockerLogin(IExecutionContext context, string configFileDirectory, string registry, string username, string password)
365368
{
366369
string args = $"--config {configFileDirectory} login {registry} -u {username} --password-stdin";
367370
context.Command($"{DockerPath} {args}");
368371

369372
var input = Channel.CreateBounded<string>(new BoundedChannelOptions(1) { SingleReader = true, SingleWriter = true });
370373
input.Writer.TryWrite(password);
371374

372-
var processInvoker = HostContext.CreateService<IProcessInvoker>();
373-
374-
return processInvoker.ExecuteAsync(
375-
workingDirectory: context.GetGitHubContext("workspace"),
376-
fileName: DockerPath,
377-
arguments: args,
378-
environment: null,
379-
requireExitCodeZero: false,
380-
outputEncoding: null,
381-
killProcessOnCancel: false,
382-
redirectStandardIn: input,
383-
cancellationToken: context.CancellationToken);
375+
using (var processInvoker = HostContext.CreateService<IProcessInvoker>())
376+
{
377+
return await processInvoker.ExecuteAsync(
378+
workingDirectory: context.GetGitHubContext("workspace"),
379+
fileName: DockerPath,
380+
arguments: args,
381+
environment: null,
382+
requireExitCodeZero: false,
383+
outputEncoding: null,
384+
killProcessOnCancel: false,
385+
redirectStandardIn: input,
386+
cancellationToken: context.CancellationToken);
387+
}
384388
}
385389

386390
private Task<int> ExecuteDockerCommandAsync(IExecutionContext context, string command, string options, CancellationToken cancellationToken = default(CancellationToken))
@@ -390,59 +394,64 @@ public Task<int> DockerLogin(IExecutionContext context, string configFileDirecto
390394

391395
private async Task<int> ExecuteDockerCommandAsync(IExecutionContext context, string command, string options, IDictionary<string, string> environment, EventHandler<ProcessDataReceivedEventArgs> stdoutDataReceived, EventHandler<ProcessDataReceivedEventArgs> stderrDataReceived, CancellationToken cancellationToken = default(CancellationToken))
392396
{
397+
if (!Constants.Runner.Platform.Equals(Constants.OSPlatform.Linux))
398+
{
399+
throw new NotSupportedException("Container operations are only supported on Linux runners");
400+
}
401+
393402
string arg = $"{command} {options}".Trim();
394403
context.Command($"{DockerPath} {arg}");
395404

396-
var processInvoker = HostContext.CreateService<IProcessInvoker>();
397-
processInvoker.OutputDataReceived += stdoutDataReceived;
398-
processInvoker.ErrorDataReceived += stderrDataReceived;
399-
400-
401-
if (!Constants.Runner.Platform.Equals(Constants.OSPlatform.Linux))
405+
using (var processInvoker = HostContext.CreateService<IProcessInvoker>())
402406
{
403-
throw new NotSupportedException("Container operations are only supported on Linux runners");
407+
processInvoker.OutputDataReceived += stdoutDataReceived;
408+
processInvoker.ErrorDataReceived += stderrDataReceived;
409+
410+
return await processInvoker.ExecuteAsync(
411+
workingDirectory: context.GetGitHubContext("workspace"),
412+
fileName: DockerPath,
413+
arguments: arg,
414+
environment: environment,
415+
requireExitCodeZero: false,
416+
outputEncoding: null,
417+
killProcessOnCancel: false,
418+
cancellationToken: cancellationToken);
404419
}
405-
return await processInvoker.ExecuteAsync(
406-
workingDirectory: context.GetGitHubContext("workspace"),
407-
fileName: DockerPath,
408-
arguments: arg,
409-
environment: environment,
410-
requireExitCodeZero: false,
411-
outputEncoding: null,
412-
killProcessOnCancel: false,
413-
cancellationToken: cancellationToken);
414420
}
415421

416422
private async Task<int> ExecuteDockerCommandAsync(IExecutionContext context, string command, string options, string workingDirectory, CancellationToken cancellationToken = default(CancellationToken))
417423
{
424+
if (!Constants.Runner.Platform.Equals(Constants.OSPlatform.Linux))
425+
{
426+
throw new NotSupportedException("Container operations are only supported on Linux runners");
427+
}
428+
418429
string arg = $"{command} {options}".Trim();
419430
context.Command($"{DockerPath} {arg}");
420431

421-
var processInvoker = HostContext.CreateService<IProcessInvoker>();
422-
processInvoker.OutputDataReceived += delegate (object sender, ProcessDataReceivedEventArgs message)
423-
{
424-
context.Output(message.Data);
425-
};
426-
427-
processInvoker.ErrorDataReceived += delegate (object sender, ProcessDataReceivedEventArgs message)
432+
using (var processInvoker = HostContext.CreateService<IProcessInvoker>())
428433
{
429-
context.Output(message.Data);
430-
};
434+
processInvoker.OutputDataReceived += delegate (object sender, ProcessDataReceivedEventArgs message)
435+
{
436+
context.Output(message.Data);
437+
};
431438

432-
if (!Constants.Runner.Platform.Equals(Constants.OSPlatform.Linux))
433-
{
434-
throw new NotSupportedException("Container operations are only supported on Linux runners");
439+
processInvoker.ErrorDataReceived += delegate (object sender, ProcessDataReceivedEventArgs message)
440+
{
441+
context.Output(message.Data);
442+
};
443+
444+
return await processInvoker.ExecuteAsync(
445+
workingDirectory: workingDirectory ?? context.GetGitHubContext("workspace"),
446+
fileName: DockerPath,
447+
arguments: arg,
448+
environment: null,
449+
requireExitCodeZero: false,
450+
outputEncoding: null,
451+
killProcessOnCancel: false,
452+
redirectStandardIn: null,
453+
cancellationToken: cancellationToken);
435454
}
436-
return await processInvoker.ExecuteAsync(
437-
workingDirectory: workingDirectory ?? context.GetGitHubContext("workspace"),
438-
fileName: DockerPath,
439-
arguments: arg,
440-
environment: null,
441-
requireExitCodeZero: false,
442-
outputEncoding: null,
443-
killProcessOnCancel: false,
444-
redirectStandardIn: null,
445-
cancellationToken: cancellationToken);
446455
}
447456

448457
private async Task<List<string>> ExecuteDockerCommandAsync(IExecutionContext context, string command, string options)
@@ -451,32 +460,34 @@ private async Task<List<string>> ExecuteDockerCommandAsync(IExecutionContext con
451460
context.Command($"{DockerPath} {arg}");
452461

453462
List<string> output = new();
454-
var processInvoker = HostContext.CreateService<IProcessInvoker>();
455-
processInvoker.OutputDataReceived += delegate (object sender, ProcessDataReceivedEventArgs message)
463+
using (var processInvoker = HostContext.CreateService<IProcessInvoker>())
456464
{
457-
if (!string.IsNullOrEmpty(message.Data))
465+
processInvoker.OutputDataReceived += delegate (object sender, ProcessDataReceivedEventArgs message)
458466
{
459-
output.Add(message.Data);
460-
context.Output(message.Data);
461-
}
462-
};
467+
if (!string.IsNullOrEmpty(message.Data))
468+
{
469+
output.Add(message.Data);
470+
context.Output(message.Data);
471+
}
472+
};
463473

464-
processInvoker.ErrorDataReceived += delegate (object sender, ProcessDataReceivedEventArgs message)
465-
{
466-
if (!string.IsNullOrEmpty(message.Data))
474+
processInvoker.ErrorDataReceived += delegate (object sender, ProcessDataReceivedEventArgs message)
467475
{
468-
context.Output(message.Data);
469-
}
470-
};
471-
472-
await processInvoker.ExecuteAsync(
473-
workingDirectory: context.GetGitHubContext("workspace"),
474-
fileName: DockerPath,
475-
arguments: arg,
476-
environment: null,
477-
requireExitCodeZero: true,
478-
outputEncoding: null,
479-
cancellationToken: CancellationToken.None);
476+
if (!string.IsNullOrEmpty(message.Data))
477+
{
478+
context.Output(message.Data);
479+
}
480+
};
481+
482+
await processInvoker.ExecuteAsync(
483+
workingDirectory: context.GetGitHubContext("workspace"),
484+
fileName: DockerPath,
485+
arguments: arg,
486+
environment: null,
487+
requireExitCodeZero: true,
488+
outputEncoding: null,
489+
cancellationToken: CancellationToken.None);
490+
}
480491

481492
return output;
482493
}

‎src/Runner.Worker/ContainerOperationProvider.cs‎

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -362,37 +362,39 @@ private async Task<List<string>> ExecuteCommandAsync(IExecutionContext context,
362362

363363
List<string> outputs = new();
364364
object outputLock = new();
365-
var processInvoker = HostContext.CreateService<IProcessInvoker>();
366-
processInvoker.OutputDataReceived += delegate (object sender, ProcessDataReceivedEventArgs message)
365+
using (var processInvoker = HostContext.CreateService<IProcessInvoker>())
367366
{
368-
if (!string.IsNullOrEmpty(message.Data))
367+
processInvoker.OutputDataReceived += delegate (object sender, ProcessDataReceivedEventArgs message)
369368
{
370-
lock (outputLock)
369+
if (!string.IsNullOrEmpty(message.Data))
371370
{
372-
outputs.Add(message.Data);
371+
lock (outputLock)
372+
{
373+
outputs.Add(message.Data);
374+
}
373375
}
374-
}
375-
};
376+
};
376377

377-
processInvoker.ErrorDataReceived += delegate (object sender, ProcessDataReceivedEventArgs message)
378-
{
379-
if (!string.IsNullOrEmpty(message.Data))
378+
processInvoker.ErrorDataReceived += delegate (object sender, ProcessDataReceivedEventArgs message)
380379
{
381-
lock (outputLock)
380+
if (!string.IsNullOrEmpty(message.Data))
382381
{
383-
outputs.Add(message.Data);
382+
lock (outputLock)
383+
{
384+
outputs.Add(message.Data);
385+
}
384386
}
385-
}
386-
};
387-
388-
await processInvoker.ExecuteAsync(
389-
workingDirectory: HostContext.GetDirectory(WellKnownDirectory.Work),
390-
fileName: command,
391-
arguments: arg,
392-
environment: null,
393-
requireExitCodeZero: true,
394-
outputEncoding: null,
395-
cancellationToken: CancellationToken.None);
387+
};
388+
389+
await processInvoker.ExecuteAsync(
390+
workingDirectory: HostContext.GetDirectory(WellKnownDirectory.Work),
391+
fileName: command,
392+
arguments: arg,
393+
environment: null,
394+
requireExitCodeZero: true,
395+
outputEncoding: null,
396+
cancellationToken: CancellationToken.None);
397+
}
396398

397399
foreach (var outputLine in outputs)
398400
{

0 commit comments

Comments
 (0)