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.
This commit is contained in:
Peter Huene 2018-04-26 17:04:37 -07:00
parent 04066cb5d8
commit b2b3947c68
No known key found for this signature in database
GPG key ID: E1D265D820213D6A
23 changed files with 260 additions and 6 deletions

View file

@ -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>

View file

@ -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()
{
}
}
}

View file

@ -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>

View file

@ -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 @@ namespace Microsoft.DotNet.BuildServer
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 @@ namespace Microsoft.DotNet.BuildServer
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 @@ namespace Microsoft.DotNet.BuildServer
"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;
}
}
}
}

View file

@ -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>

View file

@ -33,7 +33,13 @@ namespace Microsoft.DotNet.BuildServer
{
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))

View file

@ -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>

View file

@ -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>

View file

@ -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>

View file

@ -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>

View file

@ -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>

View file

@ -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>

View file

@ -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>

View file

@ -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>

View file

@ -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>

View file

@ -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>

View file

@ -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>

View file

@ -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>

View file

@ -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>

View file

@ -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}");
}
}
}

View file

@ -94,6 +94,11 @@ namespace Microsoft.Extensions.DependencyModel.Tests
int bufferSize,
FileOptions fileOptions)
{
if (fileMode == FileMode.Open && fileAccess == FileAccess.Read)
{
return OpenRead(path);
}
throw new NotImplementedException();
}

View file

@ -9,7 +9,9 @@ using FluentAssertions;
using Microsoft.DotNet.BuildServer;
using Microsoft.DotNet.Cli.Utils;
using Microsoft.DotNet.Configurer;
using Microsoft.DotNet.Tools.Test.Utilities;
using Microsoft.Extensions.DependencyModel.Tests;
using Microsoft.Extensions.EnvironmentAbstractions;
using Moq;
using Xunit;
using LocalizableStrings = Microsoft.DotNet.BuildServer.LocalizableStrings;
@ -129,12 +131,59 @@ namespace Microsoft.DotNet.Tests.BuildServerTests
razorServer.PidFile.PipeName.Should().Be(PipeName);
}
[Theory]
[InlineData(typeof(UnauthorizedAccessException))]
[InlineData(typeof(IOException))]
public void GivenAnExceptionAccessingTheRazorPidFileItPrintsAWarning(Type exceptionType)
{
const int ProcessId = 1234;
const string ErrorMessage = "failed!";
string pidDirectory = Path.GetFullPath("var/pids/build");
string pidFilePath = Path.Combine(pidDirectory, $"{RazorPidFile.FilePrefix}{ProcessId}");
var directoryMock = new Mock<IDirectory>();
directoryMock.Setup(d => d.Exists(pidDirectory)).Returns(true);
directoryMock.Setup(d => d.EnumerateFiles(pidDirectory, "*")).Returns(new [] { pidFilePath });
var fileMock = new Mock<IFile>();
fileMock
.Setup(f => f.OpenFile(
pidFilePath,
FileMode.Open,
FileAccess.Read,
FileShare.Write | FileShare.Delete,
4096,
FileOptions.None))
.Throws((Exception)Activator.CreateInstance(exceptionType, new object[] { ErrorMessage } ));
var fileSystemMock = new Mock<IFileSystem>();
fileSystemMock.SetupGet(fs => fs.Directory).Returns(directoryMock.Object);
fileSystemMock.SetupGet(fs => fs.File).Returns(fileMock.Object);
var reporter = new BufferedReporter();
var provider = new BuildServerProvider(
fileSystemMock.Object,
CreateEnvironmentProviderMock(pidDirectory).Object,
reporter);
var servers = provider.EnumerateBuildServers(ServerEnumerationFlags.Razor).ToArray();
servers.Should().BeEmpty();
reporter.Lines.Should().Equal(
string.Format(
LocalizableStrings.FailedToReadPidFile,
pidFilePath,
ErrorMessage).Yellow());
}
private Mock<IEnvironmentProvider> CreateEnvironmentProviderMock(string value = null)
{
var provider = new Mock<IEnvironmentProvider>(MockBehavior.Strict);
provider
.Setup(p => p.GetEnvironmentVariable("DOTNET_BUILD_PIDFILE_DIRECTORY"))
.Setup(p => p.GetEnvironmentVariable(BuildServerProvider.PidFileDirectoryVariableName))
.Returns(value);
return provider;

View file

@ -10,17 +10,21 @@ using Microsoft.DotNet.BuildServer;
using Microsoft.DotNet.Cli;
using Microsoft.DotNet.Cli.CommandLine;
using Microsoft.DotNet.Cli.Utils;
using Microsoft.DotNet.TestFramework;
using Microsoft.DotNet.Tools.BuildServer;
using Microsoft.DotNet.Tools.BuildServer.Shutdown;
using Microsoft.DotNet.Tools.Test.Utilities;
using Microsoft.Extensions.EnvironmentAbstractions;
using Moq;
using Xunit;
using Parser = Microsoft.DotNet.Cli.Parser;
using CommandLocalizableStrings = Microsoft.DotNet.BuildServer.LocalizableStrings;
using LocalizableStrings = Microsoft.DotNet.Tools.BuildServer.Shutdown.LocalizableStrings;
using TestBuildServerCommand = Microsoft.DotNet.Tools.Test.Utilities.BuildServerCommand;
namespace Microsoft.DotNet.Tests.Commands
{
public class BuildServerShutdownCommandTests
public class BuildServerShutdownCommandTests : TestBase
{
private readonly BufferedReporter _reporter = new BufferedReporter();
@ -157,6 +161,43 @@ namespace Microsoft.DotNet.Tests.Commands
VerifyShutdownCalls(mocks);
}
[Fact]
public void GivenARunningRazorServerItShutsDownSuccessfully()
{
var pipeName = Path.GetRandomFileName();
var pidDirectory = Path.GetFullPath(Path.Combine(TempRoot.Root, Path.GetRandomFileName()));
var testInstance = TestAssets.Get("TestRazorApp")
.CreateInstance()
.WithSourceFiles();
new BuildCommand()
.WithWorkingDirectory(testInstance.Root)
.WithEnvironmentVariable(BuildServerProvider.PidFileDirectoryVariableName, pidDirectory)
.Execute($"/p:_RazorBuildServerPipeName={pipeName}")
.Should()
.Pass();
var files = Directory.GetFiles(pidDirectory, RazorPidFile.FilePrefix + "*");
files.Length.Should().Be(1);
var pidFile = RazorPidFile.Read(new FilePath(files.First()));
pidFile.PipeName.Should().Be(pipeName);
new TestBuildServerCommand()
.WithWorkingDirectory(testInstance.Root)
.WithEnvironmentVariable(BuildServerProvider.PidFileDirectoryVariableName, pidDirectory)
.ExecuteWithCapturedOutput("shutdown --razor")
.Should()
.Pass()
.And
.HaveStdOutContaining(
string.Format(
LocalizableStrings.ShutDownSucceededWithPid,
CommandLocalizableStrings.RazorServer,
pidFile.ProcessId));
}
private BuildServerShutdownCommand CreateCommand(
string options = "",
IBuildServerProvider serverProvider = null,