From 001a1942dadd79b6b2b6b93bc933cd56b65d2ed7 Mon Sep 17 00:00:00 2001 From: Vance Morrison Date: Fri, 8 Mar 2019 09:52:06 -0800 Subject: [PATCH] Update System.Activity API to be more readonly (dotnet/corefx#35863) * Add back in Read-only-ness for ActivitySpanId and ActivityTraceId I was able to determine a way of casting away readonly-ness (using Unsafe.AsRef), so I was able to make ActivitySpanId and ActivityTraceId read-only (and cast away the read-only when updating the cache). This works better becasuse it allows refs to these IDs to be readonly and thus prevent update to Activity.TraceId and Activity.SpanId (which we don't want). Note that for some reason on NetStandard1.3 ref readonly returns fail (says it can't find the 'InAttribute'). This error seems to be bogus, but the upshot is that older standards gives the C# compiler a problem so we work around it by simply droping the read-only attribute for those (you still have all the functionality). * Resolved conflicts * Fix for NetStandard1.3 * Update to make the cache ref-equality friendly. Commit migrated from https://github.com/dotnet/corefx/commit/0846c3157735b00135500e20a61be9bf879859fa --- ...System.Diagnostics.DiagnosticSource.csproj | 1 + ...em.Diagnostics.DiagnosticSourceActivity.cs | 18 ++++++------ ...System.Diagnostics.DiagnosticSource.csproj | 1 + .../src/System/Diagnostics/Activity.cs | 29 +++++++++++++------ 4 files changed, 31 insertions(+), 18 deletions(-) diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSource.csproj b/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSource.csproj index d391c2389b1..a53ca8311e8 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSource.csproj +++ b/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSource.csproj @@ -26,6 +26,7 @@ + diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs b/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs index 846827672d9..d604f974389 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/ref/System.Diagnostics.DiagnosticSourceActivity.cs @@ -30,12 +30,12 @@ namespace System.Diagnostics public string OperationName { get { throw null; } } public System.Diagnostics.Activity Parent { get { throw null; } } public string ParentId { get { throw null; } } - public ref System.Diagnostics.ActivitySpanId ParentSpanId { get { throw null; } } + public ref readonly System.Diagnostics.ActivitySpanId ParentSpanId { get { throw null; } } public string RootId { get { throw null; } } - public ref System.Diagnostics.ActivitySpanId SpanId { get { throw null; } } + public ref readonly System.Diagnostics.ActivitySpanId SpanId { get { throw null; } } public System.DateTime StartTimeUtc { get { throw null; } } public System.Collections.Generic.IEnumerable> Tags { get { throw null; } } - public ref System.Diagnostics.ActivityTraceId TraceId { get { throw null; } } + public ref readonly System.Diagnostics.ActivityTraceId TraceId { get { throw null; } } public string TraceStateString { get { throw null; } set { } } public System.Diagnostics.Activity AddBaggage(string key, string value) { throw null; } public System.Diagnostics.Activity AddTag(string key, string value) { throw null; } @@ -56,10 +56,10 @@ namespace System.Diagnostics #if ALLOW_PARTIALLY_TRUSTED_CALLERS [System.Security.SecuritySafeCriticalAttribute] #endif - public partial struct ActivitySpanId : System.IEquatable + public readonly partial struct ActivitySpanId : System.IEquatable { - private object _dummy; - private int _dummyPrimitive; + private readonly object _dummy; + private readonly int _dummyPrimitive; public void CopyTo(System.Span destination) { } public static System.Diagnostics.ActivitySpanId CreateFromBytes(System.ReadOnlySpan idData) { throw null; } public static System.Diagnostics.ActivitySpanId CreateFromString(System.ReadOnlySpan idData) { throw null; } @@ -76,10 +76,10 @@ namespace System.Diagnostics #if ALLOW_PARTIALLY_TRUSTED_CALLERS [System.Security.SecuritySafeCriticalAttribute] #endif - public partial struct ActivityTraceId : System.IEquatable + public readonly partial struct ActivityTraceId : System.IEquatable { - private object _dummy; - private int _dummyPrimitive; + private readonly object _dummy; + private readonly int _dummyPrimitive; public void CopyTo(System.Span destination) { } public static System.Diagnostics.ActivityTraceId CreateFromBytes(System.ReadOnlySpan idData) { throw null; } public static System.Diagnostics.ActivityTraceId CreateFromString(System.ReadOnlySpan idData) { throw null; } diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj b/src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj index 9eeedb8e95c..d79905afef6 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System.Diagnostics.DiagnosticSource.csproj @@ -27,6 +27,7 @@ + diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs index bd2032a21ac..55be7e3aef2 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Activity.cs @@ -5,6 +5,7 @@ using System.Buffers.Binary; using System.Buffers.Text; using System.Collections.Generic; +using System.Runtime.CompilerServices; using System.Text; #if ALLOW_PARTIALLY_TRUSTED_CALLERS using System.Security; @@ -456,7 +457,7 @@ namespace System.Diagnostics /// which will have the effect of updating Activity's instance so all subsequent uses /// share the same converted string. /// - public ref ActivitySpanId SpanId + public ref readonly ActivitySpanId SpanId { #if ALLOW_PARTIALLY_TRUSTED_CALLERS [System.Security.SecuritySafeCriticalAttribute] @@ -486,7 +487,7 @@ namespace System.Diagnostics /// which will have the effect of updating Activity's instance so all subsequent uses /// share the same converted string. /// - public ref ActivityTraceId TraceId + public ref readonly ActivityTraceId TraceId { #if ALLOW_PARTIALLY_TRUSTED_CALLERS [System.Security.SecuritySafeCriticalAttribute] @@ -509,7 +510,7 @@ namespace System.Diagnostics /// If the parent Activity ID has the W3C format, this returns the ID for the SpanId part of the ParentId. /// Otherwise it returns a zero SpanId. /// - public ref ActivitySpanId ParentSpanId + public ref readonly ActivitySpanId ParentSpanId { #if ALLOW_PARTIALLY_TRUSTED_CALLERS [System.Security.SecuritySafeCriticalAttribute] @@ -819,7 +820,7 @@ namespace System.Diagnostics #if ALLOW_PARTIALLY_TRUSTED_CALLERS [SecuritySafeCritical] #endif - public unsafe struct ActivityTraceId : IEquatable + public unsafe readonly struct ActivityTraceId : IEquatable { /// /// Create a new TraceId with at random number in it (very likely to be unique) @@ -868,7 +869,12 @@ namespace System.Diagnostics if (_asHexString == null) { fixed (ulong* idPtr = &_id1) - _asHexString = SpanToHexString(new ReadOnlySpan(idPtr, sizeof(ulong) * 2)); + { + // Cast away the read-only-ness of _asHexString, and assign the converted value to it. + // We are OK with this because conceptually the class is still read-only. + ref string strRef = ref Unsafe.AsRef(in _asHexString); + Interlocked.CompareExchange(ref strRef, SpanToHexString(new ReadOnlySpan(idPtr, sizeof(ulong) * 2)), null); + } } return _asHexString; } @@ -989,7 +995,7 @@ namespace System.Diagnostics readonly ulong _id1; readonly ulong _id2; - string _asHexString; // Caches the Hex string + readonly string _asHexString; // Caches the Hex string #endregion } @@ -1005,7 +1011,7 @@ namespace System.Diagnostics #if ALLOW_PARTIALLY_TRUSTED_CALLERS [SecuritySafeCritical] #endif - public unsafe struct ActivitySpanId : IEquatable + public unsafe readonly struct ActivitySpanId : IEquatable { /// /// Create a new SpanId with at random number in it (very likely to be unique) @@ -1054,7 +1060,12 @@ namespace System.Diagnostics if (_asHexString == null) { fixed (ulong* idPtr = &_id1) - _asHexString = ActivityTraceId.SpanToHexString(new ReadOnlySpan(idPtr, sizeof(ulong))); + { + // Cast away the read-only-ness of _asHexString, and assign the converted value to it. + // We are OK with this because conceptually the class is still read-only. + ref string strRef = ref Unsafe.AsRef(in _asHexString); + Interlocked.CompareExchange(ref strRef, ActivityTraceId.SpanToHexString(new ReadOnlySpan(idPtr, sizeof(ulong))), null); + } } return _asHexString; } @@ -1104,7 +1115,7 @@ namespace System.Diagnostics } readonly ulong _id1; - string _asHexString; // Caches the Hex string + readonly string _asHexString; // Caches the Hex string #endregion } } -- 2.34.1