Merge pull request #70984 from CyrusNajmabadi/tornChecksumWrite

Ensure no torn writes when reading/initializing our solution checksums
dotnet · Nov 28, 2023 · 984df16 · 984df16
2 parents e0ec250 + 749f09f
commit 984df16
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 11 deletions.
diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/DocumentInfo.cs b/src/Workspaces/Core/Portable/Workspace/Solution/DocumentInfo.cs
@@ -7,9 +7,6 @@
 using System.Diagnostics;
 using System.Linq;
 using Microsoft.CodeAnalysis.Host;
-using Microsoft.CodeAnalysis.Serialization;
-using Microsoft.CodeAnalysis.Shared.Extensions;
-using Microsoft.CodeAnalysis.Text;
 using Roslyn.Utilities;
 
 namespace Microsoft.CodeAnalysis
@@ -156,6 +153,9 @@ internal sealed class DocumentAttributes(
             bool isGenerated,
             bool designTimeOnly)
         {
+            /// <summary>
+            /// Lock on <see langword="this"/> to ensure safe reading/writing of this field.
+            /// </summary>
             private Checksum? _lazyChecksum;
 
             /// <summary>
@@ -258,7 +258,16 @@ public static DocumentAttributes ReadFrom(ObjectReader reader)
             }
 
             public Checksum Checksum
-                => _lazyChecksum ??= Checksum.Create(this, static (@this, writer) => @this.WriteTo(writer));
+            {
+                get
+                {
+                    // Ensure reading/writing from the nullable checksum is atomic.
+                    lock (this)
+                    {
+                        return _lazyChecksum ??= Checksum.Create(this, static (@this, writer) => @this.WriteTo(writer));
+                    }
+                }
+            }
         }
     }
 }
diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/ProjectInfo.cs b/src/Workspaces/Core/Portable/Workspace/Solution/ProjectInfo.cs
@@ -8,7 +8,6 @@
 using System.Runtime.CompilerServices;
 using System.Text.RegularExpressions;
 using Microsoft.CodeAnalysis.Diagnostics;
-using Microsoft.CodeAnalysis.Shared.Extensions;
 using Microsoft.CodeAnalysis.Text;
 using Roslyn.Utilities;
 
@@ -497,6 +496,10 @@ internal sealed class ProjectAttributes(
             public Guid TelemetryId { get; } = telemetryId;
 
             private StrongBox<(string?, string?)>? _lazyNameAndFlavor;
+
+            /// <summary>
+            /// Lock on <see langword="this"/> to ensure safe reading/writing of this field.
+            /// </summary>
             private Checksum? _lazyChecksum;
 
             /// <summary>
@@ -649,7 +652,16 @@ public static ProjectAttributes ReadFrom(ObjectReader reader)
             }
 
             public Checksum Checksum
-                => _lazyChecksum ??= Checksum.Create(this, static (@this, writer) => @this.WriteTo(writer));
+            {
+                get
+                {
+                    // Ensure reading/writing from the nullable checksum is atomic.
+                    lock (this)
+                    {
+                        return _lazyChecksum ??= Checksum.Create(this, static (@this, writer) => @this.WriteTo(writer));
+                    }
+                }
+            }
         }
     }
 }
diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionInfo.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionInfo.cs
@@ -4,12 +4,8 @@
 
 using System;
 using System.Collections.Generic;
-using System.Collections.Immutable;
 using System.ComponentModel;
 using Microsoft.CodeAnalysis.Diagnostics;
-using Microsoft.CodeAnalysis.Remote;
-using Microsoft.CodeAnalysis.Serialization;
-using Microsoft.CodeAnalysis.Shared.Extensions;
 using Roslyn.Utilities;
 
 namespace Microsoft.CodeAnalysis
@@ -96,6 +92,9 @@ internal SolutionInfo WithTelemetryId(Guid telemetryId)
         /// </summary>
         internal sealed class SolutionAttributes(SolutionId id, VersionStamp version, string? filePath, Guid telemetryId)
         {
+            /// <summary>
+            /// Lock on <see langword="this"/> to ensure safe reading/writing of this field.
+            /// </summary>
             private Checksum? _lazyChecksum;
 
             /// <summary>
@@ -160,7 +159,16 @@ public static SolutionAttributes ReadFrom(ObjectReader reader)
             }
 
             public Checksum Checksum
-                => _lazyChecksum ??= Checksum.Create(this, static (@this, writer) => @this.WriteTo(writer));
+            {
+                get
+                {
+                    // Ensure reading/writing from the nullable checksum is atomic.
+                    lock (this)
+                    {
+                        return _lazyChecksum ??= Checksum.Create(this, static (@this, writer) => @this.WriteTo(writer));
+                    }
+                }
+            }
         }
     }
 }