pexpect / ptyprocess

Run a subprocess in a pseudo terminal
https://ptyprocess.readthedocs.io/en/latest/
Other
222 stars 71 forks source link

isalive(): check process status for non-child process #37

Open masteret opened 7 years ago

masteret commented 7 years ago

The original implementation of isalive uses os.waitpid to check if the ptyprocess is alive os.waitpid can only check the process status if that process is a child of the current process

If the pexpect object is passed from process A to process B process B can access the pexpect object but it cannot use it because os.waitpid can't find the ptyprocess under the child list of process B it will throw the no such process error while the process actually exists and is still running

To solve this It can use os.kill to send signal 0 to check if process is alive which achieve the same functionality of os.waitpid but not limit it to child process

Is it possible to add this as an extra check for isalive()?

masteret commented 7 years ago

I have made a commit in my fork repo of ptyprocess https://github.com/masteret/ptyprocess/commit/420f6575c97759611d56d06034c17fe9d347cf20 If it is ok to use please allow me to create a PR

rocmit commented 7 years ago

I also ran into this issue. It would be great if this issue can be fixed soon!

rocmit commented 7 years ago

Just wanted to point out another thing that we should consider when implementing the solution...

In the same scenario, if the child is closed, terminated, or killed by process B, it will become a zombie process (PID is still valid but the process is dead)... This os.kill(pid, 0) solution would not catch that since the return would be None, instead of an exception.

The use case is like this: 1) Process A created an child session 2) Process B is created by multiprocess and it uses the same child session 3) If Process B kills the child session that was created under process A, os.kill(pid, 0) will return None and isalive will return True.

MountainRider commented 7 years ago

Won't Process A successfully call os.waitpid(self.pid, waitpid_options) on the child?

rocmit commented 7 years ago

Yes, process A would be able to get the correct isalive() status with os.waitpid. However, if process B runs isalive check, it would run into the situation I described.

MountainRider commented 7 years ago

I just verified this with some code. After Process B killed Process C, os.kill did not raise an exception.

masteret commented 7 years ago

Thank you for the comments, I will look into the scenario and see if I can check it correctly

MountainRider commented 7 years ago

Perhaps @rocmit can make an helpful suggestion.

masteret commented 7 years ago

Since process A can get a correct isalive() status with os.waitpid The problem only happens when non-parent process call isalive with ptyprocess pid I can add an additional check to see if the pid is a zombie process by looking at the 3rd item in /proc/${pid}/stat if it is Z then that means the process is a zombie, then we can assume it is not alive

MountainRider commented 7 years ago

There won't be a proc file system on OS X.

rocmit commented 7 years ago

/proc/pid/stat file is what I use to workaround this after doing some research. Like MountainRider mentioned, this may not work on other OS.

masteret commented 7 years ago

After some research, I found psutil has a way to determine if a proc is zombie in different os. In Linux, it checks the proc and in OSX it uses sysctl in C.

jquast commented 7 years ago

Yes, a maintainer here to chime in, the only thing preventing me from reviewing this PR quickly is to get FreeBSD, Linux, and mac machines to run the current tests--we may even need additional tests to cover new logic here, we'll see what the coverage report says, and/ or write some if-else conditions around platform if necessary.