From 1c4682e50d30b8d50fcde3578cad84767bae0a2a Mon Sep 17 00:00:00 2001 From: SteveSandersonMS Date: Tue, 13 Dec 2016 11:32:32 +0000 Subject: [PATCH] Fix all the ConditionalProxyMiddleware errors that happened if you ctrl+c on a "dotnet run" (not "dotnet watch run") since beta-000002. --- .../Content/Node/entrypoint-http.js | 22 ++++++++++++++++-- .../TypeScript/HttpNodeInstanceEntryPoint.ts | 2 +- .../TypeScript/Util/ExitWhenParentExits.ts | 23 +++++++++++++++++-- 3 files changed, 42 insertions(+), 5 deletions(-) diff --git a/src/Microsoft.AspNetCore.NodeServices/Content/Node/entrypoint-http.js b/src/Microsoft.AspNetCore.NodeServices/Content/Node/entrypoint-http.js index b085a36..a80d8a0 100644 --- a/src/Microsoft.AspNetCore.NodeServices/Content/Node/entrypoint-http.js +++ b/src/Microsoft.AspNetCore.NodeServices/Content/Node/entrypoint-http.js @@ -125,7 +125,7 @@ // Signal to the NodeServices base class that we're ready to accept invocations console.log('[Microsoft.AspNetCore.NodeServices:Listening]'); }); - ExitWhenParentExits_1.exitWhenParentExits(parseInt(parsedArgs.parentPid)); + ExitWhenParentExits_1.exitWhenParentExits(parseInt(parsedArgs.parentPid), /* ignoreSigint */ true); function readRequestBodyAsJson(request, callback) { var requestBodyAsString = ''; request.on('data', function (chunk) { requestBodyAsString += chunk; }); @@ -255,7 +255,7 @@ */ "use strict"; var pollIntervalMs = 1000; - function exitWhenParentExits(parentPid) { + function exitWhenParentExits(parentPid, ignoreSigint) { setInterval(function () { if (!processExists(parentPid)) { // Can't log anything at this point, because out stdout was connected to the parent, @@ -263,6 +263,24 @@ process.exit(); } }, 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; function processExists(pid) { diff --git a/src/Microsoft.AspNetCore.NodeServices/TypeScript/HttpNodeInstanceEntryPoint.ts b/src/Microsoft.AspNetCore.NodeServices/TypeScript/HttpNodeInstanceEntryPoint.ts index eb7fc28..9b51bf1 100644 --- a/src/Microsoft.AspNetCore.NodeServices/TypeScript/HttpNodeInstanceEntryPoint.ts +++ b/src/Microsoft.AspNetCore.NodeServices/TypeScript/HttpNodeInstanceEntryPoint.ts @@ -76,7 +76,7 @@ server.listen(requestedPortOrZero, 'localhost', function () { console.log('[Microsoft.AspNetCore.NodeServices:Listening]'); }); -exitWhenParentExits(parseInt(parsedArgs.parentPid)); +exitWhenParentExits(parseInt(parsedArgs.parentPid), /* ignoreSigint */ true); function readRequestBodyAsJson(request, callback) { let requestBodyAsString = ''; diff --git a/src/Microsoft.AspNetCore.NodeServices/TypeScript/Util/ExitWhenParentExits.ts b/src/Microsoft.AspNetCore.NodeServices/TypeScript/Util/ExitWhenParentExits.ts index 672e40b..43c865f 100644 --- a/src/Microsoft.AspNetCore.NodeServices/TypeScript/Util/ExitWhenParentExits.ts +++ b/src/Microsoft.AspNetCore.NodeServices/TypeScript/Util/ExitWhenParentExits.ts @@ -34,7 +34,7 @@ to check whether the parent PID is still running. So that's what we do here. const pollIntervalMs = 1000; -export function exitWhenParentExits(parentPid: number) { +export function exitWhenParentExits(parentPid: number, ignoreSigint: boolean) { setInterval(() => { if (!processExists(parentPid)) { // 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(); } }, 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) { @@ -56,7 +75,7 @@ function processExists(pid: number) { if (ex.code === 'EPERM') { throw new Error(`Attempted to check whether process ${pid} was running, but got a permissions error.`); } - + return false; } }