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

EE: Handle notification when metadata has been invalidated #75423

Merged
merged 14 commits into from
Oct 15, 2024
Merged

EE: Handle notification when metadata has been invalidated #75423

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
7fe1a1f
EE: Get notification before GetMetaDataBytesPtr is invalidated
cston Oct 7, 2024
1280480
Remove unused exception codes; add one
cston Oct 7, 2024
cbdaac5
Use 17.13 packages
cston Oct 7, 2024
00d24fb
Update VSDConfigTool version
cston Oct 8, 2024
a5ccef7
Update VSDConfigTool version
cston Oct 8, 2024
5b621fe
Skip tests - see #75456
cston Oct 9, 2024
8cfc30a
Re-enable some tests
cston Oct 10, 2024
cd4420b
Skip BasicExpressionEvaluator tests
cston Oct 10, 2024
3e17398
Skip {CSharp,Basic}Immediate tests
cston Oct 10, 2024
5b5c136
Skip {CSharp,Basic}Immediate test classes
cston Oct 10, 2024
5fdf51d
Skip {CSharp,Basic}Immediate test bodies
cston Oct 10, 2024
bb573d0
Merge remote-tracking branch 'upstream/main' into GetMetaDataBytesPtr
cston Oct 15, 2024
c6a02f7
Skip checking EE result from {CSharp,Basic}Immediate
cston Oct 15, 2024
e2cf933
Add Skip link to BasicEditAndContinue tests
cston Oct 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions eng/Directory.Packages.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
<PackageVersion Include="Microsoft.VisualStudio.Extensibility" Version="$(MicrosoftVisualStudioExtensibilityVersion)" />
<PackageVersion Include="Microsoft.VisualStudio.Extensibility.Sdk" Version="$(MicrosoftVisualStudioExtensibilityVersion)" />
<PackageVersion Include="Microsoft.VisualStudio.Extensibility.JsonGenerators.Sdk" Version="$(MicrosoftVisualStudioExtensibilityVersion)" />
<PackageVersion Include="Microsoft.VSSDK.Debugger.VSDConfigTool" Version="17.0.1051901-preview" />
<PackageVersion Include="Microsoft.VSSDK.Debugger.VSDConfigTool" Version="17.13.1100801-preview" />
<PackageVersion Include="Microsoft.VisualStudio.ProjectSystem" Version="17.0.77-pre-g62a6cb5699" />
<PackageVersion Include="Microsoft.VisualStudio.Progression.CodeSchema" Version="15.8.27812-alpha" />
<PackageVersion Include="Microsoft.VisualStudio.Progression.Common" Version="15.8.27812-alpha" />
Expand Down Expand Up @@ -122,8 +122,8 @@
VS Debugger
-->
<PackageVersion Include="Microsoft.VisualStudio.Debugger.Contracts" Version="17.12.0-beta.24403.1" />
<PackageVersion Include="Microsoft.VisualStudio.Debugger.Engine-implementation" Version="17.8.1072001-preview" />
<PackageVersion Include="Microsoft.VisualStudio.Debugger.Metadata-implementation" Version="17.8.1072001-preview" />
<PackageVersion Include="Microsoft.VisualStudio.Debugger.Engine-implementation" Version="17.13.1100701-preview" />
<PackageVersion Include="Microsoft.VisualStudio.Debugger.Metadata-implementation" Version="17.13.1100701-preview" />
Copy link
Member Author

@cston cston Oct 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to reference a newer version of Microsoft.VSSDK.Debugger.VSDConfigTool (at line 67) as well, so that VSDConfigTool.exe recognizes the added interface? #Closed


<!--
VS .NET Runtime
Expand Down
1 change: 1 addition & 0 deletions ...xpressionEvaluator/CSharp/Source/ExpressionCompiler/CSharpExpressionCompiler.vsdconfigxml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
<InterfaceGroup
CallOnlyWhenLoaded="true">
<NoFilter/>
<Interface Name="IDkmMetaDataPointerInvalidatedNotification"/>
<Interface Name="IDkmModuleModifiedNotification"/>
<Interface Name="IDkmModuleInstanceUnloadNotification"/>
</InterfaceGroup>
Expand Down
7 changes: 6 additions & 1 deletion src/ExpressionEvaluator/Core/Source/ExpressionCompiler/ExpressionCompiler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
using System.Diagnostics;
using System.IO;
using System.Threading;
using Microsoft.CodeAnalysis.Collections;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.VisualStudio.Debugger;
using Microsoft.VisualStudio.Debugger.CallStack;
Expand All @@ -23,6 +22,7 @@ namespace Microsoft.CodeAnalysis.ExpressionEvaluator
public abstract class ExpressionCompiler :
IDkmClrExpressionCompiler,
IDkmClrExpressionCompilerCallback,
IDkmMetaDataPointerInvalidatedNotification,
IDkmModuleModifiedNotification,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IDkmModuleModifiedNotification

You shouldn't need to implement this anymore (unless this is just a temporary thing to keep tests happy until you are running against a new debugger)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'd prefer keeping the implementation of IDkmModuleModifiedNotification, for robustness, assuming it doesn't conflict with IDkmMetaDataPointerInvalidatedNotification.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They will not conflict - just a bit of a perf overhead since now Rosyln will be notified twice anytime, we modify an assembly.

IDkmModuleInstanceUnloadNotification,
IDkmLanguageFrameDecoder,
Expand Down Expand Up @@ -247,6 +247,11 @@ internal static bool ShouldTryAgainWithMoreMetadataBlocks(DkmUtilities.GetMetada
return false;
}

void IDkmMetaDataPointerInvalidatedNotification.OnMetaDataPointerInvalidated(DkmClrModuleInstance moduleInstance)
{
RemoveDataItemIfNecessary(moduleInstance);
}

void IDkmModuleModifiedNotification.OnModuleModified(DkmModuleInstance moduleInstance)
{
RemoveDataItemIfNecessary(moduleInstance);
Expand Down
1 change: 1 addition & 0 deletions src/ExpressionEvaluator/Core/Source/ExpressionCompiler/ExpressionEvaluatorFatalError.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ internal static bool CrashIfFailFastEnabled(Exception exception)
{
switch (dkmException.Code)
{
case DkmExceptionCode.E_METADATA_UPDATE_DEADLOCK: // Metadata was updated while EE had component lock
case DkmExceptionCode.E_PROCESS_DESTROYED:
case DkmExceptionCode.E_XAPI_REMOTE_CLOSED:
case DkmExceptionCode.E_XAPI_REMOTE_DISCONNECTED:
Expand Down
1 change: 1 addition & 0 deletions src/ExpressionEvaluator/Core/Source/FunctionResolver/CSharp/FunctionResolver.vsdconfigxml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
<RuntimeId RequiredValue="DkmRuntimeId.Clr"/>
<RuntimeId RequiredValue="DkmRuntimeId.ClrNativeCompilation"/>
</Filter>
<Interface Name="IDkmMetaDataPointerInvalidatedNotification"/>
<Interface Name="IDkmModuleInstanceLoadNotification"/>
<Interface Name="IDkmModuleInstanceUnloadNotification"/>
<Interface Name="IDkmModuleModifiedNotification"/>
Expand Down
8 changes: 8 additions & 0 deletions src/ExpressionEvaluator/Core/Source/FunctionResolver/FunctionResolver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ namespace Microsoft.CodeAnalysis.ExpressionEvaluator
internal abstract class FunctionResolver :
FunctionResolverBase<DkmProcess, DkmClrModuleInstance, DkmRuntimeFunctionResolutionRequest>,
IDkmRuntimeFunctionResolver,
IDkmMetaDataPointerInvalidatedNotification,
IDkmModuleInstanceLoadNotification,
IDkmModuleInstanceUnloadNotification,
IDkmModuleModifiedNotification,
Expand Down Expand Up @@ -54,6 +55,13 @@ void IDkmModuleModifiedNotification.OnModuleModified(DkmModuleInstance moduleIns
// caller from modifying modules while binding.
}

void IDkmMetaDataPointerInvalidatedNotification.OnMetaDataPointerInvalidated(DkmClrModuleInstance moduleInstance)
{
// Implementing IDkmMetaDataPointerInvalidatedNotification
// (with Synchronized="true" in .vsdconfigxml) prevents
// caller from modifying modules while binding.
}

void IDkmModuleSymbolsLoadedNotification.OnModuleSymbolsLoaded(DkmModuleInstance moduleInstance, DkmModule module, bool isReload, DkmWorkList workList, DkmEventDescriptor eventDescriptor)
{
OnModuleLoad(moduleInstance, workList);
Expand Down
1 change: 1 addition & 0 deletions ...xpressionEvaluator/Core/Source/FunctionResolver/VisualBasic/FunctionResolver.vsdconfigxml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
<RuntimeId RequiredValue="DkmRuntimeId.Clr"/>
<RuntimeId RequiredValue="DkmRuntimeId.ClrNativeCompilation"/>
</Filter>
<Interface Name="IDkmMetaDataPointerInvalidatedNotification"/>
<Interface Name="IDkmModuleInstanceLoadNotification"/>
<Interface Name="IDkmModuleInstanceUnloadNotification"/>
<Interface Name="IDkmModuleModifiedNotification"/>
Expand Down
Loading
Loading