KillMask and interrupts
Topic of discussion:
- What precisely is the purpose of
KillMask
? "To protect some code" but against what? It doesn't protect against SIGINT for example, it only protects against an explicitgreenlet.kill(SomeException)
call. - We often use
try: ... except KeyBoardInterrupt: ...
(including myself) while I suspect this is useless.
The KillMask
only works when the current greenlet is our patched Greenlet
class. So KillMask
should probably raise an exception upon context entering in a native Greenlet
shouldn't it? Wouldn't it be better to patch the greenlet instances upon entering the context (also using gevent.local.local()
instead of dict()
for MASKED_GREENLETS
)?
Our custom Greenlet
class overrides the throw
method which is a method called by the gevent loop as a result of someone explicitly calling kill
on that Greenlet
instance. In other words it does not get called on interrupts like SIGINT or SIGTERM.
The throw
method captures exceptions when inside a KillMask
context and reraises the last exception when exiting the context. This includes all exceptions (so that's BaseException
) except for gevent.Timeout
. So we are making an exception for greenlet.kill(gevent.Timeout)
? Then what about TimeoutError
?
Side note: all over the code we use try: ... except KeyBoardInterrupt: ...
and I wonder why? I did it myself without thinking about it. In fact it seems you will never see a KeyBoardInterrupt
exception in any part of our code (I guess gevent has registered its own SIGINT handler). And even if you did, it would be in only one greenlet. You could only capture a KeyBoardInterrupt
if someone explicitly calls greenlet.kill(KeyBoardInterrupt)
. We should really need to register custom signal handlers if we want to delay interrupts to protect some code, raise KeyBoardInterrupt
in all running greenlets or whatever.
So to conclude:
- What is
KillMask
for precisely? Why excludegevent.Timeout
but includeTimeoutError
? -
KillMask
needs to raise an exception upon entering when the current greenlet is not our patched one. Alternatively we patch greenlet instances instead of having our ownGreenlet
class. -
MASKED_GREENLETS
which is adict
should probably be agevent.local.local
object for cleaner implementation - We should probably include signal handlers in addition to the
Greenlet.throw
hook. InKillMask
we would delay interrupts. And maybe we need a global handler for SIGINT (perhaps SIGTERM and SIGQUIT as well) that raisesKeyBoardInterrupt
in all greenlets? Or review all the places where we try to captureKeyBoardInterrupt
.
@matias.guijarro @valentin.valls Correct me if some of these statements are wrong. I'm not 100% sure about any of this.