On 12/15/2011 07:36 PM, Josh Kupershmidt wrote:
The only niggling concern I have about this patch (and, I think, all similar ones proposed) is the possible race condition between the permissions checking and the actual call of kill() inside pg_signal_backend().
This is a problem with the existing code though, and the proposed changes don't materially alter that; there's just another quick check in one path through. Right now we check if someone is superuser, then if it's a backend PID, then we send the signal. If you assume someone can run through all the PIDs between those checks and the kill, the system is already broken that way.
I notice that BackendPidGetProc() has the comment: ... Note that it is up to the caller to be sure that the question remains meaningful for long enough for the answer to be used ... So someone has mulled this over before. It took my script maybe 15-20 minutes to cause a wraparound by running fork() in a loop, and that wraparound would somehow have to occur within the short execution of pg_signal_backend().
Right, the the possibility you're raising is the obvious flaw with this approach. Currently the only consumers of BackendPidGetProc or IsBackendPid are these cancel/terminate functions. As you measured, running through the PIDs fast enough to outrace any of that code is unlikely. I'm sure a lot of other UNIX apps would break, too, if that ever became untrue. The sort of thing that comment is warning is that you wouldn't want to do something like grab a PID, vacuum a table, and then assume that PID was still valid.
-- Greg Smith 2ndQuadrant US g...@2ndquadrant.com Baltimore, MD PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
