From 9064eed7e0e3416b65ec6fb9e8b172ef1bea919a Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Wed, 31 Mar 2021 17:08:42 -0700 Subject: [PATCH] Add ExceptionDispatchInfo.SetRemoteStackTrace (#50392) --- .../src/System/Exception.CoreCLR.cs | 17 ++++------- .../System.Private.CoreLib/src/System/Exception.cs | 30 ++++++++++++++++++++ .../ExceptionServices/ExceptionDispatchInfo.cs | 33 +++++++++++++++++++++- src/libraries/System.Runtime/ref/System.Runtime.cs | 1 + .../ExceptionDispatchInfoTests.cs | 29 ++++++++++++++++--- .../src/System/Exception.Mono.cs | 18 ++++-------- 6 files changed, 100 insertions(+), 28 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Exception.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Exception.CoreCLR.cs index d6bb435..81b9be4 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Exception.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Exception.CoreCLR.cs @@ -7,7 +7,6 @@ using System.Reflection; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Runtime.Serialization; -using System.Text; namespace System { @@ -323,14 +322,16 @@ namespace System _remoteStackTraceString, _ipForWatsonBuckets, _watsonBuckets); } - [StackTraceHidden] - internal void SetCurrentStackTrace() + // Returns true if setting the _remoteStackTraceString field is legal, false if not (immutable exception). + // A false return value means the caller should early-exit the operation. + // Can also throw InvalidOperationException if a stack trace is already set or if object has been thrown. + private bool CanSetRemoteStackTrace() { // If this is a preallocated singleton exception, silently skip the operation, // regardless of the value of throwIfHasExistingStack. if (IsImmutableAgileException(this)) { - return; + return false; } // Check to see if the exception already has a stack set in it. @@ -339,13 +340,7 @@ namespace System ThrowHelper.ThrowInvalidOperationException(); } - // Store the current stack trace into the "remote" stack trace, which was originally introduced to support - // remoting of exceptions cross app-domain boundaries, and is thus concatenated into Exception.StackTrace - // when it's retrieved. - var sb = new StringBuilder(256); - new StackTrace(fNeedFileInfo: true).ToString(System.Diagnostics.StackTrace.TraceFormat.TrailingNewLine, sb); - sb.AppendLine(SR.Exception_EndStackTraceFromPreviousThrow); - _remoteStackTraceString = sb.ToString(); + return true; } } } diff --git a/src/libraries/System.Private.CoreLib/src/System/Exception.cs b/src/libraries/System.Private.CoreLib/src/System/Exception.cs index 14e84a6..cef527e 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Exception.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Exception.cs @@ -4,6 +4,7 @@ using System.Collections; using System.Diagnostics; using System.Runtime.Serialization; +using System.Text; namespace System { @@ -197,5 +198,34 @@ namespace System public new Type GetType() => base.GetType(); partial void RestoreRemoteStackTrace(SerializationInfo info, StreamingContext context); + + [StackTraceHidden] + internal void SetCurrentStackTrace() + { + if (!CanSetRemoteStackTrace()) + { + return; // early-exit + } + + // Store the current stack trace into the "remote" stack trace, which was originally introduced to support + // remoting of exceptions cross app-domain boundaries, and is thus concatenated into Exception.StackTrace + // when it's retrieved. + var sb = new StringBuilder(256); + new StackTrace(fNeedFileInfo: true).ToString(System.Diagnostics.StackTrace.TraceFormat.TrailingNewLine, sb); + sb.AppendLine(SR.Exception_EndStackTraceFromPreviousThrow); + _remoteStackTraceString = sb.ToString(); + } + + internal void SetRemoteStackTrace(string stackTrace) + { + if (!CanSetRemoteStackTrace()) + { + return; // early-exit + } + + // Store the provided text into the "remote" stack trace, following the same format SetCurrentStackTrace + // would have generated. + _remoteStackTraceString = stackTrace + Environment.NewLineConst + SR.Exception_EndStackTraceFromPreviousThrow + Environment.NewLineConst; + } } } diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/ExceptionServices/ExceptionDispatchInfo.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/ExceptionServices/ExceptionDispatchInfo.cs index 18c7261..d89ef99 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/ExceptionServices/ExceptionDispatchInfo.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/ExceptionServices/ExceptionDispatchInfo.cs @@ -65,7 +65,7 @@ namespace System.Runtime.ExceptionServices /// Stores the current stack trace into the specified instance. /// The unthrown instance. /// The argument was null. - /// The argument was previously thrown or previously had a stack trace stored into it.. + /// The argument was previously thrown or previously had a stack trace stored into it. /// The exception instance. [StackTraceHidden] public static Exception SetCurrentStackTrace(Exception source) @@ -79,5 +79,36 @@ namespace System.Runtime.ExceptionServices return source; } + + /// + /// Stores the provided stack trace into the specified instance. + /// + /// The unthrown instance. + /// The stack trace string to persist within . This is normally acquired + /// from the property from the remote exception instance. + /// The or argument was null. + /// The argument was previously thrown or previously had a stack trace stored into it. + /// The exception instance. + /// + /// This method populates the property from an arbitrary string value. + /// The typical use case is the transmission of objects across processes with high fidelity, + /// allowing preservation of the exception object's stack trace information. .NET does not attempt to parse the + /// provided string value. The caller is responsible for normalizing line endings if required. + /// + public static Exception SetRemoteStackTrace(Exception source, string stackTrace) + { + if (source is null) + { + throw new ArgumentNullException(nameof(source)); + } + if (stackTrace is null) + { + throw new ArgumentNullException(nameof(stackTrace)); + } + + source.SetRemoteStackTrace(stackTrace); + + return source; + } } } diff --git a/src/libraries/System.Runtime/ref/System.Runtime.cs b/src/libraries/System.Runtime/ref/System.Runtime.cs index 97f2121..0bfd76c 100644 --- a/src/libraries/System.Runtime/ref/System.Runtime.cs +++ b/src/libraries/System.Runtime/ref/System.Runtime.cs @@ -9782,6 +9782,7 @@ namespace System.Runtime.ExceptionServices public System.Exception SourceException { get { throw null; } } public static System.Runtime.ExceptionServices.ExceptionDispatchInfo Capture(System.Exception source) { throw null; } public static System.Exception SetCurrentStackTrace(System.Exception source) { throw null; } + public static System.Exception SetRemoteStackTrace(System.Exception source, string stackTrace) { throw null; } [System.Diagnostics.CodeAnalysis.DoesNotReturnAttribute] public void Throw() => throw null; [System.Diagnostics.CodeAnalysis.DoesNotReturnAttribute] diff --git a/src/libraries/System.Runtime/tests/System/Runtime/ExceptionServices/ExceptionDispatchInfoTests.cs b/src/libraries/System.Runtime/tests/System/Runtime/ExceptionServices/ExceptionDispatchInfoTests.cs index c2bb2bd..e293a18 100644 --- a/src/libraries/System.Runtime/tests/System/Runtime/ExceptionServices/ExceptionDispatchInfoTests.cs +++ b/src/libraries/System.Runtime/tests/System/Runtime/ExceptionServices/ExceptionDispatchInfoTests.cs @@ -1,7 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.ComponentModel; +using System.Diagnostics; using System.Runtime.CompilerServices; using System.Text.RegularExpressions; using Xunit; @@ -29,23 +29,27 @@ namespace System.Runtime.ExceptionServices.Tests } [Fact] - public static void SetCurrentStackTrace_Invalid_Throws() + public static void SetCurrentOrRemoteStackTrace_Invalid_Throws() { Exception e; // Null argument e = null; AssertExtensions.Throws("source", () => ExceptionDispatchInfo.SetCurrentStackTrace(e)); + AssertExtensions.Throws("source", () => ExceptionDispatchInfo.SetRemoteStackTrace(e, "Hello")); + AssertExtensions.Throws("stackTrace", () => ExceptionDispatchInfo.SetRemoteStackTrace(new Exception(), stackTrace: null)); // Previously set current stack e = new Exception(); ExceptionDispatchInfo.SetCurrentStackTrace(e); Assert.Throws(() => ExceptionDispatchInfo.SetCurrentStackTrace(e)); + Assert.Throws(() => ExceptionDispatchInfo.SetRemoteStackTrace(e, "Hello")); // Previously thrown e = new Exception(); try { throw e; } catch { } Assert.Throws(() => ExceptionDispatchInfo.SetCurrentStackTrace(e)); + Assert.Throws(() => ExceptionDispatchInfo.SetRemoteStackTrace(e, "Hello")); } [Fact] @@ -55,12 +59,29 @@ namespace System.Runtime.ExceptionServices.Tests e = new Exception(); ABCDEFGHIJKLMNOPQRSTUVWXYZ(e); - Assert.Contains(nameof(ABCDEFGHIJKLMNOPQRSTUVWXYZ), e.StackTrace); + Assert.Contains(nameof(ABCDEFGHIJKLMNOPQRSTUVWXYZ), e.StackTrace, StringComparison.Ordinal); e = new Exception(); ABCDEFGHIJKLMNOPQRSTUVWXYZ(e); try { throw e; } catch { } - Assert.Contains(nameof(ABCDEFGHIJKLMNOPQRSTUVWXYZ), e.StackTrace); + Assert.Contains(nameof(ABCDEFGHIJKLMNOPQRSTUVWXYZ), e.StackTrace, StringComparison.Ordinal); + } + + [Fact] + public static void SetRemoteStackTrace_IncludedInExceptionStackTrace() + { + Exception e; + + e = new Exception(); + Assert.Same(e, ExceptionDispatchInfo.SetRemoteStackTrace(e, "pumpkin-anaconda-maritime")); // 3 randomly selected words + Assert.Contains("pumpkin-anaconda-maritime", e.StackTrace, StringComparison.Ordinal); + Assert.DoesNotContain("pumpkin-anaconda-maritime", new StackTrace(e).ToString(), StringComparison.Ordinal); // we shouldn't attempt to parse it in a StackTrace object + + e = new Exception(); + Assert.Same(e, ExceptionDispatchInfo.SetRemoteStackTrace(e, "pumpkin-anaconda-maritime")); + try { throw e; } catch { } + Assert.Contains("pumpkin-anaconda-maritime", e.StackTrace, StringComparison.Ordinal); + Assert.DoesNotContain("pumpkin-anaconda-maritime", new StackTrace(e).ToString(), StringComparison.Ordinal); } [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.NoOptimization)] diff --git a/src/mono/System.Private.CoreLib/src/System/Exception.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Exception.Mono.cs index 5e36982..bb38f05 100644 --- a/src/mono/System.Private.CoreLib/src/System/Exception.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Exception.Mono.cs @@ -2,10 +2,9 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections; -using System.Reflection; using System.Diagnostics; +using System.Reflection; using System.Runtime.InteropServices; -using System.Text; namespace System { @@ -102,22 +101,17 @@ namespace System _stackTraceString = null; } - [StackTraceHidden] - internal void SetCurrentStackTrace() + // Returns true if setting the _remoteStackTraceString field is legal, false if not (immutable exception). + // A false return value means the caller should early-exit the operation. + // Can also throw InvalidOperationException if a stack trace is already set or if object has been thrown. + private bool CanSetRemoteStackTrace() { - // Check to see if the exception already has a stack set in it. if (_traceIPs != null || _stackTraceString != null || _remoteStackTraceString != null) { ThrowHelper.ThrowInvalidOperationException(); } - // Store the current stack trace into the "remote" stack trace, which was originally introduced to support - // remoting of exceptions cross app-domain boundaries, and is thus concatenated into Exception.StackTrace - // when it's retrieved. - var sb = new StringBuilder(256); - new StackTrace(fNeedFileInfo: true).ToString(Diagnostics.StackTrace.TraceFormat.TrailingNewLine, sb); - sb.AppendLine(SR.Exception_EndStackTraceFromPreviousThrow); - _remoteStackTraceString = sb.ToString(); + return true; // mono runtime doesn't have immutable agile exceptions, always return true } private string? CreateSourceName() -- 2.7.4