Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KeyboardInterrupt should come with a warning #86506

Closed
benfogle mannequin opened this issue Nov 13, 2020 · 5 comments
Closed

KeyboardInterrupt should come with a warning #86506

benfogle mannequin opened this issue Nov 13, 2020 · 5 comments
Labels
3.10 only security fixes docs Documentation in the Doc dir type-feature A feature request or enhancement

Comments

@benfogle
Copy link
Mannequin

benfogle mannequin commented Nov 13, 2020

BPO 42340
Nosy @JelleZijlstra, @benfogle, @miss-islington
PRs
  • bpo-42340: Document issues around KeyboardInterrupt #23255
  • [3.9] bpo-42340: Document issues around KeyboardInterrupt (GH-23255) #32184
  • [3.10] bpo-42340: Document issues around KeyboardInterrupt (GH-23255) #32185
  • Files
  • sigint_condition_1.py: sample code
  • sigint_condition_2.py
  • sigint_tempfile.py
  • sigint_zipfile.py
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2022-03-29.21:49:51.641>
    created_at = <Date 2020-11-13.02:20:05.315>
    labels = ['type-feature', '3.10', 'docs']
    title = 'KeyboardInterrupt should come with a warning'
    updated_at = <Date 2022-03-29.21:49:51.640>
    user = 'https://github.com/benfogle'

    bugs.python.org fields:

    activity = <Date 2022-03-29.21:49:51.640>
    actor = 'JelleZijlstra'
    assignee = 'docs@python'
    closed = True
    closed_date = <Date 2022-03-29.21:49:51.641>
    closer = 'JelleZijlstra'
    components = ['Documentation']
    creation = <Date 2020-11-13.02:20:05.315>
    creator = 'benfogle'
    dependencies = []
    files = ['49595', '49596', '49597', '49598']
    hgrepos = []
    issue_num = 42340
    keywords = ['patch']
    message_count = 5.0
    messages = ['380868', '416295', '416298', '416299', '416300']
    nosy_count = 4.0
    nosy_names = ['docs@python', 'JelleZijlstra', 'benfogle', 'miss-islington']
    pr_nums = ['23255', '32184', '32185']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue42340'
    versions = ['Python 3.10']

    @benfogle
    Copy link
    Mannequin Author

    benfogle mannequin commented Nov 13, 2020

    This is related to bpo-29988, and I'm happy to move this to there. I made this a separate issue because this is a workaround, not a fix as was being discussed there. Also unlike bpo-29988, this is not restricted to context managers or finally blocks.

    TL;DR: Raising exceptions from interrupt handlers (most notably KeyboardInterrupt) can wreak havoc in ways that are impossible to fix. This should be noted in the documentation, with a workaround.

    I've attached a few example scripts that cause various strange behavior on Linux when a KeyboardInterrupt is raised at just the right time. There are likely many, many more possible examples:

    • sigint_condition_1.py: Cause a deadlock with threading.Condition
    • sigint_condition_2.py: Cause a double-release and/or notify on unacquired threading.Condition
    • sigint_tempfile.py: Cause NamedTemporaryFiles to not be deleted
    • sigint_zipfile.py: Cause ZipExtFile to corrupt its state

    When a user presses Ctrl+C, a KeyboardInterrupt will be raised on the main thread at some later time. This exception may be raised after any bytecode, and most Python code, including the standard library, is not designed to handle exceptions that spring up from nowhere.

    As a simple example, consider threading.Condition:

        def __enter__(self):
            return self._lock.__enter__()

    The KeyboardInterrupt could be raised just prior to return. In this case, __exit__ will never be called, and the underlying lock will remain acquired. A similar problem occurs if KeyboardInterrupt occurs at the start of __exit__.

    This can be mitigated by attempting to catch a KeyboardInterrupt *absolutely everywhere*, but even then, it can't be fixed completely.

        def __enter__(self):
            try:
                # it could happen here, in which case we should not unlock
                ret = self._lock.__enter__()
                # it could happen here, in which case we must unlock
            except KeyboardInterrupt:
    	    # it could, in theory, happen again right here
                ...
    	    raise
    	return ret
            # it could happen here, which is the same problem we had before

    This is not restricted to context handlers or try/finally blocks. The zipfile module is a good example of code that is almost certain to enter an inconsistent state if a KeyboardInterrupt is raised while it's doing work:

        class ZipExtFile:
            ...
            def read1(self, n):
                ...
                self._readbuffer = b''
                # what happens if KeyboardInterrupt happens here?
                self._offset = 0
                ...

    Due to how widespread this is, it's not worth "fixing". (And honestly, it seems to be a rare problem in practice.) I believe that it would be better to clearly document that KeyboardInterrupt (or any exception propagated from a signal handler) may leave the system
    in an inconsistent state. Complex or high reliability applications should avoid catching KeyboardInterrupt as a way of gracefully shutting down, and should prefer registering their own SIGINT handler. They should also avoid raising exceptions from signal handlers at all.

    @benfogle benfogle mannequin added the 3.10 only security fixes label Nov 13, 2020
    @benfogle benfogle mannequin assigned docspython Nov 13, 2020
    @benfogle benfogle mannequin added docs Documentation in the Doc dir type-feature A feature request or enhancement 3.10 only security fixes labels Nov 13, 2020
    @benfogle benfogle mannequin assigned docspython Nov 13, 2020
    @benfogle benfogle mannequin added docs Documentation in the Doc dir type-feature A feature request or enhancement labels Nov 13, 2020
    @JelleZijlstra
    Copy link
    Member

    New changeset d0906c9 by benfogle in branch 'main':
    bpo-42340: Document issues around KeyboardInterrupt (GH-23255)
    d0906c9

    @miss-islington
    Copy link
    Contributor

    New changeset 66cde7c by Miss Islington (bot) in branch '3.10':
    bpo-42340: Document issues around KeyboardInterrupt (GH-23255)
    66cde7c

    @miss-islington
    Copy link
    Contributor

    New changeset c26af2b by Miss Islington (bot) in branch '3.9':
    bpo-42340: Document issues around KeyboardInterrupt (GH-23255)
    c26af2b

    @JelleZijlstra
    Copy link
    Member

    Thanks for the patch!

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.10 only security fixes docs Documentation in the Doc dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants