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

TypeError: Cannot read property 'on' of undefined #17

Open
ohardy opened this issue Feb 20, 2017 · 11 comments
Open

TypeError: Cannot read property 'on' of undefined #17

ohardy opened this issue Feb 20, 2017 · 11 comments

Comments

@ohardy
Copy link

ohardy commented Feb 20, 2017

I use onchange package which crash in tree-kill.
Stack trace:

<folder>/node_modules/tree-kill/index.js:78
    ps.stdout.on('data', function (data) {
             ^

TypeError: Cannot read property 'on' of undefined
    at buildProcessTree (/***/node_modules/tree-kill/index.js:78:14)
    at /***/node_modules/tree-kill/index.js:99:11
    at Array.forEach (native)
    at ChildProcess.onClose (/***/node_modules/tree-kill/index.js:94:31)
    at emitTwo (events.js:106:13)
    at ChildProcess.emit (events.js:192:7)
    at maybeClose (internal/child_process.js:890:16)
    at Socket.<anonymous> (internal/child_process.js:334:11)
    at emitOne (events.js:96:13)
    at Socket.emit (events.js:189:7)
@billiegoose
Copy link
Collaborator

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?

@ohardy
Copy link
Author

ohardy commented Feb 20, 2017

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
toggle full log entire log
pgrep -P 64
pgrep -P 65
pgrep -P 66
pgrep -P 68
pgrep -P 70
pgrep -P 72
pgrep -P 73
pgrep -P 74
pgrep -P 75
pgrep -P 81
pgrep -P 85
pgrep -P 87
pgrep -P 88
pgrep -P 96
pgrep -P 99
pgrep -P 103
pgrep -P 104
pgrep -P 107
pgrep -P 109
pgrep -P 111
pgrep -P 112
pgrep -P 113
pgrep -P 114
pgrep -P 115
pgrep -P 116
pgrep -P 118
pgrep -P 120
pgrep -P 121
pgrep -P 122
pgrep -P 123
pgrep -P 127
pgrep -P 128
pgrep -P 129
pgrep -P 130
pgrep -P 132
pgrep -P 133
pgrep -P 134
pgrep -P 135
pgrep -P 142
pgrep -P 150
pgrep -P 163
pgrep -P 167
pgrep -P 173
pgrep -P 184
pgrep -P 186
pgrep -P 202
pgrep -P 233
pgrep -P 235
pgrep -P 236
pgrep -P 237
pgrep -P 238
pgrep -P 249
pgrep -P 250
pgrep -P 253
pgrep -P 254
pgrep -P 255
pgrep -P 256
pgrep -P 265
pgrep -P 270
pgrep -P 273
pgrep -P 274
pgrep -P 275
pgrep -P 287
pgrep -P 288
pgrep -P 291
pgrep -P 307
pgrep -P 319
pgrep -P 336
pgrep -P 347
pgrep -P 354
pgrep -P 355
pgrep -P 357
pgrep -P 359
pgrep -P 360
pgrep -P 361
pgrep -P 362
pgrep -P 363
pgrep -P 368
pgrep -P 370
pgrep -P 371
pgrep -P 373
pgrep -P 374
pgrep -P 376
pgrep -P 379
pgrep -P 380
pgrep -P 381
pgrep -P 382
pgrep -P 384
pgrep -P 385
pgrep -P 386
pgrep -P 388
pgrep -P 391
pgrep -P 395
pgrep -P 397
pgrep -P 398
pgrep -P 407
pgrep -P 409
pgrep -P 410
pgrep -P 412
pgrep -P 414
pgrep -P 419
pgrep -P 420
pgrep -P 421
pgrep -P 422
pgrep -P 426
pgrep -P 428
pgrep -P 429
pgrep -P 433
pgrep -P 434
pgrep -P 435
pgrep -P 438
pgrep -P 439
pgrep -P 440
pgrep -P 442
pgrep -P 446
pgrep -P 451
pgrep -P 453
pgrep -P 467
pgrep -P 470
pgrep -P 492
pgrep -P 493
pgrep -P 495
pgrep -P 503
pgrep -P 590
pgrep -P 593
pgrep -P 617
pgrep -P 671
pgrep -P 677
pgrep -P 678
pgrep -P 683
pgrep -P 685
pgrep -P 687
pgrep -P 691
pgrep -P 692
pgrep -P 693
pgrep -P 695
pgrep -P 696
pgrep -P 703
pgrep -P 704
pgrep -P 705
pgrep -P 707
pgrep -P 709
pgrep -P 710
pgrep -P 712
pgrep -P 731
pgrep -P 735
pgrep -P 762
pgrep -P 778
pgrep -P 787
pgrep -P 788
pgrep -P 790
pgrep -P 797
pgrep -P 802
pgrep -P 829
pgrep -P 832
pgrep -P 833
pgrep -P 835
pgrep -P 841
pgrep -P 842
pgrep -P 843
pgrep -P 847
pgrep -P 859
pgrep -P 1014
pgrep -P 1016
pgrep -P 1017
pgrep -P 1026
pgrep -P 1030
pgrep -P 1048
pgrep -P 1049
pgrep -P 1051
pgrep -P 1054
pgrep -P 1061
pgrep -P 1062
pgrep -P 1063
pgrep -P 1067
pgrep -P 1068
pgrep -P 1069
pgrep -P 1070
pgrep -P 1071
pgrep -P 1072
pgrep -P 1073
pgrep -P 1075
pgrep -P 1086
pgrep -P 1094
pgrep -P 1332
pgrep -P 1484
pgrep -P 3613
pgrep -P 5566
pgrep -P 5602
pgrep -P 5687
pgrep -P 5714
pgrep -P 5736
pgrep -P 5786
pgrep -P 11385
pgrep -P 11698
pgrep -P 13169
pgrep -P 20376
pgrep -P 24392
pgrep -P 24480
pgrep -P 28842
pgrep -P 29151
pgrep -P 29152
pgrep -P 29153
pgrep -P 34919
pgrep -P 35813
pgrep -P 35893
pgrep -P 35894
pgrep -P 36709
pgrep -P 37880
pgrep -P 37990
pgrep -P 37997
pgrep -P 38177
pgrep -P 38178
pgrep -P 38919
pgrep -P 38921
pgrep -P 38925
pgrep -P 38975
pgrep -P 38976
pgrep -P 38982
pgrep -P 38983
pgrep -P 38985
pgrep -P 38986
pgrep -P 38988
pgrep -P 38989
pgrep -P 38997
pgrep -P 39008
pgrep -P 39013
pgrep -P 39014
pgrep -P 39019
pgrep -P 39069
pgrep -P 39070
pgrep -P 39482
pgrep -P 39483
pgrep -P 39659
pgrep -P 40085
pgrep -P 40086
pgrep -P 40087
pgrep -P 40088
pgrep -P 40089
pgrep -P 40090
pgrep -P 40091
pgrep -P 40443
pgrep -P 40513
pgrep -P 40514
pgrep -P 40515
pgrep -P 40518
pgrep -P 40521
pgrep -P 40522
pgrep -P 40525
pgrep -P 40526
pgrep -P 40537
pgrep -P 40637
pgrep -P 40746
pgrep -P 40960
pgrep -P 40961
pgrep -P 40962
pgrep -P 40963
pgrep -P 40964
pgrep -P 40965
pgrep -P 40983
pgrep -P 40989
pgrep -P 40990
pgrep -P 41023
pgrep -P 41104
pgrep -P 41132
pgrep -P 42175
pgrep -P 42262
pgrep -P 42263
pgrep -P 42266
pgrep -P 42267
pgrep -P 42268
pgrep -P 42418
pgrep -P 42519
pgrep -P 42520
pgrep -P 42524
pgrep -P 42543
pgrep -P 42551
pgrep -P 42647
pgrep -P 45698
pgrep -P 51820
pgrep -P 51825
pgrep -P 51854
pgrep -P 53170
pgrep -P 64193
pgrep -P 64195
pgrep -P 64208
pgrep -P 64218
pgrep -P 64347
pgrep -P 64988
pgrep -P 65139
pgrep -P 65176
pgrep -P 65231
pgrep -P 65382
pgrep -P 65383
pgrep -P 65384
pgrep -P 70158
pgrep -P 70159
pgrep -P 72167
pgrep -P 72174
pgrep -P 75442
pgrep -P 75447
pgrep -P 75577
pgrep -P 75586
pgrep -P 75588
pgrep -P 80995
pgrep -P 81249
pgrep -P 84796
pgrep -P 20887
pgrep -P 24009
pgrep -P 24024
pgrep -P 24026
pgrep -P 24634
pgrep -P 24645
pgrep -P 24665
pgrep -P 24666
pgrep -P 24668
pgrep -P 24760
pgrep -P 24774
pgrep -P 27036
pgrep -P 27047
pgrep -P 27048
pgrep -P 27049
pgrep -P 27050
pgrep -P 27051
pgrep -P 27052
pgrep -P 27053
pgrep -P 27054
pgrep -P 27055
pgrep -P 27056
pgrep -P 27057
pgrep -P 27061
pgrep -P 52882
pgrep -P 52884
pgrep -P 52898
pgrep -P 64385
pgrep -P 64516
pgrep -P 68527
pgrep -P 68575
pgrep -P 68576
pgrep -P 68577
pgrep -P 68578
pgrep -P 69022
pgrep -P 69500
pgrep -P 69504
pgrep -P 69598
pgrep -P 69628
pgrep -P 69665
pgrep -P 69743
pgrep -P 69903
pgrep -P 69939
pgrep -P 70018
pgrep -P 70074
pgrep -P 70119
pgrep -P 70181
pgrep -P 70268
pgrep -P 70292
pgrep -P 70296
pgrep -P 70309
pgrep -P 70336
pgrep -P 70337
pgrep -P 70351
pgrep -P 70352
pgrep -P 70609
pgrep -P 70614
pgrep -P 70617
pgrep -P 57986
pgrep -P 29079
pgrep -P 29169
pgrep -P 29170
pgrep -P 29181
pgrep -P 29183
pgrep -P 29185
pgrep -P 29186
pgrep -P 29187
pgrep -P 29188
pgrep -P 29237
pgrep -P 29250
pgrep -P 29253
pgrep -P 29594
pgrep -P 80275
pgrep -P 80352
pgrep -P 80357
pgrep -P 14550
pgrep -P 23608
pgrep -P 25083
pgrep -P 28381
pgrep -P 28382
pgrep -P 28383
pgrep -P 28384
pgrep -P 28385
pgrep -P 28386
pgrep -P 28387
pgrep -P 28388
pgrep -P 28389
pgrep -P 28390
pgrep -P 29216
pgrep -P 33166
pgrep -P 33214
pgrep -P 33708
pgrep -P 33713
pgrep -P 33716
pgrep -P 33743
pgrep -P 33744
pgrep -P 33745
pgrep -P 33746
pgrep -P 33747
pgrep -P 33748
pgrep -P 33749
pgrep -P 33750
pgrep -P 33751
pgrep -P 33752
pgrep -P 34658
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

@Marak
Copy link

Marak commented Feb 20, 2017

What happens if you switch back to Node version 6?

@ohardy
Copy link
Author

ohardy commented Feb 20, 2017

Same bug with v6.9.5

@billiegoose
Copy link
Collaborator

Huh. This is on Linux?

@ohardy
Copy link
Author

ohardy commented Feb 21, 2017

Mac OS 10.12.3

@ohardy
Copy link
Author

ohardy commented Feb 21, 2017

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 ?

@ickma
Copy link

ickma commented Jul 18, 2017

I met the same issue as u @ohardy , your solution saved me!

@billiegoose
Copy link
Collaborator

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?

@DPiaz
Copy link

DPiaz commented Dec 3, 2018

In my case, i removed the option {stdio: 'inherit'} from the call spawn(command, args, options) and works.

@mogutan88
Copy link

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]);

hyangah added a commit to hyangah/node-tree-kill that referenced this issue Jun 17, 2020
Use the default pgrep, available since os x mountain lion.
proctools' pgrep does not implement `-P` correctly, returns
unrelated processes, breaks tree-kill's assumption, and may
cause a large number of pgrep processes.

Reported in pkrumins#17 (comment)

Update golang/vscode-go#90 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants