Deadlock in EventPipe when reader tries to take EventPipe config lock while tracing self

[josalem]

As detailed in #1794, when an application is tracing itself there is the potential for a deadlock to occur if the reader tries to take the EventPipe configuration lock while EventPipe::Disable (specifically EventPipe::RunWithCallbackPostponed) is in progress.

This occurs when EventPipeEventSource is reading because the Trace Event library uses Regex in its logic and Regex now has a lazily initialized ConcurrentDictionary cache that generates events when used. This causes the static EventSource provider for concurrent collections to get created (unless its been instantiated earlier in the process) which will call into native code to create the provider. This requires the config lock in EventPipe. If this occurs during EventPipe::RunWithCallbackPostponed, you may deadlock because the reader is waiting on the lock, but the writer is holding it and is blocked because the pipe's buffer is full.

See the stacks in #1794 for details.

A couple potential paths to fixing this:

• break up the logic of EventPipe::Disable so that it isn't holding the config lock for the entire duration. Rundown and other parts of disable can run for minutes in big applications, so this shouldn't be holding a global lock like that the entire time.
• find ways to defer the registration of new providers that get registered while EventPipe::Disable is in flight.

This issue breaks our EventPipe tests, but #1794 added a bypass that needs to be removed once this issue is closed.

CC - @tommcdon @noahfalk @sywhang

[tommcdon]

@davmason @josalem is there any outstanding 5.0 work for this issue?

[josalem]

The bandaid fix applied in #1794 has kept this from popping up on my radar, so I believe this underlying issue is still present. We could still go and make the changes I alluded to in my comment on the PR in EventPipeEventSource to fix the only provided, live reader that I am aware of (note that this would be out of band work in Microsoft/PerfView). As for fixing the underlying issue, I think we need to find a way to do rundown without holding the EventPipeConfig lock, or at least adding a deferral mechanism that does provider creation/removal after a rundown has concluded.

[tommcdon]

@josalem Thanks! To clarify, is this a test issue only and not a product bug?

[davmason]

This is a product issue that we added a workaround for in our tests