Check that the pidfile is not already present before overriding it #371

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
4 participants
Contributor

byroot commented Mar 21, 2012

It fix the issue reported here: rails/rails#5534

At first I tried to write some test, but because of the exit(1) it's not that easy. If someone have a idea of how to write some test about that...

Thanks for taking care of this. Hopefully it gets accepted cause it's a pain to loose the pid.

Owner

raggi commented May 4, 2012

I would like to do several things with this before merge:

  • Check if the PID is running, if it is, then abort
  • If the PID is not running, then boot and replace the pidfile.
  • Add tests.
Contributor

byroot commented May 5, 2012

Sure, I'll try to find a moment to do that this week end.

Contributor

byroot commented May 5, 2012

Here we are.

The code now check if the process is still running / dead / or owned by another user.
With associated tests.

I also tried to test the check_pid!method, but I don't know how to test exit and STDERR.puts calls.

This pull request passes (merged 3b9410e into 6141192).

@raggi raggi added a commit that referenced this pull request May 13, 2012

@byroot @raggi byroot + raggi Check if the PID in pidfile is still running #371
Then abort or remove the pidfile
b4d7b95
Owner

raggi commented May 13, 2012

Merged, thank you!

raggi closed this May 13, 2012

Owner

raggi commented May 13, 2012

FTR, I made a minor change:

edc8b92

@raggi raggi added a commit that referenced this pull request Jan 4, 2013

@byroot @raggi byroot + raggi Check if the PID in pidfile is still running #371
Then abort or remove the pidfile
88ed888

@raggi raggi added a commit that referenced this pull request Jan 4, 2013

@byroot @raggi byroot + raggi Check if the PID in pidfile is still running #371
Then abort or remove the pidfile
03a7303
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment