Skip to content
This repository was archived by the owner on Nov 23, 2017. It is now read-only.

Fix remove_signal_handler to not to crash after PyOS_FiniInterrupts #456

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions asyncio/unix_events.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""Selector event loop for Unix with signal handling."""

import atexit
import errno
import os
import signal
Expand All @@ -9,6 +10,7 @@
import sys
import threading
import warnings
import weakref


from . import base_events
Expand Down Expand Up @@ -39,6 +41,12 @@ def _sighandler_noop(signum, frame):
pass


def _loop_atexit_callback(loop_ref):
loop = loop_ref()
if loop is not None:
loop._interpreter_shutting_down = True


class _UnixSelectorEventLoop(selector_events.BaseSelectorEventLoop):
"""Unix event loop.

Expand All @@ -47,6 +55,15 @@ class _UnixSelectorEventLoop(selector_events.BaseSelectorEventLoop):

def __init__(self, selector=None):
super().__init__(selector)

# atexit callbacks are fired before PyOS_FiniInterrupts, which
# allows us to workaround bugs in remove_signal_handler.
atexit.register(_loop_atexit_callback, weakref.ref(self))
# When _interpreter_shutting_down is True, PyOS_FiniInterrupts
# has already been called and signalmodule's internal state was
# cleaned up.
self._interpreter_shutting_down = False

self._signal_handlers = {}

def _socketpair(self):
Expand Down Expand Up @@ -124,6 +141,11 @@ def remove_signal_handler(self, sig):

Return True if a signal handler was removed, False if not.
"""
if self._interpreter_shutting_down:
# The interpreter is being shutdown. `PyOS_FiniInterrupts`
# was already called and it has restored all signals already.
return
Copy link
Member Author

@1st1 1st1 Nov 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should return a bool.


self._check_signal(sig)
try:
del self._signal_handlers[sig]
Expand Down