Fix Razor server shutdown on Windows.

On Windows, the Razor server correctly creates the pid file with `FileAccess.Write` and `FileOptions.DeleteOnClose`. This requires a share mode of `FileShare.Write | FileShare.Delete` to open. However, the `dotnet build-server shutdown` command was opening the file with `FileShare.Read`. As a result, an `IOException` was being thrown and was not handled. This change first opens the file with the appropriate share access and also properly handles a failure to access or read the contents of the pid file. Additionally, an integration test was added to test that Razor server shutdown works as expected. Fixes #9158.
dotnet · Apr 27, 2018 · b2b3947 · b2b3947
1 parent 04066cb
commit b2b3947
Show file tree

Hide file tree

Showing 23 changed files with 260 additions and 6 deletions.
diff --git a/TestAssets/TestProjects/TestRazorApp/Areas/MyFeature/Pages/Page1.cshtml b/TestAssets/TestProjects/TestRazorApp/Areas/MyFeature/Pages/Page1.cshtml
@@ -0,0 +1,16 @@
+@page
+@model TestRazorApp.MyFeature.Pages.Page1Model
+@{
+    Layout = null;
+}
+
+<!DOCTYPE html>
+
+<html>
+<head>
+    <meta name="viewport" content="width=device-width" />
+    <title>Page1</title>
+</head>
+<body>
+</body>
+</html>
diff --git a/TestAssets/TestProjects/TestRazorApp/Areas/MyFeature/Pages/Page1.cshtml.cs b/TestAssets/TestProjects/TestRazorApp/Areas/MyFeature/Pages/Page1.cshtml.cs
@@ -0,0 +1,17 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Threading.Tasks;
+using Microsoft.AspNetCore.Mvc;
+using Microsoft.AspNetCore.Mvc.RazorPages;
+
+namespace TestRazorApp.MyFeature.Pages
+{
+    public class Page1Model : PageModel
+    {
+        public void OnGet()
+        {
+
+        }
+    }
+}
diff --git a/TestAssets/TestProjects/TestRazorApp/TestRazorApp.csproj b/TestAssets/TestProjects/TestRazorApp/TestRazorApp.csproj
@@ -0,0 +1,11 @@
+<Project Sdk="Microsoft.NET.Sdk.Razor">
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), testAsset.props))\testAsset.props" />
+
+  <PropertyGroup>
+    <TargetFramework>netstandard2.0</TargetFramework>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <PackageReference Include="Microsoft.AspNetCore.Mvc" Version="$(MicrosoftAspNetCoreAllPackageVersion)" />
+  </ItemGroup>
+</Project>
diff --git a/src/dotnet/BuildServer/BuildServerProvider.cs b/src/dotnet/BuildServer/BuildServerProvider.cs
@@ -12,15 +12,19 @@ namespace Microsoft.DotNet.BuildServer
 {
     internal class BuildServerProvider : IBuildServerProvider
     {
+        public const string PidFileDirectoryVariableName = "DOTNET_BUILD_PIDFILE_DIRECTORY";
         private readonly IFileSystem _fileSystem;
         private readonly IEnvironmentProvider _environmentProvider;
+        private readonly IReporter _reporter;
 
         public BuildServerProvider(
             IFileSystem fileSystem = null,
-            IEnvironmentProvider environmentProvider = null)
+            IEnvironmentProvider environmentProvider = null,
+            IReporter reporter = null)
         {
             _fileSystem = fileSystem ?? FileSystemWrapper.Default;
             _environmentProvider = environmentProvider ?? new EnvironmentProvider();
+            _reporter = reporter ?? Reporter.Error;
         }
 
         public IEnumerable<IBuildServer> EnumerateBuildServers(ServerEnumerationFlags flags = ServerEnumerationFlags.All)
@@ -59,7 +63,7 @@ public IEnumerable<IBuildServer> EnumerateBuildServers(ServerEnumerationFlags fl
                 if ((flags & ServerEnumerationFlags.Razor) == ServerEnumerationFlags.Razor &&
                     Path.GetFileName(path).StartsWith(RazorPidFile.FilePrefix))
                 {
-                    var file = RazorPidFile.Read(new FilePath(path), _fileSystem);
+                    var file = ReadRazorPidFile(new FilePath(path));
                     if (file != null)
                     {
                         yield return new RazorServer(file);
@@ -70,7 +74,7 @@ public IEnumerable<IBuildServer> EnumerateBuildServers(ServerEnumerationFlags fl
 
         public DirectoryPath GetPidFileDirectory()
         {
-            var directory = _environmentProvider.GetEnvironmentVariable("DOTNET_BUILD_PIDFILE_DIRECTORY");
+            var directory = _environmentProvider.GetEnvironmentVariable(PidFileDirectoryVariableName);
             if (!string.IsNullOrEmpty(directory))
             {
                 return new DirectoryPath(directory);
@@ -82,5 +86,22 @@ public DirectoryPath GetPidFileDirectory()
                     "pids",
                     "build"));
         }
+
+        private RazorPidFile ReadRazorPidFile(FilePath path)
+        {
+            try
+            {
+                return RazorPidFile.Read(path, _fileSystem);
+            }
+            catch (Exception ex) when (ex is IOException || ex is UnauthorizedAccessException)
+            {
+                _reporter.WriteLine(
+                    string.Format(
+                        LocalizableStrings.FailedToReadPidFile,
+                        path.Value,
+                        ex.Message).Yellow());
+                return null;
+            }
+        }
     }
 }
diff --git a/src/dotnet/BuildServer/LocalizableStrings.resx b/src/dotnet/BuildServer/LocalizableStrings.resx
@@ -129,4 +129,7 @@
   <data name="ShutdownCommandFailed" xml:space="preserve">
     <value>The shutdown command failed: {0}</value>
   </data>
+  <data name="FailedToReadPidFile" xml:space="preserve">
+    <value>Failed to read pid file '{0}': {1}</value>
+  </data>
 </root>
diff --git a/src/dotnet/BuildServer/RazorPidFile.cs b/src/dotnet/BuildServer/RazorPidFile.cs
@@ -33,7 +33,13 @@ public static RazorPidFile Read(FilePath path, IFileSystem fileSystem = null)
         {
             fileSystem = fileSystem ?? FileSystemWrapper.Default;
 
-            using (var stream = fileSystem.File.OpenRead(path.Value))
+            using (var stream = fileSystem.File.OpenFile(
+                path.Value,
+                FileMode.Open,
+                FileAccess.Read,
+                FileShare.Write | FileShare.Delete,
+                4096,
+                FileOptions.None))
             using (var reader = new StreamReader(stream, Encoding.UTF8))
             {
                 if (!int.TryParse(reader.ReadLine(), out var processId))

diff --git a/src/dotnet/BuildServer/xlf/LocalizableStrings.cs.xlf b/src/dotnet/BuildServer/xlf/LocalizableStrings.cs.xlf
@@ -22,6 +22,11 @@
         <target state="new">The shutdown command failed: {0}</target>
         <note />
       </trans-unit>
+      <trans-unit id="FailedToReadPidFile">
+        <source>Failed to read pid file '{0}': {1}</source>
+        <target state="new">Failed to read pid file '{0}': {1}</target>
+        <note />
+      </trans-unit>
     </body>
   </file>
 </xliff>
diff --git a/src/dotnet/BuildServer/xlf/LocalizableStrings.de.xlf b/src/dotnet/BuildServer/xlf/LocalizableStrings.de.xlf
@@ -22,6 +22,11 @@
         <target state="new">The shutdown command failed: {0}</target>
         <note />
       </trans-unit>
+      <trans-unit id="FailedToReadPidFile">
+        <source>Failed to read pid file '{0}': {1}</source>
+        <target state="new">Failed to read pid file '{0}': {1}</target>
+        <note />
+      </trans-unit>
     </body>
   </file>
 </xliff>
diff --git a/src/dotnet/BuildServer/xlf/LocalizableStrings.es.xlf b/src/dotnet/BuildServer/xlf/LocalizableStrings.es.xlf
@@ -22,6 +22,11 @@
         <target state="new">The shutdown command failed: {0}</target>
         <note />
       </trans-unit>
+      <trans-unit id="FailedToReadPidFile">
+        <source>Failed to read pid file '{0}': {1}</source>
+        <target state="new">Failed to read pid file '{0}': {1}</target>
+        <note />
+      </trans-unit>
     </body>
   </file>
 </xliff>
diff --git a/src/dotnet/BuildServer/xlf/LocalizableStrings.fr.xlf b/src/dotnet/BuildServer/xlf/LocalizableStrings.fr.xlf
@@ -22,6 +22,11 @@
         <target state="new">The shutdown command failed: {0}</target>
         <note />
       </trans-unit>
+      <trans-unit id="FailedToReadPidFile">
+        <source>Failed to read pid file '{0}': {1}</source>
+        <target state="new">Failed to read pid file '{0}': {1}</target>
+        <note />
+      </trans-unit>
     </body>
   </file>
 </xliff>
diff --git a/src/dotnet/BuildServer/xlf/LocalizableStrings.it.xlf b/src/dotnet/BuildServer/xlf/LocalizableStrings.it.xlf
@@ -22,6 +22,11 @@
         <target state="new">The shutdown command failed: {0}</target>
         <note />
       </trans-unit>
+      <trans-unit id="FailedToReadPidFile">
+        <source>Failed to read pid file '{0}': {1}</source>
+        <target state="new">Failed to read pid file '{0}': {1}</target>
+        <note />
+      </trans-unit>
     </body>
   </file>
 </xliff>
diff --git a/src/dotnet/BuildServer/xlf/LocalizableStrings.ja.xlf b/src/dotnet/BuildServer/xlf/LocalizableStrings.ja.xlf
@@ -22,6 +22,11 @@
         <target state="new">The shutdown command failed: {0}</target>
         <note />
       </trans-unit>
+      <trans-unit id="FailedToReadPidFile">
+        <source>Failed to read pid file '{0}': {1}</source>
+        <target state="new">Failed to read pid file '{0}': {1}</target>
+        <note />
+      </trans-unit>
     </body>
   </file>
 </xliff>
diff --git a/src/dotnet/BuildServer/xlf/LocalizableStrings.ko.xlf b/src/dotnet/BuildServer/xlf/LocalizableStrings.ko.xlf
@@ -22,6 +22,11 @@
         <target state="new">The shutdown command failed: {0}</target>
         <note />
       </trans-unit>
+      <trans-unit id="FailedToReadPidFile">
+        <source>Failed to read pid file '{0}': {1}</source>
+        <target state="new">Failed to read pid file '{0}': {1}</target>
+        <note />
+      </trans-unit>
     </body>
   </file>
 </xliff>
diff --git a/src/dotnet/BuildServer/xlf/LocalizableStrings.pl.xlf b/src/dotnet/BuildServer/xlf/LocalizableStrings.pl.xlf
@@ -22,6 +22,11 @@
         <target state="new">The shutdown command failed: {0}</target>
         <note />
       </trans-unit>
+      <trans-unit id="FailedToReadPidFile">
+        <source>Failed to read pid file '{0}': {1}</source>
+        <target state="new">Failed to read pid file '{0}': {1}</target>
+        <note />
+      </trans-unit>
     </body>
   </file>
 </xliff>
diff --git a/src/dotnet/BuildServer/xlf/LocalizableStrings.pt-BR.xlf b/src/dotnet/BuildServer/xlf/LocalizableStrings.pt-BR.xlf
@@ -22,6 +22,11 @@
         <target state="new">The shutdown command failed: {0}</target>
         <note />
       </trans-unit>
+      <trans-unit id="FailedToReadPidFile">
+        <source>Failed to read pid file '{0}': {1}</source>
+        <target state="new">Failed to read pid file '{0}': {1}</target>
+        <note />
+      </trans-unit>
     </body>
   </file>
 </xliff>
diff --git a/src/dotnet/BuildServer/xlf/LocalizableStrings.ru.xlf b/src/dotnet/BuildServer/xlf/LocalizableStrings.ru.xlf
@@ -22,6 +22,11 @@
         <target state="new">The shutdown command failed: {0}</target>
         <note />
       </trans-unit>
+      <trans-unit id="FailedToReadPidFile">
+        <source>Failed to read pid file '{0}': {1}</source>
+        <target state="new">Failed to read pid file '{0}': {1}</target>
+        <note />
+      </trans-unit>
     </body>
   </file>
 </xliff>
diff --git a/src/dotnet/BuildServer/xlf/LocalizableStrings.tr.xlf b/src/dotnet/BuildServer/xlf/LocalizableStrings.tr.xlf
@@ -22,6 +22,11 @@
         <target state="new">The shutdown command failed: {0}</target>
         <note />
       </trans-unit>
+      <trans-unit id="FailedToReadPidFile">
+        <source>Failed to read pid file '{0}': {1}</source>
+        <target state="new">Failed to read pid file '{0}': {1}</target>
+        <note />
+      </trans-unit>
     </body>
   </file>
 </xliff>
diff --git a/src/dotnet/BuildServer/xlf/LocalizableStrings.zh-Hans.xlf b/src/dotnet/BuildServer/xlf/LocalizableStrings.zh-Hans.xlf
@@ -22,6 +22,11 @@
         <target state="new">The shutdown command failed: {0}</target>
         <note />
       </trans-unit>
+      <trans-unit id="FailedToReadPidFile">
+        <source>Failed to read pid file '{0}': {1}</source>
+        <target state="new">Failed to read pid file '{0}': {1}</target>
+        <note />
+      </trans-unit>
     </body>
   </file>
 </xliff>
diff --git a/src/dotnet/BuildServer/xlf/LocalizableStrings.zh-Hant.xlf b/src/dotnet/BuildServer/xlf/LocalizableStrings.zh-Hant.xlf
@@ -22,6 +22,11 @@
         <target state="new">The shutdown command failed: {0}</target>
         <note />
       </trans-unit>
+      <trans-unit id="FailedToReadPidFile">
+        <source>Failed to read pid file '{0}': {1}</source>
+        <target state="new">Failed to read pid file '{0}': {1}</target>
+        <note />
+      </trans-unit>
     </body>
   </file>
 </xliff>
diff --git a/test/Microsoft.DotNet.Tools.Tests.Utilities/Commands/BuildServerCommand.cs b/test/Microsoft.DotNet.Tools.Tests.Utilities/Commands/BuildServerCommand.cs
@@ -0,0 +1,20 @@
+// Copyright (c) .NET Foundation and contributors. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using Microsoft.DotNet.Cli.Utils;
+
+namespace Microsoft.DotNet.Tools.Test.Utilities
+{
+    public sealed class BuildServerCommand : DotnetCommand
+    {
+        public override CommandResult Execute(string args = "")
+        {
+            return base.Execute($"build-server {args}");
+        }
+
+        public override CommandResult ExecuteWithCapturedOutput(string args = "")
+        {
+            return base.ExecuteWithCapturedOutput($"build-server {args}");
+        }
+    }
+}
diff --git a/test/Microsoft.DotNet.Tools.Tests.Utilities/Mock/FileSystemMockBuilder.cs b/test/Microsoft.DotNet.Tools.Tests.Utilities/Mock/FileSystemMockBuilder.cs
@@ -94,6 +94,11 @@ public Stream OpenRead(string path)
                 int bufferSize,
                 FileOptions fileOptions)
             {
+                if (fileMode == FileMode.Open && fileAccess == FileAccess.Read)
+                {
+                    return OpenRead(path);
+                }
+
                 throw new NotImplementedException();
             }