Open ohardy opened 7 years ago
If I understand correctly, you are using the onchange package and it is using tree-kill? I don't yet have enough information to figure out what went wrong. The error Cannot read property 'on' of undefined
would imply that ps.stdout is undefined, but I have never seen that happen before. Could you add some more details like what: operating system, Node version?
I hope it will help you
Node version: 7.5.0
I added some debug and added a test to not call ps.stdout.on if ps.stdout is null and that's it:
onchange: change to src/server.js
onchange: killing command 41413 and restarting
kill(41413, 'SIGTERM');
pgrep -P 41413
pgrep -P 1
pgrep -P 61
pgrep -P 62
pgrep -P 34661
pgrep -P 34668
pgrep -P 35612
Null stdout for : spawnChildProcessesList(35612)
pgrep -P 35636
Null stdout for : spawnChildProcessesList(35636)
pgrep -P 35641
Null stdout for : spawnChildProcessesList(35641)
pgrep -P 35669
Null stdout for : spawnChildProcessesList(35669)
pgrep -P 35714
Null stdout for : spawnChildProcessesList(35714)
pgrep -P 36565
Null stdout for : spawnChildProcessesList(36565)
pgrep -P 36566
Null stdout for : spawnChildProcessesList(36566)
pgrep -P 36567
Null stdout for : spawnChildProcessesList(36567)
pgrep -P 36731
Null stdout for : spawnChildProcessesList(36731)
pgrep -P 36732
Null stdout for : spawnChildProcessesList(36732)
pgrep -P 36733
Null stdout for : spawnChildProcessesList(36733)
pgrep -P 36734
Null stdout for : spawnChildProcessesList(36734)
pgrep -P 36738
Null stdout for : spawnChildProcessesList(36738)
pgrep -P 36739
Null stdout for : spawnChildProcessesList(36739)
pgrep -P 36740
Null stdout for : spawnChildProcessesList(36740)
pgrep -P 36741
Null stdout for : spawnChildProcessesList(36741)
pgrep -P 37001
Null stdout for : spawnChildProcessesList(37001)
pgrep -P 37002
Null stdout for : spawnChildProcessesList(37002)
pgrep -P 37004
Null stdout for : spawnChildProcessesList(37004)
pgrep -P 37006
Null stdout for : spawnChildProcessesList(37006)
pgrep -P 37026
Null stdout for : spawnChildProcessesList(37026)
pgrep -P 40026
Null stdout for : spawnChildProcessesList(40026)
pgrep -P 41303
Null stdout for : spawnChildProcessesList(41303)
pgrep -P 41412
Null stdout for : spawnChildProcessesList(41412)
pgrep -P 41413
Null stdout for : spawnChildProcessesList(41413)
pgrep -P 41424
Null stdout for : spawnChildProcessesList(41424)
events.js:161
throw er; // Unhandled 'error' event
^
Error: spawn pgrep EAGAIN
at exports._errnoException (util.js:1023:11)
at Process.ChildProcess._handle.onexit (internal/child_process.js:193:32)
at onErrorNT (internal/child_process.js:359:16)
at _combinedTickCallback (internal/process/next_tick.js:74:11)
at process._tickCallback (internal/process/next_tick.js:98:9)
killAll(tree, signal, callback);
is never called
What happens if you switch back to Node version 6?
Same bug with v6.9.5
Huh. This is on Linux?
Mac OS 10.12.3
So i try to fix the issue. I thinks it's a too many process problem for the buildProcessTree part, if i add a setTimeout for each spawn call, no more stdout null bug.
But the other problem is :
kill : 75531 SIGTERM
onClose pid : 75531 code: 0
Object.keys(pidsToProcess).length : 0
So in onClose, i change:
if (code != 0) {
to:
if (code == 0) {
And everything works.
Why do you check the return code of the process ?
I met the same issue as u @ohardy , your solution saved me!
Why do you check the return code of the process ?
The return code of pgrep -P
tells us whether the process has children that also need to be killed. From the manpage:
EXIT STATUS
0 One or more processes matched the criteria.
1 No processes matched.
2 Syntax error in the command line.
3 Fatal error: out of memory etc.
By changing the != to == you're bailing out of the recursion prematurely. However the more I study this code, the more I'm not sure it is right. I think the forEach loop and the callback cb
invocation creates a race condition. I think this code should probably be refactored to make the control flow simpler, but I don't have the time right now. Long story short @ohardy and @ickma, somehow the process you're trying to kill creates so many children that tree-kill runs into a race condition. The fix suggested will probably keep tree-kill from crashing and never calling killAll, but it might allow children/grandchildren of the process to escape it's killer grasp and become zombies.
If anyone thinks of a better way to do the recursive process killing I'm all ears. I don't have spare cycles to investigate. (And don't have a Mac.) @ickma is yours Mac OSX too?
In my case, i removed the option {stdio: 'inherit'} from the call spawn(command, args, options) and works.
In my case, I have proctools installed on my macOS at, /usr/local/bin. I confirmed that the output of '/usr/local/bin/pgrep -P' and that of '/usr/bin/pgrep -P' are different. So, uninstalling the proctools, changing the PATH, or even changing the tree-kill/index.js code like below, solved the problem.
case 'darwin':
buildProcessTree(pid, tree, pidsToProcess, function (parentPid) {
return spawn('/usr/bin/pgrep', ['-P', parentPid]);
I use onchange package which crash in tree-kill. Stack trace: