Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

behavior unhelpful for process kill default signal SIGTERM #2726

Closed
ScottFreeCode opened this issue Mar 4, 2017 · 7 comments
Closed

behavior unhelpful for process kill default signal SIGTERM #2726

ScottFreeCode opened this issue Mar 4, 2017 · 7 comments
Labels
semver-major implementation requires increase of "major" version number; "breaking changes" stale this has been inactive for a while...

Comments

@ScottFreeCode
Copy link
Contributor

ScottFreeCode commented Mar 4, 2017

So, with Mocha's programmatic API basically broken for running multiple times, I have a process that is spawning the commandline Mocha to run multiple times on demand. Then there's also the ability to stop one of these instances on demand. Naturally, child-process's .kill method makes that easy, right?

Today I learned:

  1. .kill() defaults to .kill('SIGTERM')
  2. SIGTERM kills the mocha instance without affecting the _mocha instance that is actually running the tests. [SEE UPDATE...] Not only that, but it seemed like it exited with code 0 reporting no signal stopped it (but I'd have to double-check that, as I was experimenting trying to see if I could find an easy fix, so it's possible my experiments were causing that bit). [UPDATE: Ran a quick double-check, and it seems to exit with no exit code and signal SIGTERM; exiting with code 0 and no signal reported must have been what happened when I tried to get Mocha to handle SIGTERM (see below).]

(This came up on a Mac using Node 6. I could fairly easily put together an integration test to demonstrate the issue if we wanted to see what all environments it affects, I think...)

Obviously, this is easy to handle outside of Mocha: use .kill('SIGINT') instead of .kill(). But it was such counterintuitive and unhelpful default behavior, and considering that the alternative to spawn (the programmatic API) is known to be broken in some cases, that I'm somewhat surprised we don't appear to have any existing issues about it.

Anyway, does anyone know what it would take to fix this? I tried adding a SIGTERM handler and a process.on('exit' ... handler to the mocha file that runs _mocha, but couldn't really get clean results like for SIGINT -- but maybe I was doing something wrong. I just figure that it's probably not worth it if it's difficult, but would be very much worth it if it happens to be easy. I know we're also considering switching to a library that would handle the Node flag spawning mechanism for us (#2517); I would be interested in seeing what happens if we use that and send .kill() to a spawned Mocha process.

@ORESoftware
Copy link

ORESoftware commented Mar 4, 2017

try SIGKILL? child.kill('SIGKILL')

processes can recover from SIGTERM and SIGINT, but they cannot recover from SIGKILL tmk

@ScottFreeCode
Copy link
Contributor Author

SIGINT works fine too since Mocha is specifically handling that; the real issue I see here is not lack of a signal that will work, but the fact that the default signal doesn't work.

@drazisil drazisil added the type: question support question label Mar 30, 2017
@stale stale bot added the stale this has been inactive for a while... label Jul 29, 2017
@stale
Copy link

stale bot commented Jul 29, 2017

I am a bot that watches issues for inactivity.
This issue hasn't had any recent activity, and I'm labeling it stale. In 14 days, if there are no further comments or activity, I will close this issue.
Thanks for contributing to Mocha!

@ScottFreeCode ScottFreeCode removed the type: question support question label Aug 1, 2017
@stale stale bot removed the stale this has been inactive for a while... label Aug 1, 2017
@ScottFreeCode ScottFreeCode changed the title SIGTERM behavior behavior unhelpful for process kill default signal SIGTERM Aug 1, 2017
@boneskull
Copy link
Contributor

I don't actually understand what we're trying to fix. we want to subprocess.kill() like subprocess.kill('SIGINT') works now?

@boneskull boneskull added the semver-major implementation requires increase of "major" version number; "breaking changes" label Oct 6, 2017
@ScottFreeCode
Copy link
Contributor Author

Something like that. Last I checked, if I'm spawning bin/mocha and not bin/_mocha directly, if I subprocess.kill() it I end up with bin/_mocha left running on its own -- that does not seem right in any case.

@boneskull
Copy link
Contributor

If there's any way we can just avoid spawning in the first place, maybe this problem would magically disappear.

@github-actions
Copy link
Contributor

This issue hasn't had any recent activity, and I'm labeling it stale. Remove the label or comment or this issue will be closed in 14 days. Thanks for contributing to Mocha!

@github-actions github-actions bot added the stale this has been inactive for a while... label Mar 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-major implementation requires increase of "major" version number; "breaking changes" stale this has been inactive for a while...
Projects
None yet
Development

No branches or pull requests

4 participants