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

Jump to bottom

Possible race in an Assert recently added to Compilation.GetTypeByMetadataName #59395

Closed
AlekseyTs opened this issue Feb 8, 2022 · 3 comments
Closed

Possible race in an Assert recently added to Compilation.GetTypeByMetadataName #59395

AlekseyTs opened this issue Feb 8, 2022 · 3 comments
Assignees
@333fred
Labels
Area-Compilers Bug
Milestone
17.3

Comments

@AlekseyTs
Copy link
Contributor

The following assert fails from time to time in context of different unit-tests:

Debug.Assert(result || (_getTypeCache.TryGetValue(fullyQualifiedMetadataName, out var addedType) && ReferenceEquals(addedType, val)));

Example #1:

Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.ImplementInterface.ImplementInterfaceTests.TestIntegralAndFloatLiterals3s 529ms
Error:
System.InvalidOperationException : Assertion failed

Stack trace:
   at Microsoft.CodeAnalysis.ThrowingTraceListener.Fail(String message, String detailMessage) in D:\GitHub\roslyn\src\Compilers\Test\Core\ThrowingTraceListener.cs:line 26
   at System.Diagnostics.TraceListener.Fail(String message)
   at System.Diagnostics.TraceInternal.Fail(String message)
   at System.Diagnostics.Debug.Assert(Boolean condition)
   at Microsoft.CodeAnalysis.Compilation.GetTypeByMetadataName(String fullyQualifiedMetadataName) in D:\GitHub\roslyn\src\Compilers\Core\Portable\Compilation\Compilation.cs:line 1172
   at Microsoft.CodeAnalysis.Shared.Extensions.ICompilationExtensions.ComAliasNameAttributeType(Compilation compilation) in D:\GitHub\roslyn\src\Workspaces\SharedUtilitiesAndExtensions\Compiler\Core\Extensions\ICompilationExtensions.cs:line 141
   at Microsoft.CodeAnalysis.ImplementInterface.AbstractImplementInterfaceService.ImplementInterfaceCodeAction.AttributesToRemove(Compilation compilation) in D:\GitHub\roslyn\src\Features\Core\Portable\ImplementInterface\AbstractImplementInterfaceService.CodeAction_Property.cs:line 70
   at Microsoft.CodeAnalysis.ImplementInterface.AbstractImplementInterfaceService.ImplementInterfaceCodeAction.GenerateMethod(Compilation compilation, IMethodSymbol method, Accessibility accessibility, DeclarationModifiers modifiers, Boolean generateAbstractly, Boolean useExplicitInterfaceSymbol, String memberName) in D:\GitHub\roslyn\src\Features\Core\Portable\ImplementInterface\AbstractImplementInterfaceService.CodeAction_Method.cs:line 30
   at Microsoft.CodeAnalysis.ImplementInterface.AbstractImplementInterfaceService.ImplementInterfaceCodeAction.GenerateMember(Compilation compilation, ISymbol member, String memberName, Boolean generateInvisibly, Boolean generateAbstractly, Boolean addNew, Boolean addUnsafe, ImplementTypePropertyGenerationBehavior propertyGenerationBehavior) in D:\GitHub\roslyn\src\Features\Core\Portable\ImplementInterface\AbstractImplementInterfaceService.CodeAction.cs:line 412
   at Microsoft.CodeAnalysis.ImplementInterface.AbstractImplementInterfaceService.ImplementInterfaceCodeAction.GenerateMember(Compilation compilation, ISymbol member, ArrayBuilder`1 implementedVisibleMembers, ImplementTypePropertyGenerationBehavior propertyGenerationBehavior) in D:\GitHub\roslyn\src\Features\Core\Portable\ImplementInterface\AbstractImplementInterfaceService.CodeAction.cs:line 330
   at Microsoft.CodeAnalysis.ImplementInterface.AbstractImplementInterfaceService.ImplementInterfaceCodeAction.GenerateMembers(Compilation compilation, ImmutableArray`1 unimplementedMembers, ImplementTypePropertyGenerationBehavior propertyGenerationBehavior) in D:\GitHub\roslyn\src\Features\Core\Portable\ImplementInterface\AbstractImplementInterfaceService.CodeAction.cs:line

Example #2:

Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.ImplementInterface.ImplementInterfaceTests.TestCharLiterals [FAIL]
  System.InvalidOperationException : Assertion failed
  Stack Trace:
    D:\GitHub\roslyn\src\Compilers\Test\Core\ThrowingTraceListener.cs(26,0): at Microsoft.CodeAnalysis.ThrowingTraceListener.Fail(String message, String detailMessage)
       at System.Diagnostics.TraceListener.Fail(String message)
       at System.Diagnostics.TraceInternal.Fail(String message)
       at System.Diagnostics.Debug.Assert(Boolean condition)
    D:\GitHub\roslyn\src\Compilers\Core\Portable\Compilation\Compilation.cs(1172,0): at Microsoft.CodeAnalysis.Compilation.GetTypeByMetadataName(String fullyQualifiedMetadataName)
    D:\GitHub\roslyn\src\Workspaces\SharedUtilitiesAndExtensions\Compiler\Core\Extensions\ICompilationExtensions.cs(153,0): at Microsoft.CodeAnalysis.Shared.Extensions.ICompilationExtensions.DynamicAttributeType(Compilation compilation)
    D:\GitHub\roslyn\src\Features\Core\Portable\ImplementInterface\AbstractImplementInterfaceService.CodeAction_Property.cs(70,0): at Microsoft.CodeAnalysis.ImplementInterface.AbstractImplementInterfaceService.ImplementInterfaceCodeAction.AttributesToRemove(Compilation compilation)
    D:\GitHub\roslyn\src\Features\Core\Portable\ImplementInterface\AbstractImplementInterfaceService.CodeAction_Method.cs(30,0): at Microsoft.CodeAnalysis.ImplementInterface.AbstractImplementInterfaceService.ImplementInterfaceCodeAction.GenerateMethod(Compilation compilation, IMethodSymbol method, Accessibility accessibility, DeclarationModifiers modifiers, Boolean generateAbstractly, Boolean useExplicitInterfaceSymbol, String memberName)
    D:\GitHub\roslyn\src\Features\Core\Portable\ImplementInterface\AbstractImplementInterfaceService.CodeAction.cs(412,0): at Microsoft.CodeAnalysis.ImplementInterface.AbstractImplementInterfaceService.ImplementInterfaceCodeAction.GenerateMember(Compilation compilation, ISymbol member, String memberName, Boolean generateInvisibly, Boolean generateAbstractly, Boolean addNew, Boolean addUnsafe, ImplementTypePropertyGenerationBehavior propertyGenerationBehavior)
    D:\GitHub\roslyn\src\Features\Core\Portable\ImplementInterface\AbstractImplementInterfaceService.CodeAction.cs(330,0): at Microsoft.CodeAnalysis.ImplementInterface.AbstractImplementInterfaceService.ImplementInterfaceCodeAction.GenerateMember(Compilation compilation, ISymbol member, ArrayBuilder`1 implementedVisibleMembers, ImplementTypePropertyGenerationBehavior propertyGenerationBehavior)
    D:\GitHub\roslyn\src\Features\Core\Portable\ImplementInterface\AbstractImplementInterfaceService.CodeAction.cs(247,0): at Microsoft.CodeAnalysis.ImplementInterface.AbstractImplementInterfaceService.ImplementInterfaceCodeAction.GenerateMembers(Compilation compilation, ImmutableArray`1 unimplementedMembers, ImplementTypePropertyGenerationBehavior propertyGenerationBehavior)
    D:\GitHub\roslyn\src\Features\Core\Portable\ImplementInterface\AbstractImplementInterfaceService.CodeAction.cs(201,0): at Microsoft.CodeAnalysis.ImplementInterface.AbstractImplementInterfaceService.ImplementInterfaceCodeAction.<GetUpdatedDocumentAsync>d__23.MoveNext()

It looks like the assert was added in #58366.
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged Issues and PRs which have not yet been triaged by a lead label Feb 8, 2022
@AlekseyTs
Copy link
Contributor Author

CC @333fred

@AlekseyTs
Copy link
Contributor Author

Here is a CI job failure - https://dev.azure.com/dnceng/public/_build/results?buildId=1610344&view=ms.vss-test-web.build-test-results-tab&runId=44862088&resultId=153057&paneView=debug

@jaredpar jaredpar added Bug and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Feb 16, 2022
@333fred 333fred mentioned this issue Feb 18, 2022
Closed
333fred added a commit to 333fred/roslyn that referenced this issue Feb 18, 2022
@333fred
Account for possible cache invalidation on a different thread in assert.
4570f87 
Attempt at fixing dotnet#59395. Reviewing the codepath I don't see any other obvious points that could cause the assert to fail, so if it continues to reproduce after this change we can look at further logging to determine the root cause.
@333fred 333fred mentioned this issue Mar 1, 2022
Merged
333fred added a commit that referenced this issue Mar 2, 2022
@333fred
Account for possible cache invalidation on a different thread in asse…
82e8506 
…rt. (#59842)

Attempt at fixing #59395. Reviewing the codepath I don't see any other obvious points that could cause the assert to fail, so if it continues to reproduce after this change we can look at further logging to determine the root cause.
@jcouv jcouv added this to the 17.2 milestone Mar 17, 2022
@jinujoseph jinujoseph modified the milestones: 17.2, 17.3 May 5, 2022
@333fred
Copy link
Member

333fred commented Jun 1, 2022

We haven't seen another instance of this since my fix to account for multi-threading in cache invalidation, so I'm going to call this fixed.

@333fred 333fred closed this as completed Jun 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@333fred 333fred

Labels
Area-Compilers Bug
Projects
None yet
Milestone
17.3
Development

No branches or pull requests
5 participants
@jaredpar @333fred @AlekseyTs @jinujoseph @jcouv