strengthen debug assertions and add clarifying comments

dotnet · Jun 18, 2021 · 99529f7 · 99529f7
1 parent a858d3b
commit 99529f7
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 0 deletions.
diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs
@@ -220,7 +220,12 @@ internal bool TryRead(ref Utf8JsonReader reader, Type typeToConvert, JsonSeriali
             // Remember if we were a continuation here since Push() may affect IsContinuation.
             bool wasContinuation = state.IsContinuation;
 
+#if DEBUG
+            // DEBUG: ensure push/pop operations preserve stack integrity
+            JsonTypeInfo originalJsonTypeInfo = state.Current.JsonTypeInfo;
+#endif
             state.Push();
+            Debug.Assert(TypeToConvert.IsAssignableFrom(state.Current.JsonTypeInfo.Type));
 
 #if !DEBUG
             // For performance, only perform validation on internal converters on debug builds.
@@ -257,6 +262,9 @@ internal bool TryRead(ref Utf8JsonReader reader, Type typeToConvert, JsonSeriali
 
                         value = default;
                         state.Pop(true);
+#if DEBUG
+                        Debug.Assert(ReferenceEquals(originalJsonTypeInfo, state.Current.JsonTypeInfo));
+#endif
                         return true;
                     }
 
@@ -288,6 +296,9 @@ internal bool TryRead(ref Utf8JsonReader reader, Type typeToConvert, JsonSeriali
             }
 
             state.Pop(success);
+#if DEBUG
+            Debug.Assert(ReferenceEquals(originalJsonTypeInfo, state.Current.JsonTypeInfo));
+#endif
             return success;
         }
 
@@ -298,6 +309,9 @@ internal override sealed bool TryReadAsObject(ref Utf8JsonReader reader, JsonSer
             return success;
         }
 
+        /// <summary>
+        /// Overridden by the nullable converter to prevent boxing of values by the JIT.
+        /// </summary>
         internal virtual bool IsNull(in T value) => value == null;
 
         internal bool TryWrite(Utf8JsonWriter writer, in T value, JsonSerializerOptions options, ref WriteStack state)
@@ -416,7 +430,12 @@ internal bool TryWrite(Utf8JsonWriter writer, in T value, JsonSerializerOptions
 
             bool isContinuation = state.IsContinuation;
 
+#if DEBUG
+            // DEBUG: ensure push/pop operations preserve stack integrity
+            JsonTypeInfo originalJsonTypeInfo = state.Current.JsonTypeInfo;
+#endif
             state.Push();
+            Debug.Assert(TypeToConvert.IsAssignableFrom(state.Current.JsonTypeInfo.Type));
 
             if (!isContinuation)
             {
@@ -432,6 +451,9 @@ internal bool TryWrite(Utf8JsonWriter writer, in T value, JsonSerializerOptions
             }
 
             state.Pop(success);
+#if DEBUG
+            Debug.Assert(ReferenceEquals(originalJsonTypeInfo, state.Current.JsonTypeInfo));
+#endif
 
             if (ignoreCyclesPopReference)
             {

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStack.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStack.cs
@@ -175,6 +175,7 @@ public void Push()
             }
 
             SetConstructorArgumentState();
+            Debug.Assert(JsonPath() is not null);
         }
 
         public void Pop(bool success)

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStack.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStack.cs
@@ -155,6 +155,8 @@ public void Push()
                     _count++;
                 }
             }
+
+            Debug.Assert(PropertyPath() is not null);
         }
 
         public void Pop(bool success)
-Original file line number
+Diff line change
@@ Expand Up / @@ -175,6 +175,7 @@ public void Push() @@
                 }
                 SetConstructorArgumentState();
+                Debug.Assert(JsonPath() is not null);
             }
             public void Pop(bool success)
@@ Expand Down @@