-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Fix deadlock in NativeRuntimeEventSourceTest #86370
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.
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change looks fine. Given that this looks like a workaround, what meaning should I ascribe to the test failure? Is it a bug in the test, a bug in the product that we are choosing not to fix, a bug in the product that will be fixed later?
Build failed:
|
It's a bug in the product, but I don't think customers are likely to hit it. There is a potential deadlock between shutting down EventPipeEventDispatcher and an EventListener if you create EventSources in your OnEventWritten callback. I don't think this is a common scenario, we are particularly vulnerable to it in the tests because they are short lived and only write an event or two, and touching framework code can cause EventSources like ArrayPoolEventSource to be lazy initted. To my knowledge we haven't had customer reports, because their EventListeners are generally longer lived. |
I'd suggest you add a comment to the test code saying that we believe a product issue exists here, but for now we are choosing not to fix it because we believe it doesn't impact any customers. Thanks David! |
Fixes #86233