Comment 6 for bug 495023

Revision history for this message
Martin Pool (mbp) wrote : Re: [Bug 495023] Re: Interrupting commit to smart server sometimes removes files

2009/12/23 Andrew Bennetts <email address hidden>:
> But maybe the way _readdir_pyx handles EINTR from readdir isn't quite
> right.  I'm not sure, but I think we may need to call PyErr_CheckSignals
> if we get EINTR, otherwise the signal that just arrived (potentially
> SIGINT, i.e. KeyboardInterrupt) may not be noticed until some arbitrary
> point later, causing KeyboardInterrupt at strange times?  I've attached
> a patch to _readdir_pyx.pyx that tries this.

Well spotted.

So if a sigint arrives during the read, the OS will run Python's
signal handler, which does Py_AddPendingCall, and it's possible
unhappiness will result if that stalls for too long.

http://docs.python.org/c-api/exceptions.html#PyErr_CheckSignals

also of interest

btw http://manpages.ubuntu.com/manpages/lucid/en/man3/siginterrupt.3posix.html

in your patch, I think we need to check the return code of
PyErr_CheckSignals and return - or will pyrex automatically take care
of that if an exception is raised?

Strictly speaking EAGAIN doesn't need this handling but it's probably harmless.

It looks like we actually need similar handling in every case where
this code is turning errno into an OSError or similar.
PyErr_CheckSignals is implicitly called by PyErr_SetFromErrno but
we're not doing that. Alternatively we could delete the manual
'raise' calls and just do PyErr_SetFromErrno but that would leave out
the messages. Perhaps we should add a common function to do this.

--
Martin <http://launchpad.net/~mbp/>