Switch MemoryStreamDoubleDispose event level from Critical to Verbose#406
Switch MemoryStreamDoubleDispose event level from Critical to Verbose#406inklesspen1rus wants to merge 3 commits intomicrosoft:masterfrom
Conversation
|
@microsoft-github-policy-service agree |
|
I'm convinced. I just checked and internally, we don't even track this metric. Verbose makes sense. However, I think you should make one more modification. See code review comment. |
benmwatson
left a comment
There was a problem hiding this comment.
The Dispose(bool) method call is grabbing a stack trace, and this will only make sense now if Verbose logging is enabled. Can you modify the code like this in Dispose?
if (this.memoryManager.options.GenerateCallStacks && Events.Log.IsEnabled(EventLevel.Verbose))
...|
AllocationStack is used for StreamDoubleDisposed event so we need to check this event too, but we can check the event only in RecyclableMemoryStreamManager. So I dedicated internal property to check if we need generate stacktrace. Ideally I'd move stacktrace generation into ReportStreamDoubleDisposed but that changes behaviour (adds one more frame into stacktrace). |
Multiple disposes are common thing in .NET. MSDN says that too:
Critical events may spuriously fill up event log with critical events while operations like that aren't even logical error.
So I switched it's event level to
Verbose. I don't think there are reasons to use Information for events like that, when AspNetCore disposes streams twice.Closes #394