Skip to content

Commit

Permalink
Dispose root context before unloading process. (#2430)
Browse files Browse the repository at this point in the history 
* Dispose root context before unloading process.

* Ensure root context is disposed.

* Add vertical whitespace.
  • Loading branch information
@michaelcfanning
michaelcfanning committed Jan 27, 2022
1 parent 64a4b7e commit c18abd7
Show file tree
Hide file tree
Showing 8 changed files with 165 additions and 123 deletions.
133 changes: 70 additions & 63 deletions src/Sarif.Driver/Sdk/AnalyzeCommandBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public abstract class AnalyzeCommandBase<TContext, TOptions> : PluginDriverComma

private Run _run;
private Tool _tool;
private TContext _rootContext;
internal TContext _rootContext;
private CacheByFileHashLogger _cacheByFileHashLogger;
private IDictionary<string, HashData> _pathToHashDataMap;

Expand Down Expand Up @@ -55,87 +55,94 @@ public string DefaultConfigurationPath

public override int Run(TOptions options)
{
// To correctly initialize the logger, we must first add Hashes to dataToInsert
#pragma warning disable CS0618 // Type or member is obsolete
if (options.ComputeFileHashes)
#pragma warning restore CS0618
{
OptionallyEmittedData dataToInsert = options.DataToInsert.ToFlags();
dataToInsert |= OptionallyEmittedData.Hashes;

options.DataToInsert = Enum.GetValues(typeof(OptionallyEmittedData)).Cast<OptionallyEmittedData>()
.Where(oed => dataToInsert.HasFlag(oed)).ToList();
}

// 0. Initialize an common logger that drives all outputs. This
// object drives logging for console, statistics, etc.
using (AggregatingLogger logger = InitializeLogger(options))
try
{
// Once the logger has been correctly initialized, we can raise a warning
_rootContext = CreateContext(options, logger, RuntimeErrors);
// To correctly initialize the logger, we must first add Hashes to dataToInsert
#pragma warning disable CS0618 // Type or member is obsolete
if (options.ComputeFileHashes)
#pragma warning restore CS0618
{
Warnings.LogObsoleteOption(_rootContext, "--hashes", SdkResources.ComputeFileHashes_ReplaceInsertHashes);
}
OptionallyEmittedData dataToInsert = options.DataToInsert.ToFlags();
dataToInsert |= OptionallyEmittedData.Hashes;

try
{
Analyze(options, logger);
}
catch (ExitApplicationException<ExitReason> ex)
{
// These exceptions have already been logged
ExecutionException = ex;
return FAILURE;
}
catch (Exception ex)
{
// These exceptions escaped our net and must be logged here
RuntimeErrors |= Errors.LogUnhandledEngineException(_rootContext, ex);
ExecutionException = ex;
return FAILURE;
options.DataToInsert = Enum.GetValues(typeof(OptionallyEmittedData)).Cast<OptionallyEmittedData>()
.Where(oed => dataToInsert.HasFlag(oed)).ToList();
}
finally

// 0. Initialize an common logger that drives all outputs. This
// object drives logging for console, statistics, etc.
using (AggregatingLogger logger = InitializeLogger(options))
{
logger.AnalysisStopped(RuntimeErrors);
// Once the logger has been correctly initialized, we can raise a warning
_rootContext = CreateContext(options, logger, RuntimeErrors);
#pragma warning disable CS0618 // Type or member is obsolete
if (options.ComputeFileHashes)
#pragma warning restore CS0618
{
Warnings.LogObsoleteOption(_rootContext, "--hashes", SdkResources.ComputeFileHashes_ReplaceInsertHashes);
}

try
{
Analyze(options, logger);
}
catch (ExitApplicationException<ExitReason> ex)
{
// These exceptions have already been logged
ExecutionException = ex;
return FAILURE;
}
catch (Exception ex)
{
// These exceptions escaped our net and must be logged here
RuntimeErrors |= Errors.LogUnhandledEngineException(_rootContext, ex);
ExecutionException = ex;
return FAILURE;
}
finally
{
logger.AnalysisStopped(RuntimeErrors);
}
}
}

bool succeeded = (RuntimeErrors & ~RuntimeConditions.Nonfatal) == RuntimeConditions.None;
bool succeeded = (RuntimeErrors & ~RuntimeConditions.Nonfatal) == RuntimeConditions.None;

if (succeeded)
{
try
if (succeeded)
{
ProcessBaseline(_rootContext, options, FileSystem);
}
catch (Exception ex)
{
RuntimeErrors |= RuntimeConditions.ExceptionProcessingBaseline;
ExecutionException = ex;
return FAILURE;
}
try
{
ProcessBaseline(_rootContext, options, FileSystem);
}
catch (Exception ex)
{
RuntimeErrors |= RuntimeConditions.ExceptionProcessingBaseline;
ExecutionException = ex;
return FAILURE;
}

try
{
PostLogFile(options.PostUri, options.OutputFilePath, FileSystem);
try
{
PostLogFile(options.PostUri, options.OutputFilePath, FileSystem);
}
catch (Exception ex)
{
RuntimeErrors |= RuntimeConditions.ExceptionPostingLogFile;
ExecutionException = ex;
return FAILURE;
}
}
catch (Exception ex)

if (options.RichReturnCode)
{
RuntimeErrors |= RuntimeConditions.ExceptionPostingLogFile;
ExecutionException = ex;
return FAILURE;
return (int)RuntimeErrors;
}
}

if (options.RichReturnCode)
return succeeded ? SUCCESS : FAILURE;
}
finally
{
return (int)RuntimeErrors;
_rootContext?.Dispose();
}

return succeeded ? SUCCESS : FAILURE;
}

private void Analyze(TOptions options, AggregatingLogger logger)
Expand Down
114 changes: 60 additions & 54 deletions src/Sarif.Driver/Sdk/MultithreadedAnalyzeCommandBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public abstract class MultithreadedAnalyzeCommandBase<TContext, TOptions> : Plug
private Run _run;
private Tool _tool;
private bool _computeHashes;
private TContext _rootContext;
internal TContext _rootContext;
private int _fileContextsCount;
private Channel<int> _hashChannel;
private OptionallyEmittedData _dataToInsert;
Expand Down Expand Up @@ -66,71 +66,78 @@ public string DefaultConfigurationPath

public override int Run(TOptions options)
{
// Initialize an common logger that drives all outputs. This
// object drives logging for console, statistics, etc.
using (AggregatingLogger logger = InitializeLogger(options))
try
{
try
{
Analyze(options, logger);
}
catch (ExitApplicationException<ExitReason> ex)
// Initialize an common logger that drives all outputs. This
// object drives logging for console, statistics, etc.
using (AggregatingLogger logger = InitializeLogger(options))
{
// These exceptions have already been logged
ExecutionException = ex;
return FAILURE;
}
catch (Exception ex)
{
ex = ex.InnerException ?? ex;
try
{
Analyze(options, logger);
}
catch (ExitApplicationException<ExitReason> ex)
{
// These exceptions have already been logged
ExecutionException = ex;
return FAILURE;
}
catch (Exception ex)
{
ex = ex.InnerException ?? ex;

if (!(ex is ExitApplicationException<ExitReason>))
if (!(ex is ExitApplicationException<ExitReason>))
{
// These exceptions escaped our net and must be logged here
RuntimeErrors |= Errors.LogUnhandledEngineException(_rootContext, ex);
}
ExecutionException = ex;
return FAILURE;
}
finally
{
// These exceptions escaped our net and must be logged here
RuntimeErrors |= Errors.LogUnhandledEngineException(_rootContext, ex);
logger.AnalysisStopped(RuntimeErrors);
}
ExecutionException = ex;
return FAILURE;
}
finally
{
logger.AnalysisStopped(RuntimeErrors);
}
}

bool succeeded = (RuntimeErrors & ~RuntimeConditions.Nonfatal) == RuntimeConditions.None;
bool succeeded = (RuntimeErrors & ~RuntimeConditions.Nonfatal) == RuntimeConditions.None;

if (succeeded)
{
try
if (succeeded)
{
ProcessBaseline(_rootContext, options, FileSystem);
}
catch (Exception ex)
{
RuntimeErrors |= RuntimeConditions.ExceptionProcessingBaseline;
ExecutionException = ex;
return FAILURE;
}
try
{
ProcessBaseline(_rootContext, options, FileSystem);
}
catch (Exception ex)
{
RuntimeErrors |= RuntimeConditions.ExceptionProcessingBaseline;
ExecutionException = ex;
return FAILURE;
}

try
{
PostLogFile(options.PostUri, options.OutputFilePath, FileSystem);
try
{
PostLogFile(options.PostUri, options.OutputFilePath, FileSystem);
}
catch (Exception ex)
{
RuntimeErrors |= RuntimeConditions.ExceptionPostingLogFile;
ExecutionException = ex;
return FAILURE;
}
}
catch (Exception ex)

if (options.RichReturnCode)
{
RuntimeErrors |= RuntimeConditions.ExceptionPostingLogFile;
ExecutionException = ex;
return FAILURE;
return (int)RuntimeErrors;
}
}

if (options.RichReturnCode)
return succeeded ? SUCCESS : FAILURE;
}
finally
{
return (int)RuntimeErrors;
_rootContext?.Dispose();
}

return succeeded ? SUCCESS : FAILURE;
}

private void Analyze(TOptions analyzeOptions, AggregatingLogger logger)
Expand Down Expand Up @@ -805,10 +812,9 @@ private SupportedPlatform GetCurrentRunningOS()
#endif
}

protected virtual void AnalyzeTargets(
TOptions options,
TContext rootContext,
IEnumerable<Skimmer<TContext>> skimmers)
protected virtual void AnalyzeTargets(TOptions options,
TContext rootContext,
IEnumerable<Skimmer<TContext>> skimmers)
{
var disabledSkimmers = new SortedSet<string>();

Expand Down
2 changes: 1 addition & 1 deletion src/Sarif/Core/SarifLog.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ public static SarifLog Load(Stream source, bool deferred = false)

if (!fileSystem.FileExists(filePath))
{
throw new ArgumentException(nameof(filePath));
throw new ArgumentException($"File path does not exist: '{filePath}'", nameof(filePath));
}

using Stream fileStream = fileSystem.FileOpenRead(filePath);
Expand Down
19 changes: 16 additions & 3 deletions src/Test.UnitTests.Sarif.Driver/Sdk/AnalyzeCommandBaseTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,19 @@ public AnalyzeCommandBaseTests(ITestOutputHelper output)
this.Output = output;
}

[Fact]
public void AnalyzeCommandBase_RootContextIsDisposed()
{
var options = new TestAnalyzeOptions();
var singleThreadedCommand = new TestAnalyzeCommand();
int result = singleThreadedCommand.Run(options);
singleThreadedCommand._rootContext.Disposed.Should().BeTrue();

var multithreadedAnalyzeCommand = new TestMultithreadedAnalyzeCommand();
result = singleThreadedCommand.Run(options);
singleThreadedCommand._rootContext.Disposed.Should().BeTrue();
}

private void ExceptionTestHelper(
RuntimeConditions runtimeConditions,
ExitReason expectedExitReason = ExitReason.None,
Expand Down Expand Up @@ -1237,10 +1250,10 @@ private void AnalyzeScenarios(int[] scenarios)

testCase.FileSystem = null;
testCase.Files = multiThreadTargets;
Run runMultiThread = RunAnalyzeCommand(options, testCase, multithreaded: true);
Run runMultithreaded = RunAnalyzeCommand(options, testCase, multithreaded: true);

runMultiThread.Results.Should().BeEquivalentTo(runSingleThread.Results);
runMultiThread.Artifacts.Should().BeEquivalentTo(runSingleThread.Artifacts);
runMultithreaded.Results.Should().BeEquivalentTo(runSingleThread.Results);
runMultithreaded.Artifacts.Should().BeEquivalentTo(runSingleThread.Artifacts);
}
}

Expand Down
9 changes: 9 additions & 0 deletions src/Test.UnitTests.Sarif.Driver/TestAnalyzeCommand.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
using System.Collections.Generic;
using System.Reflection;

using FluentAssertions;

namespace Microsoft.CodeAnalysis.Sarif.Driver
{
public class TestAnalyzeCommand : AnalyzeCommandBase<TestAnalysisContext, TestAnalyzeOptions>, ITestAnalyzeCommand
Expand Down Expand Up @@ -70,5 +72,12 @@ protected override void ProcessBaseline(IAnalysisContext context, TestAnalyzeOpt

base.ProcessBaseline(context, options, fileSystem);
}

public override int Run(TestAnalyzeOptions options)
{
int result = base.Run(options);
this._rootContext?.Disposed.Should().BeTrue();
return result;
}
}
}
Loading

0 comments on commit c18abd7

Please sign in to comment.