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

pm2 restart does not honor args from process start #3085

Closed
ninja- opened this issue Aug 10, 2017 · 9 comments
Closed

pm2 restart does not honor args from process start #3085

ninja- opened this issue Aug 10, 2017 · 9 comments

Comments

@ninja-
Copy link

ninja- commented Aug 10, 2017

Your issue may already be reported!
Please search on the issue tracker before creating one.

What's going wrong?

So, I start my app either via
$ pm2 start --no-treekill myappbin
$ pm2 start myapp.json
with json having {treekill: false} inside.

I expect $ pm2 restart myapp to honor no-treekill the way it was declared at start.
But it's ignored.

So every pm2 restart brings a total disaster to my app killing all the detached childs.
What works is
$ pm2 restart --no-treekill myapp

How could we reproduce this issue?

  1. Create a node app that spawns a running-forever child with detached=true
  2. Start the app with --no-treekill
  3. Verify that pm2 restart kills the app along with the child
  4. Verify that pm2 restart --no-treekill does not

Supporting information

Please run the following command (available on PM2 >= 2.6)

$ pm2 report
pm2d version         : 2.6.1
node version         : 8.2.1
node path            : /usr/bin/pm2
argv                 : /usr/bin/nodejs,/usr/lib/node_modules/pm2/lib/Daemon.js
argv0                : node
user                 : root
uid                  : 0
gid                  : 0
uptime               : 81min
@ninja-
Copy link
Author

ninja- commented Aug 11, 2017

Here's a ready unit test showing the bug ^^

# setup test files
cat <<EOF > test2.js
var child_process = require('child_process');

setInterval(() => {}, 1000)

EOF

cat <<EOF > test1.js
var child_process = require('child_process');
child_process.spawn('sh', ['-c', 'echo hello; sleep 100'], {detached:true})

setInterval(() => {}, 1000)
console.log("spawned!!");

EOF



# First test with --no-treekill
echo Testing with --no-treekill

kill -9 `pgrep -f hello`&>/dev/null; 
pm2 delete test.js&>/dev/null; 
cp test1.js test.js; 
pm2 start --no-treekill test.js&>/dev/null; 
sleep 3
ps aux | grep hello | grep -v grep || echo "Expected process not found"; 
cp test2.js test.js; 
pm2 restart --no-treekill test.js&>/dev/null; 
sleep 5; 
ps aux | grep hello | grep -v grep && echo "TEST OK" || echo "TEST FAIL"

echo Testing WITHOUT --no-treekill

kill -9 `pgrep -f hello`&>/dev/null; 
pm2 delete test.js&>/dev/null; 
cp test1.js test.js; 
pm2 start --no-treekill test.js&>/dev/null; 
sleep 3
ps aux | grep hello | grep -v grep || echo "Expected process not found"; 
cp test2.js test.js; 
pm2 restart test.js&>/dev/null; 
sleep 5; 
ps aux | grep hello | grep -v grep &&  echo"TEST OK" || echo "TEST FAIL"
root@x ~ # bash bug.sh
Testing with --no-treekill
root     26668  0.0  0.0   4508   784 ?        Ss   02:04   0:00 sh -c echo hello; sleep 100
root     26668  0.0  0.0   4508   784 ?        Ss   02:04   0:00 sh -c echo hello; sleep 100
TEST OK
Testing WITHOUT --no-treekill
root     26732  0.0  0.0   4508   796 ?        Ss   02:04   0:00 sh -c echo hello; sleep 100
TEST FAIL

@ninja-
Copy link
Author

ninja- commented Aug 11, 2017

I can see some code that's supposed to restore the 'env' during restart/reload so I am not sure why it doesnt work i.e.

God.restartProcessId = function(opts, cb) {

https://github.com/Unitech/pm2/blob/master/lib/God/Reload.js#L117

@Unitech Unitech added the T: Bug label Aug 11, 2017
@Unitech
Copy link
Owner

Unitech commented Aug 11, 2017

thanks for the extensive report, bug confirmed looking to it

@Unitech
Copy link
Owner

Unitech commented Aug 11, 2017

fixed, you can try it now:

$ npm install Unitech/pm2#development -g
$ pm2 update
$ pm2 start app.js --no-treekill
$ pm2 restart app

@ninja-
Copy link
Author

ninja- commented Aug 12, 2017

Thanks!!
In the meantime I modified my app to spawn child processes in full background so they get parent id 1 and deployed pm2 anyway.
This took me a while of refactoring because the child was signaling it's readyness to parent via stdin.close() :/ so that needed to be changed to something else.

Happy to see it fixed :)

@ninja-
Copy link
Author

ninja- commented Sep 6, 2017

@Unitech when can you release? :)

@Unitech
Copy link
Owner

Unitech commented Sep 15, 2017

Fixed on recently published pm2 (2.7.0):

$ npm install pm2@latest -g
$ pm2 update

Let me know if there is any issue

@Unitech Unitech closed this as completed Sep 15, 2017
@Flicksie
Copy link

I'm still observing this issue in v3.2.2
args from ecosystem file aren't kept after restarting

@ninja-
Copy link
Author

ninja- commented Nov 29, 2018

@Flicksie I wonder if you can reproduce with my unit test? But maybe ecosystem files are working differently and need a seperate fix...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants