Skip to content

Commit c2c359d

Browse files
committed
Cleanup processes started by UseReactDevelopmentServer and UseAngularCliServer
Fixes #11597
1 parent 840e10d commit c2c359d

8 files changed

+280
-17
lines changed

src/Middleware/SpaServices.Extensions/src/AngularCli/AngularCliBuilder.cs

+7-2
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
using Microsoft.AspNetCore.NodeServices.Util;
77
using Microsoft.AspNetCore.SpaServices.Prerendering;
88
using Microsoft.AspNetCore.SpaServices.Util;
9+
using Microsoft.Extensions.DependencyInjection;
10+
using Microsoft.Extensions.Hosting;
911
using System;
1012
using System.IO;
1113
using System.Text.RegularExpressions;
@@ -48,15 +50,18 @@ public async Task Build(ISpaBuilder spaBuilder)
4850
throw new InvalidOperationException($"To use {nameof(AngularCliBuilder)}, you must supply a non-empty value for the {nameof(SpaOptions.SourcePath)} property of {nameof(SpaOptions)} when calling {nameof(SpaApplicationBuilderExtensions.UseSpa)}.");
4951
}
5052

53+
var appBuilder = spaBuilder.ApplicationBuilder;
54+
var applicationStoppingToken = appBuilder.ApplicationServices.GetRequiredService<IHostApplicationLifetime>().ApplicationStopping;
5155
var logger = LoggerFinder.GetOrCreateLogger(
52-
spaBuilder.ApplicationBuilder,
56+
appBuilder,
5357
nameof(AngularCliBuilder));
5458
var scriptRunner = new NodeScriptRunner(
5559
sourcePath,
5660
_scriptName,
5761
"--watch",
5862
null,
59-
pkgManagerCommand);
63+
pkgManagerCommand,
64+
applicationStoppingToken);
6065
scriptRunner.AttachToLogger(logger);
6166

6267
using (var stdOutReader = new EventedStreamStringReader(scriptRunner.StdOut))

src/Middleware/SpaServices.Extensions/src/AngularCli/AngularCliMiddleware.cs

+6-3
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
using System.Threading;
1414
using System.Net.Http;
1515
using Microsoft.AspNetCore.SpaServices.Extensions.Util;
16+
using Microsoft.Extensions.Hosting;
17+
using Microsoft.Extensions.DependencyInjection;
1618

1719
namespace Microsoft.AspNetCore.SpaServices.AngularCli
1820
{
@@ -40,8 +42,9 @@ public static void Attach(
4042

4143
// Start Angular CLI and attach to middleware pipeline
4244
var appBuilder = spaBuilder.ApplicationBuilder;
45+
var applicationStoppingToken = appBuilder.ApplicationServices.GetRequiredService<IHostApplicationLifetime>().ApplicationStopping;
4346
var logger = LoggerFinder.GetOrCreateLogger(appBuilder, LogCategoryName);
44-
var angularCliServerInfoTask = StartAngularCliServerAsync(sourcePath, scriptName, pkgManagerCommand, devServerPort, logger);
47+
var angularCliServerInfoTask = StartAngularCliServerAsync(sourcePath, scriptName, pkgManagerCommand, devServerPort, logger, applicationStoppingToken);
4548

4649
// Everything we proxy is hardcoded to target http://localhost because:
4750
// - the requests are always from the local machine (we're not accepting remote
@@ -64,7 +67,7 @@ public static void Attach(
6467
}
6568

6669
private static async Task<AngularCliServerInfo> StartAngularCliServerAsync(
67-
string sourcePath, string scriptName, string pkgManagerCommand, int portNumber, ILogger logger)
70+
string sourcePath, string scriptName, string pkgManagerCommand, int portNumber, ILogger logger, CancellationToken applicationStoppingToken)
6871
{
6972
if (portNumber == default(int))
7073
{
@@ -73,7 +76,7 @@ private static async Task<AngularCliServerInfo> StartAngularCliServerAsync(
7376
logger.LogInformation($"Starting @angular/cli on port {portNumber}...");
7477

7578
var scriptRunner = new NodeScriptRunner(
76-
sourcePath, scriptName, $"--port {portNumber}", null, pkgManagerCommand);
79+
sourcePath, scriptName, $"--port {portNumber}", null, pkgManagerCommand, applicationStoppingToken);
7780
scriptRunner.AttachToLogger(logger);
7881

7982
Match openBrowserLine;

src/Middleware/SpaServices.Extensions/src/Npm/NodeScriptRunner.cs

+17-4
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using System.Runtime.InteropServices;
99
using System.Text.RegularExpressions;
1010
using System.Collections.Generic;
11+
using System.Threading;
1112

1213
// This is under the NodeServices namespace because post 2.1 it will be moved to that package
1314
namespace Microsoft.AspNetCore.NodeServices.Npm
@@ -18,12 +19,13 @@ namespace Microsoft.AspNetCore.NodeServices.Npm
1819
/// </summary>
1920
internal class NodeScriptRunner
2021
{
22+
private Process _npmProcess;
2123
public EventedStreamReader StdOut { get; }
2224
public EventedStreamReader StdErr { get; }
2325

2426
private static Regex AnsiColorRegex = new Regex("\x001b\\[[0-9;]*m", RegexOptions.None, TimeSpan.FromSeconds(1));
2527

26-
public NodeScriptRunner(string workingDirectory, string scriptName, string arguments, IDictionary<string, string> envVars, string pkgManagerCommand)
28+
public NodeScriptRunner(string workingDirectory, string scriptName, string arguments, IDictionary<string, string> envVars, string pkgManagerCommand, CancellationToken applicationStoppingToken)
2729
{
2830
if (string.IsNullOrEmpty(workingDirectory))
2931
{
@@ -69,9 +71,11 @@ public NodeScriptRunner(string workingDirectory, string scriptName, string argum
6971
}
7072
}
7173

72-
var process = LaunchNodeProcess(processStartInfo, pkgManagerCommand);
73-
StdOut = new EventedStreamReader(process.StandardOutput);
74-
StdErr = new EventedStreamReader(process.StandardError);
74+
_npmProcess = LaunchNodeProcess(processStartInfo, pkgManagerCommand);
75+
StdOut = new EventedStreamReader(_npmProcess.StandardOutput);
76+
StdErr = new EventedStreamReader(_npmProcess.StandardError);
77+
78+
applicationStoppingToken.Register(EnsureNpmIsDead);
7579
}
7680

7781
public void AttachToLogger(ILogger logger)
@@ -132,5 +136,14 @@ private static Process LaunchNodeProcess(ProcessStartInfo startInfo, string comm
132136
throw new InvalidOperationException(message, ex);
133137
}
134138
}
139+
140+
private void EnsureNpmIsDead()
141+
{
142+
if (_npmProcess != null && !_npmProcess.HasExited)
143+
{
144+
_npmProcess.Kill(entireProcessTree: true);
145+
_npmProcess = null;
146+
}
147+
}
135148
}
136149
}

src/Middleware/SpaServices.Extensions/src/ReactDevelopmentServer/ReactDevelopmentServerMiddleware.cs

+7-3
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@
1212
using System.Text.RegularExpressions;
1313
using System.Threading.Tasks;
1414
using Microsoft.AspNetCore.SpaServices.Extensions.Util;
15+
using Microsoft.Extensions.DependencyInjection;
16+
using Microsoft.Extensions.Hosting;
17+
using System.Threading;
1518

1619
namespace Microsoft.AspNetCore.SpaServices.ReactDevelopmentServer
1720
{
@@ -39,8 +42,9 @@ public static void Attach(
3942

4043
// Start create-react-app and attach to middleware pipeline
4144
var appBuilder = spaBuilder.ApplicationBuilder;
45+
var applicationStoppingToken = appBuilder.ApplicationServices.GetRequiredService<IHostApplicationLifetime>().ApplicationStopping;
4246
var logger = LoggerFinder.GetOrCreateLogger(appBuilder, LogCategoryName);
43-
var portTask = StartCreateReactAppServerAsync(sourcePath, scriptName, pkgManagerCommand, devServerPort, logger);
47+
var portTask = StartCreateReactAppServerAsync(sourcePath, scriptName, pkgManagerCommand, devServerPort, logger, applicationStoppingToken);
4448

4549
// Everything we proxy is hardcoded to target http://localhost because:
4650
// - the requests are always from the local machine (we're not accepting remote
@@ -63,7 +67,7 @@ public static void Attach(
6367
}
6468

6569
private static async Task<int> StartCreateReactAppServerAsync(
66-
string sourcePath, string scriptName, string pkgManagerCommand, int portNumber, ILogger logger)
70+
string sourcePath, string scriptName, string pkgManagerCommand, int portNumber, ILogger logger, CancellationToken applicationStoppingToken)
6771
{
6872
if (portNumber == default(int))
6973
{
@@ -77,7 +81,7 @@ private static async Task<int> StartCreateReactAppServerAsync(
7781
{ "BROWSER", "none" }, // We don't want create-react-app to open its own extra browser window pointing to the internal dev server port
7882
};
7983
var scriptRunner = new NodeScriptRunner(
80-
sourcePath, scriptName, null, envVars, pkgManagerCommand);
84+
sourcePath, scriptName, null, envVars, pkgManagerCommand, applicationStoppingToken);
8185
scriptRunner.AttachToLogger(logger);
8286

8387
using (var stdErrReader = new EventedStreamStringReader(scriptRunner.StdErr))
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using Microsoft.Extensions.Logging;
4+
using Microsoft.Extensions.Logging.Abstractions;
5+
using Xunit.Abstractions;
6+
7+
namespace Microsoft.AspNetCore.SpaServices.Extensions.Tests
8+
{
9+
public class ListLoggerFactory : ILoggerFactory
10+
{
11+
private readonly Func<string, bool> _shouldLogCategory;
12+
private bool _disposed;
13+
14+
public ListLoggerFactory()
15+
: this(_ => true)
16+
{
17+
}
18+
19+
public ListLoggerFactory(Func<string, bool> shouldLogCategory)
20+
{
21+
_shouldLogCategory = shouldLogCategory;
22+
Logger = new ListLogger();
23+
}
24+
25+
public List<(LogLevel Level, EventId Id, string Message, object State, Exception Exception)> Log => Logger.LoggedEvents;
26+
protected ListLogger Logger { get; set; }
27+
28+
public virtual void Clear() => Logger.Clear();
29+
30+
public void SetTestOutputHelper(ITestOutputHelper testOutputHelper)
31+
{
32+
Logger.TestOutputHelper = testOutputHelper;
33+
}
34+
35+
public virtual ILogger CreateLogger(string name)
36+
{
37+
CheckDisposed();
38+
39+
return !_shouldLogCategory(name)
40+
? (ILogger)NullLogger.Instance
41+
: Logger;
42+
}
43+
44+
private void CheckDisposed()
45+
{
46+
if (_disposed)
47+
{
48+
throw new ObjectDisposedException(nameof(ListLoggerFactory));
49+
}
50+
}
51+
52+
public void AddProvider(ILoggerProvider provider)
53+
{
54+
CheckDisposed();
55+
}
56+
57+
public void Dispose()
58+
{
59+
_disposed = true;
60+
}
61+
62+
protected class ListLogger : ILogger
63+
{
64+
private readonly object _sync = new object();
65+
66+
public ITestOutputHelper TestOutputHelper { get; set; }
67+
68+
public List<(LogLevel, EventId, string, object, Exception)> LoggedEvents { get; }
69+
= new List<(LogLevel, EventId, string, object, Exception)>();
70+
71+
public void Clear()
72+
{
73+
lock (_sync) // Guard against tests with explicit concurrency
74+
{
75+
LoggedEvents.Clear();
76+
}
77+
}
78+
79+
public void Log<TState>(
80+
LogLevel logLevel, EventId eventId, TState state, Exception exception, Func<TState, Exception, string> formatter)
81+
{
82+
lock (_sync) // Guard against tests with explicit concurrency
83+
{
84+
var message = formatter(state, exception)?.Trim();
85+
if (message != null)
86+
{
87+
TestOutputHelper?.WriteLine(message + Environment.NewLine);
88+
}
89+
90+
LoggedEvents.Add((logLevel, eventId, message, state, exception));
91+
}
92+
}
93+
94+
public bool IsEnabled(LogLevel logLevel) => true;
95+
96+
public IDisposable BeginScope(object state) => null;
97+
98+
public IDisposable BeginScope<TState>(TState state) => null;
99+
}
100+
}
101+
}

src/Middleware/SpaServices.Extensions/test/Microsoft.AspNetCore.SpaServices.Extensions.Tests.csproj

+7-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<Project Sdk="Microsoft.NET.Sdk">
1+
<Project Sdk="Microsoft.NET.Sdk">
22

33
<PropertyGroup>
44
<TargetFramework>$(DefaultNetCoreTargetFramework)</TargetFramework>
@@ -14,4 +14,10 @@
1414
<Content Include="js\**\*" />
1515
</ItemGroup>
1616

17+
<ItemGroup>
18+
<None Update="package.json">
19+
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
20+
</None>
21+
</ItemGroup>
22+
1723
</Project>

0 commit comments

Comments
 (0)