Skip to content

Consider MVID when testing EC cache validity - #1250

Merged
amcasey merged 1 commit into
dotnet:masterfrom
amcasey:DD1138139
Mar 13, 2015
Merged

amcasey merged 1 commit into
dotnet:masterfrom
amcasey:DD1138139

Conversation

@amcasey

@amcasey amcasey commented Mar 13, 2015

Copy link
Copy Markdown
Member

Our EvaluationContext cache invalidation computation was consuming the
method token and version, but not the identity of the declaring assembly.
This lead to a strange bug where we would reuse the cache across assembly
boundaries if consecutive breakpoint happened to have the same method
token (each in its own assembly).

I believe this bug existed before I revised the cache invalidation
computation and we were just getting lucky - we used to check the spans
of all containing scopes, which are unlikely to match exactly across
assemblies.

@amcasey

amcasey commented Mar 13, 2015

Copy link
Copy Markdown
Member Author

FYI @KevinH-MS @ManishJayaswal

@amcasey

amcasey commented Mar 13, 2015

Copy link
Copy Markdown
Member Author

@pnelsonmsft I got a good laugh out of this one. :)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Perhaps test with Guid.NewGuid() rather than default(Guid).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done.

@cston

cston commented Mar 13, 2015

Copy link
Copy Markdown
Contributor

LGTM

Our EvaluationContext cache invalidation computation was consuming the
method token and version, but not the identity of the declaring assembly.
This lead to a strange bug where we would reuse the cache across assembly
boundaries if consecutive breakpoint happened to have the same method
token (each in its own assembly).

I believe this bug existed before I revised the cache invalidation
computation and we were just getting lucky - we used to check the spans
of all containing scopes, which are unlikely to match exactly across
assemblies.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants