Skip to content

[SPA] Ensure spawned NPM processes are terminated when the dotnet process is ungracefully terminated#35591

Merged
javiercn merged 4 commits intomainfrom
javiercn/stop-npm-processes-mac-os
Aug 24, 2021
Merged

[SPA] Ensure spawned NPM processes are terminated when the dotnet process is ungracefully terminated#35591
javiercn merged 4 commits intomainfrom
javiercn/stop-npm-processes-mac-os

Conversation

@javiercn
Copy link
Member

Adds a script to terminate the spawned NPM processes automatically when the host dotnet process is ungracefully terminated in the same way its done on windows.

The script watches every second that the parent dotnet process is still running and when it detects it is not longer doing so, it lists all kills all the spawned node processes.

@javiercn javiercn requested a review from a team August 23, 2021 11:49
private void LaunchStopScriptMacOS()
{
var fileName = Guid.NewGuid().ToString("N") + ".sh";
var scriptPath = Path.Combine(AppContext.BaseDirectory, fileName);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why write it into the app directory? Could it go into a temp directory instead?

Reason: if for some reason this fails or takes a nontrivial amount of time to run, it will show up as a file in source control. Or if there is some file watcher (e.g., hot reload), it's going to trigger some action.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AppContext.BaseDirectory points to the bin\Debug\net6.0 folder

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, sounds fine then. I’m not certain how spaces in the path would be handled on macOS but assuming the script will still get executed in this case, all good!

Copy link
Member

@SteveSandersonMS SteveSandersonMS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There were a few things I'm unsure about and have posted comments, but the overall approach looks great and I'm sure you'll make good choices about how/whether to address the comments.

@javiercn javiercn force-pushed the javiercn/stop-npm-processes-mac-os branch from ccaa25e to 9738d37 Compare August 24, 2021 11:12
@javiercn javiercn enabled auto-merge (squash) August 24, 2021 11:16
@javiercn javiercn merged commit 22b077e into main Aug 24, 2021
@javiercn javiercn deleted the javiercn/stop-npm-processes-mac-os branch August 24, 2021 17:25
@ghost ghost added this to the 7.0-preview1 milestone Aug 24, 2021
@javiercn
Copy link
Member Author

/backport to release/6.0-rc1

@github-actions
Copy link
Contributor

Started backporting to release/6.0-rc1: https://github.com/dotnet/aspnetcore/actions/runs/1166604471

@amcasey amcasey added area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares and removed area-runtime labels Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants