Skip to content

Commit

Permalink
fix: Concurrency issue with MutantPlacer (#2914)
Browse files Browse the repository at this point in the history 
fix: random exception during tests (issue #2916)
  • Loading branch information
@dupdob
dupdob authored Apr 17, 2024
1 parent 4b4e5ad commit fae558c
Show file tree
Hide file tree
Showing 4 changed files with 110 additions and 94 deletions.
2 changes: 1 addition & 1 deletion src/Stryker.Core/Stryker.Core/Mutants/BaseMutantOrchestrator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ public abstract class BaseMutantOrchestrator

public ICollection<Mutant> Mutants { get; set; }

public int MutantCount { get; set; }
protected int MutantCount;

protected BaseMutantOrchestrator(StrykerOptions options) => Options = options;

Expand Down
157 changes: 80 additions & 77 deletions src/Stryker.Core/Stryker.Core/Mutants/CsharpMutantOrchestrator.cs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Linq;
using System.Threading;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
Expand All @@ -16,11 +17,15 @@ namespace Stryker.Core.Mutants;
/// <inheritdoc/>
public class CsharpMutantOrchestrator : BaseMutantOrchestrator<SyntaxTree, SemanticModel>
{
private readonly TypeBasedStrategy<SyntaxNode, INodeOrchestrator> _specificOrchestrator =
private static readonly TypeBasedStrategy<SyntaxNode, INodeOrchestrator> specificOrchestrator =
new();

private ILogger Logger { get; }

static CsharpMutantOrchestrator() =>
// declare node specific orchestrators. Note that order is relevant, they should be declared from more specific to more generic one
specificOrchestrator.RegisterHandlers(BuildOrchestratorList());

/// <summary>
/// <param name="mutators">The mutators that should be active during the mutation process</param>
/// </summary>
Expand All @@ -30,85 +35,83 @@ public CsharpMutantOrchestrator(MutantPlacer placer, IEnumerable<IMutator> mutat
Mutators = mutators ?? DefaultMutatorList();
Mutants = new Collection<Mutant>();
Logger = ApplicationLogging.LoggerFactory.CreateLogger<CsharpMutantOrchestrator>();

// declare node specific orchestrators. Note that order is relevant, they should be declared from more specific to more generic one
_specificOrchestrator.RegisterHandlers(BuildOrchestratorList());

}

private static List<INodeOrchestrator> BuildOrchestratorList() =>
new()
{
// Those node types describe compile time constants and thus cannot be mutated at run time
// attributes
new DoNotMutateOrchestrator<AttributeListSyntax>(),
// parameter list
new DoNotMutateOrchestrator<ParameterListSyntax>(),
// enum values
new DoNotMutateOrchestrator<EnumMemberDeclarationSyntax>(),
// pattern marching
new DoNotMutateOrchestrator<RecursivePatternSyntax>(),
new DoNotMutateOrchestrator<UsingDirectiveSyntax>(),
// constants and constant fields
new DoNotMutateOrchestrator<FieldDeclarationSyntax>(t => t.Modifiers.Any(x => x.IsKind(SyntaxKind.ConstKeyword))),
new DoNotMutateOrchestrator<LocalDeclarationStatementSyntax>(t => t.IsConst),
// ensure pre/post increment/decrement mutations are mutated at statement level
new MutateAtStatementLevelOrchestrator<PostfixUnaryExpressionSyntax>( t => t.Parent is ExpressionStatementSyntax or ForStatementSyntax),
new MutateAtStatementLevelOrchestrator<PrefixUnaryExpressionSyntax>( t => t.Parent is ExpressionStatementSyntax or ForStatementSyntax),
// prevent mutations to happen within member access expression
new MemberAccessExpressionOrchestrator<MemberAccessExpressionSyntax>(),
new MemberAccessExpressionOrchestrator<MemberBindingExpressionSyntax>(),
new MemberAccessExpressionOrchestrator<SimpleNameSyntax>(),
new MemberAccessExpressionOrchestrator<PostfixUnaryExpressionSyntax>(t => t.IsKind(SyntaxKind.SuppressNullableWarningExpression)),
new ConditionalExpressionOrchestrator(),
// ensure static constructs are marked properly
new StaticFieldDeclarationOrchestrator(),
new StaticConstructorOrchestrator(),
// ensure array initializer mutations are controlled at statement level
new MutateAtStatementLevelOrchestrator<InitializerExpressionSyntax>( t => t.Kind() == SyntaxKind.ArrayInitializerExpression && t.Expressions.Count > 0),
// ensure properties are properly mutated (including expression to body conversion if required)
new ExpressionBodiedPropertyOrchestrator(),
// ensure method, lambda... are properly mutated (including expression to body conversion if required)
new LocalFunctionStatementOrchestrator(),
new AnonymousFunctionExpressionOrchestrator(),
new BaseMethodDeclarationOrchestrator<BaseMethodDeclarationSyntax>(),
new AccessorSyntaxOrchestrator(),
// ensure declaration are mutated at the block level
new LocalDeclarationOrchestrator(),
new InvocationExpressionOrchestrator(),
new MemberDefinitionOrchestrator<MemberDeclarationSyntax>(),
new MutateAtStatementLevelOrchestrator<AssignmentExpressionSyntax>(),
new BlockOrchestrator(),
new StatementSpecificOrchestrator<StatementSyntax>(),
new ExpressionSpecificOrchestrator<ExpressionSyntax>(),
new SyntaxNodeOrchestrator()
};
[
new DoNotMutateOrchestrator<AttributeListSyntax>(),
// parameter list
new DoNotMutateOrchestrator<ParameterListSyntax>(),
// enum values
new DoNotMutateOrchestrator<EnumMemberDeclarationSyntax>(),
// pattern marching
new DoNotMutateOrchestrator<RecursivePatternSyntax>(),
new DoNotMutateOrchestrator<UsingDirectiveSyntax>(),
// constants and constant fields
new DoNotMutateOrchestrator<FieldDeclarationSyntax>(
t => t.Modifiers.Any(x => x.IsKind(SyntaxKind.ConstKeyword))),
new DoNotMutateOrchestrator<LocalDeclarationStatementSyntax>(t => t.IsConst),
// ensure pre/post increment/decrement mutations are mutated at statement level
new MutateAtStatementLevelOrchestrator<PostfixUnaryExpressionSyntax>(t =>
t.Parent is ExpressionStatementSyntax or ForStatementSyntax),
new MutateAtStatementLevelOrchestrator<PrefixUnaryExpressionSyntax>(t =>
t.Parent is ExpressionStatementSyntax or ForStatementSyntax),
// prevent mutations to happen within member access expression
new MemberAccessExpressionOrchestrator<MemberAccessExpressionSyntax>(),
new MemberAccessExpressionOrchestrator<MemberBindingExpressionSyntax>(),
new MemberAccessExpressionOrchestrator<SimpleNameSyntax>(),
new MemberAccessExpressionOrchestrator<PostfixUnaryExpressionSyntax>(t =>
t.IsKind(SyntaxKind.SuppressNullableWarningExpression)),
new ConditionalExpressionOrchestrator(),
// ensure static constructs are marked properly
new StaticFieldDeclarationOrchestrator(),
new StaticConstructorOrchestrator(),
// ensure array initializer mutations are controlled at statement level
new MutateAtStatementLevelOrchestrator<InitializerExpressionSyntax>(t =>
t.Kind() == SyntaxKind.ArrayInitializerExpression && t.Expressions.Count > 0),
// ensure properties are properly mutated (including expression to body conversion if required)
new ExpressionBodiedPropertyOrchestrator(),
// ensure method, lambda... are properly mutated (including expression to body conversion if required)
new LocalFunctionStatementOrchestrator(),
new AnonymousFunctionExpressionOrchestrator(),
new BaseMethodDeclarationOrchestrator<BaseMethodDeclarationSyntax>(),
new AccessorSyntaxOrchestrator(),
// ensure declaration are mutated at the block level
new LocalDeclarationOrchestrator(),
new InvocationExpressionOrchestrator(),
new MemberDefinitionOrchestrator<MemberDeclarationSyntax>(),
new MutateAtStatementLevelOrchestrator<AssignmentExpressionSyntax>(),
new BlockOrchestrator(),
new StatementSpecificOrchestrator<StatementSyntax>(),
new ExpressionSpecificOrchestrator<ExpressionSyntax>(),
new SyntaxNodeOrchestrator()
];

private static List<IMutator> DefaultMutatorList() =>
new()
{
// the default list of mutators
new BinaryExpressionMutator(),
new BlockMutator(),
new BooleanMutator(),
new AssignmentExpressionMutator(),
new PrefixUnaryMutator(),
new PostfixUnaryMutator(),
new CheckedMutator(),
new LinqMutator(),
new StringMutator(),
new StringEmptyMutator(),
new InterpolatedStringMutator(),
new NegateConditionMutator(),
new InitializerMutator(),
new ObjectCreationMutator(),
new ArrayCreationMutator(),
new StatementMutator(),
new RegexMutator(),
new NullCoalescingExpressionMutator(),
new MathMutator(),
new SwitchExpressionMutator(),
new IsPatternExpressionMutator()
};
[
new BinaryExpressionMutator(),
new BlockMutator(),
new BooleanMutator(),
new AssignmentExpressionMutator(),
new PrefixUnaryMutator(),
new PostfixUnaryMutator(),
new CheckedMutator(),
new LinqMutator(),
new StringMutator(),
new StringEmptyMutator(),
new InterpolatedStringMutator(),
new NegateConditionMutator(),
new InitializerMutator(),
new ObjectCreationMutator(),
new ArrayCreationMutator(),
new StatementMutator(),
new RegexMutator(),
new NullCoalescingExpressionMutator(),
new MathMutator(),
new SwitchExpressionMutator(),
new IsPatternExpressionMutator()
];

private IEnumerable<IMutator> Mutators { get; }

Expand All @@ -124,7 +127,7 @@ public override SyntaxTree Mutate(SyntaxTree input, SemanticModel semanticModel)
// search for node specific handler
input.WithRootAndOptions(GetHandler(input.GetRoot()).Mutate(input.GetRoot(), semanticModel, new MutationContext(this)), input.Options);

internal INodeOrchestrator GetHandler(SyntaxNode currentNode) => _specificOrchestrator.FindHandler(currentNode);
internal INodeOrchestrator GetHandler(SyntaxNode currentNode) => specificOrchestrator.FindHandler(currentNode);

internal IEnumerable<Mutant> GenerateMutationsForNode(SyntaxNode current, SemanticModel semanticModel, MutationContext context)
{
Expand All @@ -142,7 +145,7 @@ internal IEnumerable<Mutant> GenerateMutationsForNode(SyntaxNode current, Semant
}

Mutants.Add(newMutant);
MutantCount++;
Interlocked.Increment(ref MutantCount);
mutations.Add(newMutant);
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/Stryker.Core/Stryker.Core/Mutants/CsharpNodeOrchestrators/BaseFunctionOrchestrator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ namespace Stryker.Core.Mutants.CsharpNodeOrchestrators;
/// <remarks>This class is helpful because there is no (useful) shared parent class for those syntax construct</remarks>
internal abstract class BaseFunctionOrchestrator<T> :MemberDefinitionOrchestrator<T>, IInstrumentCode where T : SyntaxNode
{

protected BaseFunctionOrchestrator() => Marker = MutantPlacer.RegisterEngine(this, true);

private SyntaxAnnotation Marker { get; }
Expand Down Expand Up @@ -81,7 +82,7 @@ public SyntaxNode RemoveInstrumentation(SyntaxNode node)
{
if (node is not T typedNode)
{
throw new InvalidOperationException($"Expected a {typeof(T).ToString()}, found:\n{node.ToFullString()}.");
throw new InvalidOperationException($"Expected a {typeof(T)}, found:\n{node.ToFullString()}.");
}
var (block, _) = GetBodies(typedNode);
var expression = block?.Statements[0] switch
Expand All @@ -98,6 +99,7 @@ public SyntaxNode RemoveInstrumentation(SyntaxNode node)
protected override T InjectMutations(T sourceNode, T targetNode, SemanticModel semanticModel, MutationContext context)
{
var (blockBody, expressionBody) = GetBodies(targetNode);

if (expressionBody == null && blockBody == null)
{
// no implementation provided
Expand Down
41 changes: 26 additions & 15 deletions src/Stryker.Core/Stryker.Core/Mutants/MutantPlacer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ public class MutantPlacer
private const string MutationTypeMarker = "MutationType";
public static readonly string Injector = "Injector";

private static readonly IDictionary<string, IInstrumentCode> InstrumentEngines = new Dictionary<string, IInstrumentCode>();
private static readonly HashSet<string> RequireRecursiveRemoval = new();
private static readonly Dictionary<string, (IInstrumentCode engine, SyntaxAnnotation annotation)> instrumentEngines = new ();
private static readonly HashSet<string> requireRecursiveRemoval = new();

private static readonly StaticInstrumentationEngine StaticEngine = new();
private static readonly StaticInitializerMarkerEngine StaticInitializerEngine = new();
Expand All @@ -32,6 +32,7 @@ public class MutantPlacer
private readonly CodeInjection _injection;
private ExpressionSyntax _binaryExpression;
private SyntaxNode _placeHolderNode;

public static IEnumerable<string> MutationMarkers => new[] { MutationIdMarker, MutationTypeMarker, Injector };

public MutantPlacer(CodeInjection injection) => _injection = injection;
Expand All @@ -43,16 +44,26 @@ public class MutantPlacer
/// <param name="requireRecursive">true if inner injections should be removed first.</param>
public static SyntaxAnnotation RegisterEngine(IInstrumentCode engine, bool requireRecursive = false)
{
if (InstrumentEngines.TryGetValue(engine.InstrumentEngineId, out var existing) && existing!.GetType() != engine.GetType())
{
throw new InvalidOperationException($"Cannot register {engine.GetType().Name} as name {engine.InstrumentEngineId} is already registered to {existing.GetType().Name}.");
}
InstrumentEngines[engine.InstrumentEngineId] = engine;
if (requireRecursive)
lock(instrumentEngines)
{
RequireRecursiveRemoval.Add(engine.InstrumentEngineId);
if (instrumentEngines.TryGetValue(engine.InstrumentEngineId, out var existing))
{
if (existing.engine!.GetType() != engine.GetType())
{
throw new InvalidOperationException(
$"Cannot register {engine.GetType().Name} as name {engine.InstrumentEngineId} is already registered to {existing.engine.GetType().Name}.");
}
return existing.annotation;
}

var syntaxAnnotation = new SyntaxAnnotation(Injector, engine.InstrumentEngineId);
instrumentEngines[engine.InstrumentEngineId] = (engine, syntaxAnnotation) ;
if (requireRecursive)
{
requireRecursiveRemoval.Add(engine.InstrumentEngineId);
}
return syntaxAnnotation;
}
return new SyntaxAnnotation(Injector, engine.InstrumentEngineId);
}

/// <summary>
Expand Down Expand Up @@ -85,6 +96,7 @@ public ExpressionSyntax PlaceStaticContextMarker(ExpressionSyntax expression) =>
/// Add initialization for all out parameters
/// </summary>
/// <param name="block">block to augment with an initialization block</param>
/// <param name="parameters"></param>
/// <returns><paramref name="block"/> with assignment statements in a block.</returns>
/// <remarks>return <paramref name="block"/> if there is no 'out' parameter.</remarks>
public static BlockSyntax InjectOutParametersInitialization(BlockSyntax block, IEnumerable<ParameterSyntax> parameters) =>
Expand Down Expand Up @@ -130,10 +142,10 @@ public static SyntaxNode RemoveMutant(SyntaxNode nodeToRemove)
var annotatedNode = nodeToRemove.GetAnnotatedNodes(Injector).FirstOrDefault();
if (annotatedNode != null)
{
var engine = annotatedNode.GetAnnotations(Injector).First().Data;
if (!string.IsNullOrEmpty(engine))
var id = annotatedNode.GetAnnotations(Injector).First().Data;
if (!string.IsNullOrEmpty(id))
{
var restoredNode = InstrumentEngines[engine].RemoveInstrumentation(annotatedNode);
var restoredNode = instrumentEngines[id].engine.RemoveInstrumentation(annotatedNode);
return annotatedNode == nodeToRemove ? restoredNode : nodeToRemove.ReplaceNode(annotatedNode, restoredNode);
}
}
Expand All @@ -152,7 +164,7 @@ public static bool RequiresRemovingChildMutations(SyntaxNode node)
{
throw new InvalidOperationException("No mutation in this node!");
}
return annotations.Exists(a => RequireRecursiveRemoval.Contains(a.Data));
return annotations.Exists(a => requireRecursiveRemoval.Contains(a.Data));
}

/// <summary>
Expand Down Expand Up @@ -208,5 +220,4 @@ private ExpressionSyntax GetBinaryExpression(int mutantId)
return _binaryExpression.ReplaceNode(_placeHolderNode,
SyntaxFactory.LiteralExpression(SyntaxKind.NumericLiteralExpression, SyntaxFactory.Literal(mutantId)));
}

}

0 comments on commit fae558c

Please sign in to comment.