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

jungle: Using ps -p will check correctly whether $PID exists #1545

Conversation

hoshinotsuyoshi
Copy link
Contributor

I think that It is appropriate to use ps -p in that place.

I think that under certain conditions, existing code [ "`ps -A -o pid= | grep -c $PID`" -eq 0 ] may behave incorrectly.

For example, if the content of $PID happens to be a short string(such as 199), and another process includes the string(process such as 2199), then unfortunately grep -c will return
positive number even if 199 is not running.

It is appropriate to use `ps -p` in that place.

Under certain conditions, existing code ``[ "`ps -A -o pid=
| grep -c $PID`" -eq 0 ]`` may behave incorrectly.

For example, if the content of $PID happens to be a short string
(such as `199`), and another process includes the string
(process such as `2199`), then unfortunately `grep -c` will return
positive number even if `199` is not running.
@nateberkopec
Copy link
Member

Paging previous contributors @vipulnsward @michaelsauter @snow @kwilczynski

@michaelsauter
Copy link
Contributor

Thanks for paging! We never had the issue described here, but I'd agree with @hoshinotsuyoshi that this needs fixing.

Copy link
Contributor

@kwilczynski kwilczynski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice improvement.

@kwilczynski
Copy link
Contributor

Hi @hoshinotsuyoshi, thank you fixing this!

A side note - this whole script is almost crying out loud to be re-written and maintained, somewhat.

@nateberkopec nateberkopec merged commit 9aea96e into puma:master Mar 28, 2018
@hoshinotsuyoshi
Copy link
Contributor Author

Thank you!

@hoshinotsuyoshi hoshinotsuyoshi deleted the jungle_check_correctly_whether_the_pid_exists branch March 28, 2018 16:03
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 this pull request may close these issues.

None yet

4 participants