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

Remove PyOS_ReadlineFunctionPointer from the stable ABI list #88034

Closed
encukou opened this issue Apr 16, 2021 · 3 comments
Closed

Remove PyOS_ReadlineFunctionPointer from the stable ABI list #88034

encukou opened this issue Apr 16, 2021 · 3 comments

Comments

@encukou
Copy link
Member

@encukou encukou commented Apr 16, 2021

BPO 43868
Nosy @pfmoore, @vstinner, @tjguk, @encukou, @zware, @zooba
PRs
  • #25442
  • 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 2021-04-27.14:17:45.393>
    created_at = <Date 2021-04-16.14:48:28.426>
    labels = ['expert-C-API', '3.10', 'OS-windows']
    title = 'Remove PyOS_ReadlineFunctionPointer from the stable ABI list'
    updated_at = <Date 2021-04-27.14:17:45.393>
    user = 'https://github.com/encukou'

    bugs.python.org fields:

    activity = <Date 2021-04-27.14:17:45.393>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2021-04-27.14:17:45.393>
    closer = 'vstinner'
    components = ['Windows', 'C API']
    creation = <Date 2021-04-16.14:48:28.426>
    creator = 'petr.viktorin'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 43868
    keywords = ['patch']
    message_count = 3.0
    messages = ['391206', '391237', '391690']
    nosy_count = 6.0
    nosy_names = ['paul.moore', 'vstinner', 'tim.golden', 'petr.viktorin', 'zach.ware', 'steve.dower']
    pr_nums = ['25442']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue43868'
    versions = ['Python 3.10']

    @encukou
    Copy link
    Member Author

    @encukou encukou commented Apr 16, 2021

    The inclusion of PyOS_ReadlineFunctionPointer in python3dll.c (*) was a mistake. According to PEP-384:

    functions expecting FILE* are not part of the ABI, to avoid depending on a specific version of the Microsoft C runtime DLL on Windows.

    The situation may have changed and it might be reasonable to revisit this decision, but that would call for a larger discussion. There are FILE*-taking functions that are probably much ore useful than this one. (But, I think it's a good idea to limit the stable ABI to file-like Python objects anyway.)

    I see PEP-384 as being definitive (where it's not ambiguous). The python3dll.c list and public/private headers do not actually define the stable ABI.

    So, I'd like to remove the function from the list.

    ---

    (*) it was actually PC/python3.def in 3.2

    @vstinner
    Copy link
    Member

    @vstinner vstinner commented Apr 16, 2021

    Yes, please remove it from the limited C API.

    PyOS_ReadlineFunctionPointer has a bad API. I fixed recently PyOS_StdioReadline because it used the Python C API without holding the GIL. To acquire the GIL, you need to access a *private* _PyOS_ReadlineTState variable which is kind of unfortunate :-(

    commit c353764
    Author: Victor Stinner <vstinner@python.org>
    Date: Mon Jun 1 20:59:35 2020 +0200

    bpo-40826: Fix GIL usage in PyOS_Readline() (GH-20579)
    
    Fix GIL usage in PyOS_Readline(): lock the GIL to set an exception.
    
    Pass tstate to my_fgets() and _PyOS_WindowsConsoleReadline(). Cleanup
    these functions.
    

    See also:

    commit fa7ab6a
    Author: Victor Stinner <vstinner@python.org>
    Date: Wed Jun 3 14:39:59 2020 +0200

    bpo-40826: Add _PyOS_InterruptOccurred(tstate) function (GH-20599)
    
    my_fgets() now calls _PyOS_InterruptOccurred(tstate) to check for
    pending signals, rather calling PyOS_InterruptOccurred().
    
    my_fgets() is called with the GIL released, whereas
    PyOS_InterruptOccurred() must be called with the GIL held.
    
    test_repl: use text=True and avoid SuppressCrashReport in
    test_multiline_string_parsing().
    
    Fix my_fgets() on Windows: fgets(fp) does crash if fileno(fp) is closed.
    

    @encukou
    Copy link
    Member Author

    @encukou encukou commented Apr 23, 2021

    New changeset 91b69b7 by Petr Viktorin in branch 'master':
    bpo-43868: Remove PyOS_ReadlineFunctionPointer from the stable ABI list (GH-25442)
    91b69b7

    @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
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants