-
Notifications
You must be signed in to change notification settings - Fork 4.5k
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
Test failure: tracing/eventpipe/eventsourceerror/eventsourceerror/eventsourceerror.sh #35884
Comments
Tagging subscribers to this area: @Maoni0 |
Tagging subscribers to this area: @tommcdon |
I'll take a look! |
CC @davmason It looks like this test has been failing under gcstress since it went in with #35334. It fails across all configurations except Linux arm32 gcstress=0x3 (at least on the linked run). I'm not sure why it's happening, but my hunch is that is has to do with the lifetime of the delegate in the failure path for the EventSource constructor now that |
I'll take a look |
So, this turned in to way more of an involved investigation than I thought. This turned out to not be a test issue but exposed a fundamental problem with the way EventSources interact with EventPipe. An EventSource will call dispose on its underlying providers when it is finalized or Dispose is called, and in turn the EventPipeProvider will call EventUnregister which calls EventPipeInternal::DeleteProvider. Up until this point everything is ok, but inside of EventPipe::DeleteProvider there is a check for runtime/src/coreclr/src/vm/eventpipe.cpp Lines 616 to 627 in c2b1018
Which means if any other provider is active when the EventSource tries to delete itself from EventPipe, it will get placed in the deferred list. For example, in this test I suspect this has gone undetected for so long because we don't have a ton of testing around EventSources and how they interact with EventPipe, and likely the tests that do test the scenario just happen to be lucky and the delegate doesn't get collected in the timeframe. Even this test passes fine under normal circumstances and it takes GCStress to fail. @noahfalk @josalem @sywhang - I don't fully understand the need to keep providers around until tracing stops. I see it was added in this PR: dotnet/coreclr#11651, but I don't see anything that even attempts to root the EventSource/EventPipeProvider managed objects. So we either need to root those objects, or if the native provider is all we need then we should not attempt to do the reverse pinvoke during deferred deletion. |
This is still failing in the GCStress runs. I submitted PR #38516 to disable it for GCStress while this bug is open. |
failed in job: runtime-coreclr gcstress0x3-gcstress0xc 20200503.1
Error message
The text was updated successfully, but these errors were encountered: