Fix all the ConditionalProxyMiddleware errors that happened if you ctrl+c on a "dotnet run" (not "dotnet watch run") since beta-000002.

This commit is contained in:
SteveSandersonMS
2016-12-13 11:32:32 +00:00
parent 6545e11bf2
commit 1c4682e50d
3 changed files with 42 additions and 5 deletions

View File

@@ -125,7 +125,7 @@
// Signal to the NodeServices base class that we're ready to accept invocations // Signal to the NodeServices base class that we're ready to accept invocations
console.log('[Microsoft.AspNetCore.NodeServices:Listening]'); console.log('[Microsoft.AspNetCore.NodeServices:Listening]');
}); });
ExitWhenParentExits_1.exitWhenParentExits(parseInt(parsedArgs.parentPid)); ExitWhenParentExits_1.exitWhenParentExits(parseInt(parsedArgs.parentPid), /* ignoreSigint */ true);
function readRequestBodyAsJson(request, callback) { function readRequestBodyAsJson(request, callback) {
var requestBodyAsString = ''; var requestBodyAsString = '';
request.on('data', function (chunk) { requestBodyAsString += chunk; }); request.on('data', function (chunk) { requestBodyAsString += chunk; });
@@ -255,7 +255,7 @@
*/ */
"use strict"; "use strict";
var pollIntervalMs = 1000; var pollIntervalMs = 1000;
function exitWhenParentExits(parentPid) { function exitWhenParentExits(parentPid, ignoreSigint) {
setInterval(function () { setInterval(function () {
if (!processExists(parentPid)) { if (!processExists(parentPid)) {
// Can't log anything at this point, because out stdout was connected to the parent, // Can't log anything at this point, because out stdout was connected to the parent,
@@ -263,6 +263,24 @@
process.exit(); process.exit();
} }
}, pollIntervalMs); }, pollIntervalMs);
if (ignoreSigint) {
// Pressing ctrl+c in the terminal sends a SIGINT to all processes in the foreground process tree.
// By default, the Node process would then exit before the .NET process, because ASP.NET implements
// a delayed shutdown to allow ongoing requests to complete.
//
// This is problematic, because if Node exits first, the CopyToAsync code in ConditionalProxyMiddleware
// will experience a read fault, and logs a huge load of errors. Fortunately, since the Node process is
// already set up to shut itself down if it detects the .NET process is terminated, all we have to do is
// ignore the SIGINT. The Node process will then terminate automatically after the .NET process does.
//
// A better solution would be to have WebpackDevMiddleware listen for SIGINT and gracefully close any
// ongoing EventSource connections before letting the Node process exit, independently of the .NET
// process exiting. However, doing this well in general is very nontrivial (see all the discussion at
// https://github.com/nodejs/node/issues/2642).
process.on('SIGINT', function () {
console.log('Received SIGINT. Waiting for .NET process to exit...');
});
}
} }
exports.exitWhenParentExits = exitWhenParentExits; exports.exitWhenParentExits = exitWhenParentExits;
function processExists(pid) { function processExists(pid) {

View File

@@ -76,7 +76,7 @@ server.listen(requestedPortOrZero, 'localhost', function () {
console.log('[Microsoft.AspNetCore.NodeServices:Listening]'); console.log('[Microsoft.AspNetCore.NodeServices:Listening]');
}); });
exitWhenParentExits(parseInt(parsedArgs.parentPid)); exitWhenParentExits(parseInt(parsedArgs.parentPid), /* ignoreSigint */ true);
function readRequestBodyAsJson(request, callback) { function readRequestBodyAsJson(request, callback) {
let requestBodyAsString = ''; let requestBodyAsString = '';

View File

@@ -34,7 +34,7 @@ to check whether the parent PID is still running. So that's what we do here.
const pollIntervalMs = 1000; const pollIntervalMs = 1000;
export function exitWhenParentExits(parentPid: number) { export function exitWhenParentExits(parentPid: number, ignoreSigint: boolean) {
setInterval(() => { setInterval(() => {
if (!processExists(parentPid)) { if (!processExists(parentPid)) {
// Can't log anything at this point, because out stdout was connected to the parent, // Can't log anything at this point, because out stdout was connected to the parent,
@@ -42,6 +42,25 @@ export function exitWhenParentExits(parentPid: number) {
process.exit(); process.exit();
} }
}, pollIntervalMs); }, pollIntervalMs);
if (ignoreSigint) {
// Pressing ctrl+c in the terminal sends a SIGINT to all processes in the foreground process tree.
// By default, the Node process would then exit before the .NET process, because ASP.NET implements
// a delayed shutdown to allow ongoing requests to complete.
//
// This is problematic, because if Node exits first, the CopyToAsync code in ConditionalProxyMiddleware
// will experience a read fault, and logs a huge load of errors. Fortunately, since the Node process is
// already set up to shut itself down if it detects the .NET process is terminated, all we have to do is
// ignore the SIGINT. The Node process will then terminate automatically after the .NET process does.
//
// A better solution would be to have WebpackDevMiddleware listen for SIGINT and gracefully close any
// ongoing EventSource connections before letting the Node process exit, independently of the .NET
// process exiting. However, doing this well in general is very nontrivial (see all the discussion at
// https://github.com/nodejs/node/issues/2642).
process.on('SIGINT', () => {
console.log('Received SIGINT. Waiting for .NET process to exit...');
});
}
} }
function processExists(pid: number) { function processExists(pid: number) {
@@ -56,7 +75,7 @@ function processExists(pid: number) {
if (ex.code === 'EPERM') { if (ex.code === 'EPERM') {
throw new Error(`Attempted to check whether process ${pid} was running, but got a permissions error.`); throw new Error(`Attempted to check whether process ${pid} was running, but got a permissions error.`);
} }
return false; return false;
} }
} }