Skip to content

Conversation

@sywhang
Copy link
Contributor

@sywhang sywhang commented May 23, 2020

Another attempt at fixing #643 since the old PR is quite outdated and have merge conflicts to deal with.

This PR fixes AddTypeToGlobalCacheIfNotExists by pulling out the constructors of LoggedTypesFromModule and TypeLoggingInfo out of the scope of EtwTypeLogHash Crst.

Copy link
Member

@noahfalk noahfalk left a comment

Choose a reason for hiding this comment

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

One question about the ETW keyword check, otherwise LGTM!

// Check if ETW is enabled, and if not, bail here.
// We do this inside of the lock to ensure that we don't immediately
// re-allocate the global type hash after it has been cleaned up.
if (!ETW_TRACING_CATEGORY_ENABLED(
Copy link
Member

Choose a reason for hiding this comment

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

I think it is safe to leave this inside the lock where it was. Did you observe a problem that was causing?

@janvorli
Copy link
Member

janvorli commented Jun 2, 2020

I've verified that this PR fixes the problem for me.

@sywhang sywhang merged commit 8deadd2 into dotnet:master Jun 3, 2020
@davmason davmason mentioned this pull request Jul 7, 2020
davmason added a commit that referenced this pull request Jul 9, 2020
#36932 had a typo that caused types to never be logged
@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants