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

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 d391c23..a53ca83 100644 (file)
--- 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 @@
   </ItemGroup>
 
   <ItemGroup Condition="'$(TargetGroup)' == 'netstandard1.1' or '$(TargetGroup)' == 'netstandard1.3'">
+    <Reference Include="System.Runtime.InteropServices" />
     <Reference Include="System.Runtime" />
   </ItemGroup>
 </Project>

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 8468276..d604f97 100644 (file)
--- 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<System.Collections.Generic.KeyValuePair<string, string>> 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<System.Diagnostics.ActivitySpanId>
+    public readonly partial struct ActivitySpanId : System.IEquatable<System.Diagnostics.ActivitySpanId>
     {
-        private object _dummy;
-        private int _dummyPrimitive;
+        private readonly object _dummy;
+        private readonly int _dummyPrimitive;
         public void CopyTo(System.Span<byte> destination) { }
         public static System.Diagnostics.ActivitySpanId CreateFromBytes(System.ReadOnlySpan<byte> idData) { throw null; }
         public static System.Diagnostics.ActivitySpanId CreateFromString(System.ReadOnlySpan<char> 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<System.Diagnostics.ActivityTraceId>
+    public readonly partial struct ActivityTraceId : System.IEquatable<System.Diagnostics.ActivityTraceId>
     {
-        private object _dummy;
-        private int _dummyPrimitive;
+        private readonly object _dummy;
+        private readonly int _dummyPrimitive;
         public void CopyTo(System.Span<byte> destination) { }
         public static System.Diagnostics.ActivityTraceId CreateFromBytes(System.ReadOnlySpan<byte> idData) { throw null; }
         public static System.Diagnostics.ActivityTraceId CreateFromString(System.ReadOnlySpan<char> 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 9eeedb8..d79905a 100644 (file)
--- 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 @@
     <Compile Include="System\Diagnostics\Activity.cs" />
     <Compile Include="System\Diagnostics\DiagnosticSourceActivity.cs" />
     <Reference Include="System.Memory" />
+    <Reference Include="System.Runtime.CompilerServices.Unsafe" />
     <None Include="ActivityUserGuide.md" />
   </ItemGroup>
   <ItemGroup Condition=" '$(TargetGroup)' != 'net45' And '$(TargetGroup)' != 'netstandard1.1'">

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 bd2032a..55be7e3 100644 (file)
--- 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.  
         /// </summary>
-        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.  
         /// </summary>
-        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. 
         /// </summary>
-        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<ActivityTraceId>
+    public unsafe readonly struct ActivityTraceId : IEquatable<ActivityTraceId>
     {
         /// <summary>
         /// 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<byte>(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<byte>(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<ActivitySpanId>
+    public unsafe readonly struct ActivitySpanId : IEquatable<ActivitySpanId>
     {
         /// <summary>
         /// 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<byte>(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<byte>(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
     }
 }