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

Jump to bottom

Improve stdlib module initialization error handling. #83004

Open
brandtbucher opened this issue Nov 16, 2019 · 31 comments
Open

Improve stdlib module initialization error handling. #83004

brandtbucher opened this issue Nov 16, 2019 · 31 comments
Assignees
@hauntsaninja
Labels
3.10 only security fixes 3.11 bug and security fixes 3.12 new features, bugs and security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@brandtbucher
Copy link
Member

brandtbucher commented Nov 16, 2019 •
edited by bedevere-bot

BPO 38823
Nosy @Yhg1s, @vstinner, @asvetlov, @serhiy-storchaka, @miss-islington, @brandtbucher, @shihai1991, @erlend-aasland
PRs
  • bpo-38823: Clean up refleaks in _asyncio initialization. #17195
  • [3.8] bpo-38823: Clean up refleaks in _asyncio initialization. (GH-17195) #17196
  • [3.7] bpo-38823: Clean up refleaks in _asyncio initialization. (GH-17195) #17197
  • bpo-38823: Clean up refleaks in _contextvars initialization. #17198
  • [3.7] bpo-38823: Clean up refleaks in _contextvars initialization. (GH-17198) #17199
  • [3.8] bpo-38823: Clean up refleaks in _contextvars initialization. (GH-17198) #17200
  • bpo-38823: Clean up refleaks in _tkinter initialization. #17206
  • bpo-38823: Clean up _statistics initialization. #17215
  • bpo-38823: Clean up _xxtestfuzz initialization. #17216
  • [3.8] bpo-38823: Clean up refleaks in _tkinter initialization. (GH-17206) #17226
  • [3.7] bpo-38823: Clean up refleaks in _tkinter initialization. (GH-17206) #17227
  • bpo-38823: Clean up refleak in _tracemalloc initialization. #17235
  • bpo-38823: Clean up refleak in fcntl initialization. #17236
  • bpo-38823: Clean up refleaks in faulthandler initialization. #17250
  • bpo-38823: Clean up refleak in marshal initialization. #17260
  • [3.8] bpo-38823: Fix refleaks in faulthandler init error path on Windows (GH-17250) #17263
  • [3.7] bpo-38823: Fix refleaks in faulthandler init error path on Windows (GH-17250) #17264
  • [3.8] bpo-38823: Fix refleak in marshal init error path (GH-17260) #17271
  • [3.7] bpo-38823: Fix refleak in marshal init error path (GH-17260) #17272
  • bpo-38823: Clean up refleaks in _ast initialization. #17276
  • [3.8] bpo-38823: Fix refleak in marshal init error path (GH-17260) #17280
  • [3.7] bpo-38823: Fix refleak in marshal init error path (GH-17260) #17281
  • [3.8] bpo-38823: Fix refleak in _tracemalloc init error handling (GH-17235) #17282
  • [3.7] bpo-38823: Fix refleak in _tracemalloc init error handling (GH-17235) #17283
  • bpo-38823: Add a private _PyModule_StealObject API. #17298
  • bpo-1635741: Calling Py_INCREF() after PyModule_AddObject() success to run #18365
  • bpo-38823: Fix refleaks in _ctypes extension init #23247
  • bpo-38823: Always build _ctypes with wchar_t #23248
  • bpo-38823: Fix compiler warning in _ctypes on Windows #23258
    		•

    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 = 'https://github.com/brandtbucher'
closed_at = None
created_at = <Date 2019-11-16.18:54:25.878>
labels = ['3.7', '3.8', 'type-bug', 'library', '3.9']
title = 'Improve stdlib module initialization error handling.'
updated_at = <Date 2020-11-13.13:44:17.121>
user = 'https://github.com/brandtbucher'

    bugs.python.org fields:

    activity = <Date 2020-11-13.13:44:17.121>
actor = 'vstinner'
assignee = 'brandtbucher'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2019-11-16.18:54:25.878>
creator = 'brandtbucher'
dependencies = []
files = []
hgrepos = []
issue_num = 38823
keywords = ['patch']
message_count = 30.0
messages = ['356764', '356765', '356773', '356774', '356775', '356781', '356782', '356783', '356809', '356820', '356872', '356873', '356874', '356984', '357002', '357003', '357004', '357014', '357019', '357029', '357044', '357046', '357047', '357050', '357051', '357114', '361486', '380822', '380824', '380883']
nosy_count = 8.0
nosy_names = ['twouters', 'vstinner', 'asvetlov', 'serhiy.storchaka', 'miss-islington', 'brandtbucher', 'shihai1991', 'erlendaasland']
pr_nums = ['17195', '17196', '17197', '17198', '17199', '17200', '17206', '17215', '17216', '17226', '17227', '17235', '17236', '17250', '17260', '17263', '17264', '17271', '17272', '17276', '17280', '17281', '17282', '17283', '17298', '18365', '23247', '23248', '23258']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue38823'
versions = ['Python 3.7', 'Python 3.8', 'Python 3.9']
    @brandtbucher
    Copy link
    Member Author

    Many of the C stdlib modules can benefit from improved error handling during initialization. I've now had two PRs where the authors had reference leaks on error conditions, but defended their decisions by pointing to examples of similar idioms all over the stdlib.

    The problems fall into two related categories, mostly:

    • Not DECREF'ing the new module object on failure.
    • Not handling errors raised by the PyModule_Add* family of functions... specifically, the weird steal-on-success semantics of PyModule_AddObject. I've already improved the docs for this, so we should see the issue less, but our own code should still be fixed.

    I intend to turn this into a longer term project. I'll be working my way through these modules bit-by-bit over time, using this issue to track all of them (since there are a few dozen cases). I'd rather not make one huge one that spams all of the code owners and is impossible to review.

    If anybody want to make themselves available to review/merge these as I go along, that would be great! Many of the ones I'll start with are just adding trivial DECREFs to the more popular modules (_asyncio, _contextvars, _functools, _random, _warnings, etc...).

    @brandtbucher brandtbucher added the 3.9 only security fixes label Nov 16, 2019
    @brandtbucher brandtbucher self-assigned this Nov 16, 2019
    @brandtbucher brandtbucher added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Nov 16, 2019
    @brandtbucher
    Copy link
    Member Author

    How do others feel about the creation of a new private API? It would keep these diffs smaller and ease refactoring... and it would probably be good to have around anyways:

    /* Like PyModule_AddObject, but steals o on success AND failure. */

    int
_PyModule_StealObject(PyObject *m, const char *name, PyObject *o)
{
    if (PyModule_AddObject(m, name, o) < 0) {
        Py_XDECREF(o);
        return -1;
    }
    return 0;
}

    @miss-islington
    Copy link
    Contributor

    New changeset c3f6bdc by Miss Islington (bot) (Brandt Bucher) in branch 'master':
    bpo-38823: Clean up refleaks in _asyncio initialization. (GH-17195)
    c3f6bdc

    @miss-islington
    Copy link
    Contributor

    New changeset 48f4f75 by Miss Islington (bot) in branch '3.8':
    bpo-38823: Clean up refleaks in _asyncio initialization. (GH-17195)
    48f4f75

    @miss-islington
    Copy link
    Contributor

    New changeset 825e91b by Miss Islington (bot) in branch '3.7':
    bpo-38823: Clean up refleaks in _asyncio initialization. (GH-17195)
    825e91b

    @miss-islington
    Copy link
    Contributor

    New changeset 143a97f by Miss Islington (bot) (Brandt Bucher) in branch 'master':
    bpo-38823: Clean up refleaks in _contextvars initialization. (GH-17198)
    143a97f

    @miss-islington
    Copy link
    Contributor

    New changeset 8a334af by Miss Islington (bot) in branch '3.7':
    bpo-38823: Clean up refleaks in _contextvars initialization. (GH-17198)
    8a334af

    @miss-islington
    Copy link
    Contributor

    New changeset 1fe79a4 by Miss Islington (bot) in branch '3.8':
    bpo-38823: Clean up refleaks in _contextvars initialization. (GH-17198)
    1fe79a4

    @asvetlov
    Copy link
    Contributor

    Is anything left to be done for the issue?

    @asvetlov asvetlov added 3.7 only security fixes 3.8 only security fixes labels Nov 17, 2019
    @brandtbucher
    Copy link
    Member Author

    Yes, there are still a few dozen modules I plan to update!

    @miss-islington
    Copy link
    Contributor

    New changeset 289cf0f by Miss Islington (bot) (Brandt Bucher) in branch 'master':
    bpo-38823: Clean up refleaks in _tkinter initialization. (GH-17206)
    289cf0f

    @miss-islington
    Copy link
    Contributor

    New changeset 9e4d031 by Miss Islington (bot) in branch '3.7':
    bpo-38823: Clean up refleaks in _tkinter initialization. (GH-17206)
    9e4d031

    @miss-islington
    Copy link
    Contributor

    New changeset 42a4359 by Miss Islington (bot) in branch '3.8':
    bpo-38823: Clean up refleaks in _tkinter initialization. (GH-17206)
    42a4359

    @Yhg1s
    Copy link
    Member

    Yhg1s commented Nov 19, 2019

    New changeset 54b32c9 by T. Wouters (Brandt Bucher) in branch 'master':
    bpo-38823: Clean up refleak in fcntl module initialization. (GH-17236)
    54b32c9

    @vstinner
    Copy link
    Member

    New changeset ac22354 by Victor Stinner (Brandt Bucher) in branch 'master':
    bpo-38823: Fix refleaks in faulthandler init error path on Windows (GH-17250)
    ac22354

    @miss-islington
    Copy link
    Contributor

    New changeset 5bd2af9 by Miss Islington (bot) in branch '3.7':
    bpo-38823: Fix refleaks in faulthandler init error path on Windows (GH-17250)
    5bd2af9

    @miss-islington
    Copy link
    Contributor

    New changeset a5ed2fe by Miss Islington (bot) in branch '3.8':
    bpo-38823: Fix refleaks in faulthandler init error path on Windows (GH-17250)
    a5ed2fe

    @vstinner
    Copy link
    Member

    New changeset 33b671e by Victor Stinner (Brandt Bucher) in branch 'master':
    bpo-38823: Fix refleak in marshal init error path (GH-17260)
    33b671e

    @vstinner
    Copy link
    Member

    The Azure Pipelines are sick tonight (unable to publish test results). PR bpo-17272 and PR bpo-17271 are blocked by this CI.

    @brandtbucher
    Copy link
    Member Author

    Thanks Victor. These obviously aren’t urgent, so feel free to return to them whenever’s convenient.

    I also pinged you on bpo-17235 (_tracemalloc) too.

    @vstinner
    Copy link
    Member

    New changeset d51a363 by Victor Stinner (Brandt Bucher) in branch 'master':
    bpo-38823: Fix refleak in _tracemalloc init error handling (GH-17235)
    d51a363

    @miss-islington
    Copy link
    Contributor

    New changeset 63f09e7 by Miss Islington (bot) in branch '3.7':
    bpo-38823: Fix refleak in marshal init error path (GH-17260)
    63f09e7

    @miss-islington
    Copy link
    Contributor

    New changeset 2ea4c37 by Miss Islington (bot) in branch '3.8':
    bpo-38823: Fix refleak in marshal init error path (GH-17260)
    2ea4c37

    @miss-islington
    Copy link
    Contributor

    New changeset daf7a08 by Miss Islington (bot) in branch '3.8':
    bpo-38823: Fix refleak in _tracemalloc init error handling (GH-17235)
    daf7a08

    miss-islington pushed a commit to miss-islington/cpython that referenced this issue Nov 6, 2022
    @hauntsaninja @miss-islington
    pythongh-83004: Clean up refleak in _zoneinfo initialisation (pythonG…
    7cf8db3 
    …H-98842)

(cherry picked from commit 31f2f65)

Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com>
    JelleZijlstra pushed a commit that referenced this issue Nov 6, 2022
    @hauntsaninja
    gh-83004: Clean up refleak in _pickle initialisation (#98841)
    d3b82b4
    miss-islington pushed a commit to miss-islington/cpython that referenced this issue Nov 6, 2022
    @hauntsaninja @miss-islington
    pythongh-83004: Clean up refleak in _pickle initialisation (pythonGH-…
    a676ed7 
    …98841)

(cherry picked from commit d3b82b4)

Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com>
    @bedevere-bot bedevere-bot mentioned this issue Nov 6, 2022
    Merged
    miss-islington pushed a commit to miss-islington/cpython that referenced this issue Nov 6, 2022
    @hauntsaninja @miss-islington
    pythongh-83004: Clean up refleak in _pickle initialisation (pythonGH-…
    fca03c0 
    …98841)

(cherry picked from commit d3b82b4)

Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com>
    @bedevere-bot bedevere-bot mentioned this issue Nov 6, 2022
    Merged
    miss-islington added a commit that referenced this issue Nov 6, 2022
    @miss-islington @hauntsaninja
    gh-83004: Clean up refleak in _zoneinfo initialisation (GH-98842)
    4b1e45e 
    (cherry picked from commit 31f2f65)

Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com>
    @kumaraditya303 kumaraditya303 added 3.11 bug and security fixes 3.10 only security fixes 3.12 new features, bugs and security fixes and removed 3.9 only security fixes 3.8 only security fixes 3.7 only security fixes labels Nov 6, 2022
    miss-islington added a commit that referenced this issue Nov 6, 2022
    @miss-islington @hauntsaninja
    gh-83004: Clean up refleak in _zoneinfo initialisation (GH-98842)
    70006d6 
    (cherry picked from commit 31f2f65)

Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com>
    miss-islington added a commit that referenced this issue Nov 6, 2022
    @miss-islington @hauntsaninja
    gh-83004: Clean up refleak in _pickle initialisation (GH-98841)
    5707792 
    (cherry picked from commit d3b82b4)

Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com>
    miss-islington added a commit that referenced this issue Nov 6, 2022
    @miss-islington @hauntsaninja
    gh-83004: Clean up refleak in _pickle initialisation (GH-98841)
    380b12e 
    (cherry picked from commit d3b82b4)

Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com>
    @brandtbucher
    Copy link
    Member Author

    I know I'm a bit late, but thanks so much for picking up this old project @hauntsaninja!

    erlend-aasland added a commit to erlend-aasland/cpython that referenced this issue Apr 5, 2023
    @erlend-aasland
    pythongh-83004: Harden _socket init
    8f9bea1
    @bedevere-bot bedevere-bot mentioned this issue Apr 5, 2023
    Merged
    miss-islington pushed a commit that referenced this issue Apr 7, 2023
    @erlend-aasland
    gh-83004: Harden _socket init (GH-103261)
    91794e5 
    Automerge-Triggered-By: GH:erlend-aasland
    @bedevere-bot bedevere-bot mentioned this issue Apr 8, 2023
    Open
    erlend-aasland added a commit to erlend-aasland/cpython that referenced this issue Apr 8, 2023
    @erlend-aasland
    pythongh-83004: Harden msvcrt init
    18820c1
    @bedevere-bot bedevere-bot mentioned this issue Apr 8, 2023
    Merged
    erlend-aasland added a commit to erlend-aasland/cpython that referenced this issue Apr 8, 2023
    @erlend-aasland
    pythongh-83004: Harden winsound init
    ecef2d9
    erlend-aasland added a commit to erlend-aasland/cpython that referenced this issue Apr 8, 2023
    @erlend-aasland
    pythongh-83004: Harden winreg init
    25a07f8
    @bedevere-bot bedevere-bot mentioned this issue Apr 8, 2023
    Open
    erlend-aasland added a commit to erlend-aasland/cpython that referenced this issue Apr 8, 2023
    @erlend-aasland
    pythongh-83004: Harden winreg init
    e664a52
    erlend-aasland added a commit that referenced this issue Apr 9, 2023
    @erlend-aasland
    gh-83004: Harden winsound init (#103385)
    6d97e52
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @hauntsaninja hauntsaninja

    Labels
    3.10 only security fixes 3.11 bug and security fixes 3.12 new features, bugs and security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    7 participants
    @vstinner @asvetlov @Yhg1s @hauntsaninja @miss-islington @brandtbucher @kumaraditya303