Fix remove_signal_handler to not to crash after PyOS_FiniInterrupts

[1st1]

I think I've figured out what's going on with remove_signal_handler.

This PR fixes:

• TypeError: signal handler must be signal.SIG_IGN, signal.SIG_DFL, or a callable object #396
• http://bugs.python.org/issue23548
• http://bugs.python.org/issue28628

[1st1]

Instead of just return, we can emit a warning that the loop wasn't properly closed. @gvanrossum what do you think about that?

[gvanrossum]

Only in debug mode. But frankly I don't understand this code.

[gvanrossum]

This should return a bool.

[gvanrossum]

I don't understand all this well enough to know whether this is a good idea or not. We really need to find someone else who wants to co-own asyncio -- ATM I feel my role is mostly that of giving you permission to commit what you see fit and I am not actually comfortable with this -- we'll end up with there being only one person who understands asyncio and that's not enough for such an important stdlib module.

[gvanrossum]

Only in debug mode. But frankly I don't understand this code.

[gvanrossum]

Do we know that the atexit handler will always be called early enough?

[1st1]

I don't understand all this well enough to know whether this is a good idea or not. We really need to find someone else who wants to co-own asyncio -- ATM I feel my role is mostly that of giving you permission to commit what you see fit and I am not actually comfortable with this -- we'll end up with there being only one person who understands asyncio and that's not enough for such an important stdlib module.

I agree. I'm myself not feeling comfortable, even two of us doesn't seem to be enough to support asyncio.

@asvetlov, @methane and @Haypo: are you interested to help us with asyncio? At least to offer help with PRs and participate in related discussions.

Do we know that the atexit handler will always be called early enough?

Py_FinalizeEx first calls atexit functions and only after that it calls PyOS_FiniInterrupts(). The latter cleans up the internal state of signals module, making it useless. So yes, we're confident about atexit.

[asvetlov]

I think the PR makes sense: we raise ResourceWarning for unclosed transports in debug mode.
I believe we should do the same for unclosed loop too.

[gvanrossum]

OK, then LGTM.

[vxgmichel]

Shouldn't it be fixed in signalmodule.c instead? (see my comment in PR #396)

[gvanrossum]

If something needs to be changed in signalmodule.c please open an issue on bugs.python.org.

[1st1]

Yes, the bug in _signal can (and should) be fixed, and I have a working patch for that. My worry is that it might be a bit too late to do that for 3.6. I'll submit the patch and ask Ned's opinion.

[1st1]

I've no idea what's going on. I can reproduce this bug with python installed from MacPorts. I can't reproduce it with python I build from source.

[1st1]

The bug is the following program:

import signal
import asyncio

def foo():
    pass

async def bar():
    pass

def main():
    loop = asyncio.get_event_loop()
    loop.add_signal_handler(signal.SIGINT, lambda: None)
    loop.add_signal_handler(signal.SIGHUP, lambda: None)
    loop.run_until_complete(bar())

if __name__ == "__main__":
    main()

It should spit out something like this:

Exception ignored in: <bound method BaseEventLoop.__del__ of <_UnixSelectorEventLoop running=False closed=True debug=False>>
Traceback (most recent call last):
  File "/opt/local/Library/Frameworks/Python.framework/Versions/3.5/lib/python3.5/asyncio/base_events.py", line 431, in __del__
  File "/opt/local/Library/Frameworks/Python.framework/Versions/3.5/lib/python3.5/asyncio/unix_events.py", line 58, in close
  File "/opt/local/Library/Frameworks/Python.framework/Versions/3.5/lib/python3.5/asyncio/unix_events.py", line 139, in remove_signal_handler
  File "/opt/local/Library/Frameworks/Python.framework/Versions/3.5/lib/python3.5/signal.py", line 47, in signal
TypeError: signal handler must be signal.SIG_IGN, signal.SIG_DFL, or a callable object

[gvanrossum]

Maybe they ran configure with different parameters or in a different context or with a different version of Xcode?

[1st1]

Maybe. It should be something that changes how & when objects are GCed. The above traceback happens when objects are GCed after the signals module is finalized (and that's what I've seen in reports from asyncio and uvloop users). I'll try to get to the bottom of this.

[vxgmichel]

@1st1
I tried to reproduce the bug with python 3.5 and python 3.6 (built from the latest sources) with your test for asyncio and this one for signal:

import _signal

class A:
    def __del__(self):
        _signal.signal(_signal.SIGTERM, _signal.SIG_DFL)

a = A()

My results:

	asyncio test	signal test
python 3.5	TypeError	TypeError
python 3.6	No Error	TypeError