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

PyEval_AcquireLock() and PyEval_AcquireThread() do not handle runtime finalization properly. #80656

Closed
ericsnowcurrently opened this issue Mar 29, 2019 · 7 comments
Labels
3.8 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@ericsnowcurrently
Copy link
Member

BPO 36475
Nosy @vstinner, @ericsnowcurrently, @nanjekyejoannah
PRs
  • bpo-36475: Finalize PyEval_AcquireLock() and PyEval_AcquireThread() properly #12667
  • bpo-36475: Make PyThread_exit_thread with _Py_NO_RETURN #13068
  • 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 2019-04-29.09:23:39.689>
    created_at = <Date 2019-03-29.19:26:20.924>
    labels = ['interpreter-core', 'type-bug', '3.8']
    title = 'PyEval_AcquireLock() and PyEval_AcquireThread() do not handle runtime finalization properly.'
    updated_at = <Date 2020-03-09.10:53:27.812>
    user = 'https://github.com/ericsnowcurrently'

    bugs.python.org fields:

    activity = <Date 2020-03-09.10:53:27.812>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2019-04-29.09:23:39.689>
    closer = 'vstinner'
    components = ['Interpreter Core']
    creation = <Date 2019-03-29.19:26:20.924>
    creator = 'eric.snow'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 36475
    keywords = ['patch']
    message_count = 7.0
    messages = ['339138', '339365', '339876', '341054', '341055', '341389', '363714']
    nosy_count = 3.0
    nosy_names = ['vstinner', 'eric.snow', 'nanjekyejoannah']
    pr_nums = ['12667', '13068']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue36475'
    versions = ['Python 3.8']

    @ericsnowcurrently
    Copy link
    Member Author

    Daemon threads keep running until they finish or until finalization starts. For the latter, there is a check right after the thread acquires the GIL which causes the thread to exit if runtime finalization has started. [1] However, there are functions in the C-API that facilitate acquiring the GIL, but do not cause the thread to exit during finalization:

      PyEval_AcquireLock()
      PyEval_AcquireThread()

    Daemon threads that acquire the GIL through these can cause a deadlock during finalization. (See issue bpo-36469.) They should probably be updated to match what PyEval_RestoreThread() does.

    [1] see PyEval_RestoreThread() and the eval loop, in PyEval_EvalFrameEx()

    @ericsnowcurrently ericsnowcurrently added 3.7 (EOL) end of life 3.8 only security fixes type-bug An unexpected behavior, bug, or error interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Mar 29, 2019
    @nanjekyejoannah
    Copy link
    Member

    @eric.snow , you can review the PR I submitted for this.

    @nanjekyejoannah
    Copy link
    Member

    @eric do we need any tests for this?

    @vstinner
    Copy link
    Member

    New changeset f781d20 by Victor Stinner (Joannah Nanjekye) in branch 'master':
    bpo-36475: Finalize PyEval_AcquireLock() and PyEval_AcquireThread() properly (GH-12667)
    f781d20

    @vstinner
    Copy link
    Member

    I am not comfortable to backport this change to Python 3.7. It's too early to know how it will impact applications and how many complains we will get :-) If someone really wants to backport this scary change to 3.7, I would suggest to wait for 1 month after Python 3.8.0 final release.

    I close the issue.

    See bpo-36479 for the follow-up.

    @vstinner vstinner added 3.8 only security fixes and removed 3.7 (EOL) end of life 3.8 only security fixes labels Apr 29, 2019
    @vstinner
    Copy link
    Member

    vstinner commented May 4, 2019

    New changeset c664b34 by Victor Stinner in branch 'master':
    bpo-36475: Make PyThread_exit_thread with _Py_NO_RETURN (GH-13068)
    c664b34

    @vstinner
    Copy link
    Member

    vstinner commented Mar 9, 2020

    I marked bpo-23592 as duplicate of this issue.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.8 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants