Skip to content

Use weak handle for the back reference to EventProvider - #80482

Merged
jkotas merged 5 commits into
dotnet:mainfrom
jkotas:issue-80450
Jan 13, 2023
Merged

Use weak handle for the back reference to EventProvider#80482
jkotas merged 5 commits into
dotnet:mainfrom
jkotas:issue-80450

Conversation

@jkotas

@jkotasjkotas commented Jan 11, 2023•
edited
Loading

Copy link
Copy Markdown
Member

  • Use weak handle for the back reference to EventProvider. This avoids leak when the event provider is orphaned without explicit unregistration.

  • Delete unnecessary IEventProvider interface. Repurpose no-op event provider as a base class

Fixes#80450

@jkotas
Use weak handle for the back reference to EventProvider
f204ad2
This avoids leak when the event provider is orphaned without explicit unregistration.
Fixesdotnet#80450
@jkotas
Delete unnecessary IEventProvider interface
c29b8c5
Repurpose no-op event provider as a base class
@ghostghost assigned jkotasJan 11, 2023
@jkotas

jkotas commented Jan 11, 2023

Copy link
Copy Markdown
MemberAuthor

@janvorli Is it possible to trigger the unloadability test run from CI?

@build-analysisbuild-analysisBot mentioned this pull request Jan 11, 2023
Closed
@janvorli

janvorli commented Jan 11, 2023

Copy link
Copy Markdown
Member

@jkotas I am not sure if that's possible. But we have weekly runs that execute those (https://dev.azure.com/dnceng-public/public/_build?definitionId=121)

@jkotas

jkotas commented Jan 13, 2023

Copy link
Copy Markdown
MemberAuthor

I have manually triggered a run - the tests that you have flagged are passing.

noahfalk
noahfalk approved these changes Jan 13, 2023
View reviewed changes

@noahfalknoahfalk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks Jan!

@davmason - you might want to take a look as well.

janvorli
janvorli approved these changes Jan 13, 2023
View reviewed changes

@janvorlijanvorli left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thank you!

@runfoapprunfoappBot mentioned this pull request Jan 13, 2023
Closed
@jkotas
jkotas merged commit 32cb6bb into dotnet:mainJan 13, 2023
@jkotas
jkotas deleted the issue-80450 branch January 14, 2023 06:31
@ghostghost locked as resolved and limited conversation to collaborators Feb 13, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@stephentoubstephentoubstephentoub left review comments
@noahfalknoahfalknoahfalk approved these changes
@janvorlijanvorlijanvorli approved these changes

Assignees

@jkotasjkotas

Labels

area-Tracing-coreclr

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

EventListener unloadability

4 participants

@jkotas@janvorli@stephentoub@noahfalk