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

Stopping by index terminates multiple processes (0.14.1) #699

Closed
kegsay opened this issue Apr 14, 2015 · 11 comments · Fixed by #756
Closed

Stopping by index terminates multiple processes (0.14.1) #699

kegsay opened this issue Apr 14, 2015 · 11 comments · Fixed by #756

Comments

@kegsay
Copy link

kegsay commented Apr 14, 2015

$ forever --version
v0.14.1

$ forever list
info:    Forever processes running
data:        uid  command         script forever pid   id logfile                       uptime       
data:    [0] yinz /usr/bin/nodejs app.js 13778   13780    /home/kegsay/.forever/yinz.log 0:0:4:3.199  
data:    [1] kABa /usr/bin/nodejs app.js 13801   13803    /home/kegsay/.forever/kABa.log 0:0:3:45.139

$ forever stop 0
info:    Forever stopped process:
    uid  command         script forever pid   id logfile                       uptime       
[0] yinz /usr/bin/nodejs app.js 13778   13780    /home/kegsay/.forever/yinz.log 0:0:4:12.408 
[1] kABa /usr/bin/nodejs app.js 13801   13803    /home/kegsay/.forever/kABa.log 0:0:3:54.348

$ forever list
info:    No forever processes running

Expected: That the first process [0] would be killed by forever stop 0.
Actual: Both processes were terminated.

This may be related to: #481
Running node v0.10.25

@kegsay
Copy link
Author

kegsay commented Apr 14, 2015

This doesn't appear to be based on having the same name such as app.js. This only seems to happen if you try to forever stop 0. In other words, forever stop 1 works correctly (stopping the 2nd process marked [1] in the list).

@kegsay
Copy link
Author

kegsay commented Apr 14, 2015

I suspect the bug is at https://github.com/foreverjs/forever/blob/509eaf24deaf1ea8515298d5fcf16708715e658c/lib/forever.js#L692 which is doing an == check when I think it really needs ===. The variables when reproducing this bug are:

> p.id
false
> id
0

@rubaboo
Copy link

rubaboo commented Apr 15, 2015

Related to #659?

@danzaner
Copy link

danzaner commented May 8, 2015

@kegsay I can confirm that your fix solves this problem for me. I consistently had all processes stopped when issuing a 'forever stop 0'. Modifying forever.findById() from 'return p.id == id;' to 'return p.id === id;' results in 'forever stop 0' killing only the process at index 0.

Many thanks!

@devinivy
Copy link

We're also experiencing this issue.

@indexzero
Copy link
Member

Fixed in 0.14.2

@rubaboo
Copy link

rubaboo commented Jul 5, 2015

I have 0.14.2, and forever stop 0 still stops everything. Is restart fixed but not stop (see #659)???

@devinivy
Copy link

devinivy commented Jul 5, 2015

Sounds like this simply needs a failing test. Does anyone mind writing one?

@ultimate-tester
Copy link

ultimate-tester commented May 17, 2016

I can confirm that the latest NPM version does not contain the fix yet! Code of the affected file does not match the file in master branch which was updated with commit faa5d3a

@playeren
Copy link

playeren commented Dec 4, 2016

This issue still exists in v0.15.1 where #756 is fixed.

root@freja:/srv/freshHQ/main# forever --version
v0.15.1

root@freja:/srv/freshHQ/main# forever restart 0
info:    Forever restarted process(es):
data:        uid  command                                  script    forever pid   id logfile                 uptime
data:    [0] ZX72 /root/.nvm/versions/node/v6.9.1/bin/node websrv.js 28449   28455    /root/.forever/ZX72.log 0:0:0:15.776
data:    [1] OoP3 /root/.nvm/versions/node/v6.9.1/bin/node btcapi.js 28475   28481    /root/.forever/OoP3.log 0:0:0:8.245

@kegsay
Copy link
Author

kegsay commented Dec 4, 2016

I don't think #756 landed in 0.15.1:

  • 31 Jul 2015 : Bumped to 0.15.1 : 6c157fa
  • 29 Aug 2015 : Fix landed a957354
  • 26 May 2016 : Bumped to 0.15.2 66e8c0b

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

Successfully merging a pull request may close this issue.

7 participants