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

is_running() should be called before kill() #58

Closed
giampaolo opened this issue May 23, 2014 · 8 comments
Closed

is_running() should be called before kill() #58

giampaolo opened this issue May 23, 2014 · 8 comments
Assignees

Comments

@giampaolo
Copy link
Owner

From [email protected] on July 13, 2009 17:01:25

I think it makes sense to call is_running() before kill() as a safety
measure since the original process could be gone in meantime and the same
PID assigned to a different process.

Original issue: http://code.google.com/p/psutil/issues/detail?id=58

@giampaolo giampaolo self-assigned this May 23, 2014
@giampaolo
Copy link
Owner Author

From [email protected] on July 13, 2009 18:17:11

That doesn't really save us from anything, since the process could disappear between
calling is_running() and calling kill() anyway. A race condition is a race condition,
even if you do it really fast ;)

Also, bear in mind that on Windows, we're checking if the PID is running in the C
code before calling TerminateProcess() and again when we open the process handle. On
UNIX platforms we use the os.kill() function with a try/except watching for ESRCH
being thrown in the event the process does not exist. 

I think we're covered just fine on this front without adding additional code.

@giampaolo
Copy link
Owner Author

From [email protected] on July 14, 2009 00:52:09

I didn't mean to do this to prevent race conditions, also because NoSuchProcess is
already raised in case the process disappears on us, and there's also a test case
(test_zombie_process) showing how that already works.
I wanted to do this for checking the process identity to make sure that we aren't
going to kill another process we didn't mean to.

is_running() can help us on doing this as it creates a new Process object for the pid
and compares the "hash" for it (cmdline, path, etc...) with the hash of the current
object before executing the action (kill()).

Hope you can see my point now.

@giampaolo
Copy link
Owner Author

From [email protected] on July 14, 2009 06:33:03

There's still a race condition, since such a scenario could happen in between calling
is_running() and kill() on the process. It's unlikely since the OS usually doesn't
reuse PIDs that quickly, at least as far as I know that's the case for the OSes we
support so far, but it's still theoretically possible. 

However, I can see this being at least more accurate than a use case where someone
creates a Process object and then an hour later issues kill() against it. It could
potentially be a performance issue at some point but we can worry about that later if
it comes up I guess.

@giampaolo
Copy link
Owner Author

From [email protected] on July 14, 2009 07:08:44

Calling is_running() before kill() surely requires a considerable additional amount
of time compared to the current implementation, in fact I was thinking that we could
also speed up the __eq__ implementation somehow.

Anyway, I don't see kill() as a kind function that is called so often to represent an
actual performance problem, imho.

@giampaolo
Copy link
Owner Author

From [email protected] on July 14, 2009 07:51:24

Fixed as r415 .

Status: Fixed
Labels: -Progress-0in4

@giampaolo
Copy link
Owner Author

From [email protected] on September 03, 2009 12:48:09

Status: FixedInSVN

@giampaolo
Copy link
Owner Author

From [email protected] on September 17, 2009 01:57:42

Status: Fixed

@giampaolo
Copy link
Owner Author

From g.rodola on March 02, 2013 03:49:55

Updated csets after the SVN -> Mercurial migration: r415 == revision f6713a409197

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

1 participant