From 0a32a71fd5a8add95d1f265f161a22312c66109d Mon Sep 17 00:00:00 2001 From: Pavel Krymets Date: Wed, 25 Jul 2018 15:34:15 -0700 Subject: [PATCH 1/2] Do not expand stacktraces when completion exception is rethrown multiple times --- .../src/System/IO/Pipelines/PipeCompletion.cs | 23 ++++++++++--------- .../tests/PipeReaderWriterFacts.cs | 5 ++++ 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/System.IO.Pipelines/src/System/IO/Pipelines/PipeCompletion.cs b/src/System.IO.Pipelines/src/System/IO/Pipelines/PipeCompletion.cs index b9b582c6589c..88314c8c1dc0 100644 --- a/src/System.IO.Pipelines/src/System/IO/Pipelines/PipeCompletion.cs +++ b/src/System.IO.Pipelines/src/System/IO/Pipelines/PipeCompletion.cs @@ -13,25 +13,25 @@ namespace System.IO.Pipelines internal struct PipeCompletion { private static readonly ArrayPool s_completionCallbackPool = ArrayPool.Shared; - private static readonly Exception s_completedNoException = new Exception(); private const int InitialCallbacksSize = 1; - private Exception _exception; + private bool _isCompleted; + private ExceptionDispatchInfo _exception; private PipeCompletionCallback[] _callbacks; private int _callbackCount; - public bool IsCompleted => _exception != null; + public bool IsCompleted => _isCompleted; - public bool IsFaulted => IsCompleted && _exception != s_completedNoException; + public bool IsFaulted => IsCompleted && _exception != null; public PipeCompletionCallbacks TryComplete(Exception exception = null) { - if (_exception == null) + _isCompleted = true; + if (exception != null) { - // Set the exception object to the exception passed in or a sentinel value - _exception = exception ?? s_completedNoException; + _exception = ExceptionDispatchInfo.Capture(exception); } return GetCallbacks(); } @@ -68,12 +68,12 @@ public PipeCompletionCallbacks AddCallback(Action callback, o [MethodImpl(MethodImplOptions.AggressiveInlining)] public bool IsCompletedOrThrow() { - if (_exception == null) + if (!_isCompleted) { return false; } - if (_exception != s_completedNoException) + if (_exception != null) { ThrowLatchedException(); } @@ -91,7 +91,7 @@ private PipeCompletionCallbacks GetCallbacks() var callbacks = new PipeCompletionCallbacks(s_completionCallbackPool, _callbackCount, - _exception == s_completedNoException ? null : _exception, + _exception?.SourceException, _callbacks); _callbacks = null; @@ -103,13 +103,14 @@ public void Reset() { Debug.Assert(IsCompleted); Debug.Assert(_callbacks == null); + _isCompleted = false; _exception = null; } [MethodImpl(MethodImplOptions.NoInlining)] private void ThrowLatchedException() { - ExceptionDispatchInfo.Capture(_exception).Throw(); + _exception.Throw(); } public override string ToString() diff --git a/src/System.IO.Pipelines/tests/PipeReaderWriterFacts.cs b/src/System.IO.Pipelines/tests/PipeReaderWriterFacts.cs index f25f08b2a22c..6f1117909b8d 100644 --- a/src/System.IO.Pipelines/tests/PipeReaderWriterFacts.cs +++ b/src/System.IO.Pipelines/tests/PipeReaderWriterFacts.cs @@ -9,6 +9,7 @@ using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Text; +using System.Text.RegularExpressions; using System.Threading; using System.Threading.Tasks; using Xunit; @@ -238,6 +239,8 @@ void ThrowTestException() invalidOperationException = await Assert.ThrowsAsync(async () => await _pipe.Writer.FlushAsync()); Assert.Equal("Reader exception", invalidOperationException.Message); Assert.Contains("ThrowTestException", invalidOperationException.StackTrace); + + Assert.Single(Regex.Matches(invalidOperationException.StackTrace, "Pipe.GetFlushResult")); } [Fact] @@ -304,6 +307,8 @@ void ThrowTestException() invalidOperationException = await Assert.ThrowsAsync(async () => await _pipe.Reader.ReadAsync()); Assert.Equal("Writer exception", invalidOperationException.Message); Assert.Contains("ThrowTestException", invalidOperationException.StackTrace); + + Assert.Single(Regex.Matches(invalidOperationException.StackTrace, "Pipe.GetReadResult")); } [Fact] From 1c0a6f3b7bb3f0b38bbdc5572706bfe0296409aa Mon Sep 17 00:00:00 2001 From: Pavel Krymets Date: Wed, 25 Jul 2018 16:58:42 -0700 Subject: [PATCH 2/2] fb --- .../src/System/IO/Pipelines/PipeCompletion.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/System.IO.Pipelines/src/System/IO/Pipelines/PipeCompletion.cs b/src/System.IO.Pipelines/src/System/IO/Pipelines/PipeCompletion.cs index 88314c8c1dc0..db7939f79f47 100644 --- a/src/System.IO.Pipelines/src/System/IO/Pipelines/PipeCompletion.cs +++ b/src/System.IO.Pipelines/src/System/IO/Pipelines/PipeCompletion.cs @@ -17,21 +17,21 @@ internal struct PipeCompletion private const int InitialCallbacksSize = 1; private bool _isCompleted; - private ExceptionDispatchInfo _exception; + private ExceptionDispatchInfo _exceptionInfo; private PipeCompletionCallback[] _callbacks; private int _callbackCount; public bool IsCompleted => _isCompleted; - public bool IsFaulted => IsCompleted && _exception != null; + public bool IsFaulted => _exceptionInfo != null; public PipeCompletionCallbacks TryComplete(Exception exception = null) { _isCompleted = true; if (exception != null) { - _exception = ExceptionDispatchInfo.Capture(exception); + _exceptionInfo = ExceptionDispatchInfo.Capture(exception); } return GetCallbacks(); } @@ -73,7 +73,7 @@ public bool IsCompletedOrThrow() return false; } - if (_exception != null) + if (_exceptionInfo != null) { ThrowLatchedException(); } @@ -91,7 +91,7 @@ private PipeCompletionCallbacks GetCallbacks() var callbacks = new PipeCompletionCallbacks(s_completionCallbackPool, _callbackCount, - _exception?.SourceException, + _exceptionInfo?.SourceException, _callbacks); _callbacks = null; @@ -104,13 +104,13 @@ public void Reset() Debug.Assert(IsCompleted); Debug.Assert(_callbacks == null); _isCompleted = false; - _exception = null; + _exceptionInfo = null; } [MethodImpl(MethodImplOptions.NoInlining)] private void ThrowLatchedException() { - _exception.Throw(); + _exceptionInfo.Throw(); } public override string ToString()