GitLab

Topic of discussion:

1. 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 explicit greenlet.kill(SomeException) call.
2. 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 exclude gevent.Timeout but include TimeoutError?
• 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 own Greenlet class.
• MASKED_GREENLETS which is a dict should probably be a gevent.local.local object for cleaner implementation
• We should probably include signal handlers in addition to the Greenlet.throw hook. In KillMask we would delay interrupts. And maybe we need a global handler for SIGINT (perhaps SIGTERM and SIGQUIT as well) that raises KeyBoardInterrupt in all greenlets? Or review all the places where we try to capture KeyBoardInterrupt.

@matias.guijarro @valentin.valls Correct me if some of these statements are wrong. I'm not 100% sure about any of this.