Merge pull request #73238 from CyrusNajmabadi/tempStorage

src/Workspaces/Core/Portable/TemporaryStorage/TemporaryStorageService.Factory.cs

@@ -22,13 +22,7 @@ internal partial class Factory(
         public IWorkspaceService CreateService(HostWorkspaceServices workspaceServices)
         {
             var textFactory = workspaceServices.GetRequiredService<ITextFactoryService>();
-
-            // MemoryMapped files which are used by the TemporaryStorageService are present in .NET Framework (including Mono)
-            // and .NET Core Windows. For non-Windows .NET Core scenarios, we can return the TrivialTemporaryStorageService
-            // until https://github.com/dotnet/runtime/issues/30878 is fixed.
-            return PlatformInformation.IsWindows || PlatformInformation.IsRunningOnMono
-                ? new TemporaryStorageService(workspaceThreadingService, textFactory)
-                : TrivialTemporaryStorageService.Instance;
+            return new TemporaryStorageService(workspaceThreadingService, textFactory);
         }
     }
 }

src/Workspaces/Core/Portable/TemporaryStorage/TemporaryStorageService.MemoryMappedInfo.cs

@@ -24,7 +24,7 @@ internal partial class TemporaryStorageService
     /// <para>This class and its nested types have familiar APIs and predictable behavior when used in other code, but
     /// are non-trivial to work on.</para>
     /// </remarks>
-    internal sealed class MemoryMappedInfo(MemoryMappedFile memoryMappedFile, string name, long offset, long size)
+    internal sealed class MemoryMappedInfo(MemoryMappedFile memoryMappedFile, string? name, long offset, long size)
     {
         /// <summary>
         /// The memory mapped file.
@@ -45,16 +45,14 @@ internal sealed class MemoryMappedInfo(MemoryMappedFile memoryMappedFile, string
         /// </remarks>
         private ReferenceCountedDisposable<MemoryMappedViewAccessor>.WeakReference _weakReadAccessor;
 
-        public static MemoryMappedInfo OpenExisting(string name, long offset, long size)
-            => new(MemoryMappedFile.OpenExisting(name), name, offset, size);
-
-        public static MemoryMappedInfo CreateNew(string name, long size)
+        public static MemoryMappedInfo CreateNew(string? name, long size)
             => new(MemoryMappedFile.CreateNew(name, size), name, offset: 0, size);
 
         /// <summary>
-        /// The name of the memory mapped file.
+        /// The name of the memory mapped file. Non null on systems that support named memory mapped files, null
+        /// otherwise..
         /// </summary>
-        public string Name { get; } = name;
+        public string? Name { get; } = name;
 
         /// <summary>
         /// The offset into the memory mapped file of the region described by the current

src/Workspaces/Core/Portable/TemporaryStorage/TemporaryStorageService.TemporaryStorageStreamHandle.cs

@@ -18,9 +18,6 @@ public sealed class TemporaryStorageStreamHandle(
     {
         public TemporaryStorageIdentifier Identifier => identifier;
 
-        Stream ITemporaryStorageStreamHandle.ReadFromTemporaryStorage(CancellationToken cancellationToken)
-            => ReadFromTemporaryStorage(cancellationToken);
-
         public UnmanagedMemoryStream ReadFromTemporaryStorage(CancellationToken cancellationToken)
         {
             using (Logger.LogBlock(FunctionId.TemporaryStorageServiceFactory_ReadStream, cancellationToken))

src/Workspaces/Core/Portable/TemporaryStorage/TemporaryStorageService.cs

@@ -173,6 +173,7 @@ MemoryMappedInfo WriteToMemoryMappedFile()
 
     internal static TemporaryStorageStreamHandle GetStreamHandle(TemporaryStorageIdentifier storageIdentifier)
     {
+        Contract.ThrowIfNull(storageIdentifier.Name, $"{nameof(GetStreamHandle)} should only be called for VS on Windows (where named memory mapped files as supported)");
         var memoryMappedFile = MemoryMappedFile.OpenExisting(storageIdentifier.Name);
         return new(memoryMappedFile, storageIdentifier);
     }
@@ -183,6 +184,7 @@ internal TemporaryStorageTextHandle GetTextHandle(
         Encoding? encoding,
         ImmutableArray<byte> contentHash)
     {
+        Contract.ThrowIfNull(storageIdentifier.Name, $"{nameof(GetTextHandle)} should only be called for VS on Windows (where named memory mapped files as supported)");
         var memoryMappedFile = MemoryMappedFile.OpenExisting(storageIdentifier.Name);
         return new(this, memoryMappedFile, storageIdentifier, checksumAlgorithm, encoding, contentHash);
     }
@@ -222,15 +224,21 @@ private MemoryMappedInfo CreateTemporaryStorage(long size)
             else
             {
                 // Reserve additional space in the existing storage location
-                Contract.ThrowIfNull(_name);
                 _offset += size;
                 return new MemoryMappedInfo(reference, _name, _offset - size, size);
             }
         }
     }
 
-    public static string CreateUniqueName(long size)
-        => "Roslyn Temp Storage " + size.ToString() + " " + Guid.NewGuid().ToString("N");
+    public static string? CreateUniqueName(long size)
+    {
+        // MemoryMapped files which are used by the TemporaryStorageService are present in .NET Framework (including
+        // Mono) and .NET Core Windows. For non-Windows .NET Core scenarios, we return null to enable create the memory
+        // mapped file (just not in a way that can be shared across processes).
+        return PlatformInformation.IsWindows || PlatformInformation.IsRunningOnMono
+            ? $"Roslyn Shared File: Size={size} Id={Guid.NewGuid():N}"
+            : null;
+    }
 
     public sealed class TemporaryStorageTextHandle(
         TemporaryStorageService storageService,

src/Workspaces/Core/Portable/Workspace/Host/TemporaryStorage/ITemporaryStorageService.cs

@@ -18,7 +18,13 @@ public interface ITemporaryStorageService : IWorkspaceService
 }
 
 /// <summary>
-/// API to allow a client to write data to memory-mapped-file storage (allowing it to be shared across processes).
+/// API to allow a client to write data to memory-mapped-file storage.  That data can be read back in within the same
+/// process using a handle returned from the writing call.  The data can optionally be read back in from a different
+/// process, using the information contained with the handle's <code>Identifier</code> (see <see
+/// cref="TemporaryStorageIdentifier"/>), but only on systems that support named memory mapped files.  Currently, this
+/// is any .net on Windows and mono on unix systems.  This is not supported on .net core on unix systems (tracked here
+/// https://github.com/dotnet/runtime/issues/30878).  This is not a problem in practice as cross process sharing is only
+/// needed by the VS host, which is windows only.
 /// </summary>
 internal interface ITemporaryStorageServiceInternal : IWorkspaceService
 {

src/Workspaces/Core/Portable/Workspace/Host/TemporaryStorage/ITemporaryStorageStreamHandle.cs

@@ -22,5 +22,5 @@ internal interface ITemporaryStorageStreamHandle
     /// Reads the data indicated to by this handle into a stream.  This stream can be created in a different process
     /// than the one that wrote the data originally.
     /// </summary>
-    Stream ReadFromTemporaryStorage(CancellationToken cancellationToken);
+    UnmanagedMemoryStream ReadFromTemporaryStorage(CancellationToken cancellationToken);
 }

src/Workspaces/Core/Portable/Workspace/Host/TemporaryStorage/TemporaryStorageIdentifier.cs

@@ -2,21 +2,22 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-using System.Runtime.Serialization;
 using Roslyn.Utilities;
 
 namespace Microsoft.CodeAnalysis.Host;
 
 /// <summary>
-/// Identifier for a stream of data placed in a segment of temporary storage (generally a memory mapped file). Can be
-/// used to identify that segment across processes, allowing for efficient sharing of data.
+/// Identifier for a stream of data placed in a segment of a memory mapped file. Can be used to identify that segment
+/// across processes (where supported), allowing for efficient sharing of data.
 /// </summary>
+/// <param name="Name">The name of the segment in the temporary storage.  <see langword="null"/> on platforms that don't
+/// support cross process sharing of named memory mapped files.</param>
 internal sealed record TemporaryStorageIdentifier(
-    string Name, long Offset, long Size)
+    string? Name, long Offset, long Size)
 {
     public static TemporaryStorageIdentifier ReadFrom(ObjectReader reader)
         => new(
-            reader.ReadRequiredString(),
+            reader.ReadString(),
             reader.ReadInt64(),
             reader.ReadInt64());
 

src/Workspaces/CoreTest/SolutionTests/SolutionTests.cs

@@ -2639,11 +2639,8 @@ public void TestUpdatingFilePathUpdatesSyntaxTree()
             }
         }
 
-#if NETCOREAPP
-        [SupportedOSPlatform("windows")]
-#endif
         [MethodImpl(MethodImplOptions.NoInlining)]
-        [Fact, WorkItem("http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/542736")]
+        [ConditionalFact(typeof(WindowsOnly)), WorkItem("http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/542736")]
         public void TestDocumentChangedOnDiskIsNotObserved()
         {
             var text1 = "public class A {}";
@@ -2669,17 +2666,7 @@ public void TestDocumentChangedOnDiskIsNotObserved()
             Assert.Equal(text2, textOnDisk);
 
             // stop observing it and let GC reclaim it
-            if (PlatformInformation.IsWindows || PlatformInformation.IsRunningOnMono)
-            {
-                Assert.IsType<TemporaryStorageService>(workspace.Services.GetService<ITemporaryStorageServiceInternal>());
-                observedText.AssertReleased();
-            }
-            else
-            {
-                // If this assertion fails, it means a new target supports the true temporary storage service, and the
-                // condition above should be updated to ensure 'AssertReleased' is called for this target.
-                Assert.IsType<TrivialTemporaryStorageService>(workspace.Services.GetService<ITemporaryStorageServiceInternal>());
-            }
+            observedText.AssertReleased();
 
             // if we ask for the same text again we should get the original content
             var observedText2 = sol.GetDocument(did).GetTextAsync().Result;