Open 1st1 opened 7 years ago
Do we know that the atexit handler will always be called early enough?
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.
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.
OK, then LGTM.
Shouldn't it be fixed in signalmodule.c
instead? (see my comment in PR #396)
If something needs to be changed in signalmodule.c please open an issue on bugs.python.org.
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.
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.
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
Maybe they ran configure with different parameters or in a different context or with a different version of Xcode?
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.
@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 |
I think I've figured out what's going on with
remove_signal_handler
.This PR fixes: