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

Removing old buffer support #85275

Open
methane opened this issue Jun 24, 2020 · 12 comments
Open

Removing old buffer support #85275

methane opened this issue Jun 24, 2020 · 12 comments

Comments

@methane
Copy link
Member

@methane methane commented Jun 24, 2020

BPO 41103
Nosy @vstinner, @encukou, @methane, @ambv, @hroncok, @miss-islington
PRs
  • #21117
  • #27437
  • #27441
  • 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/encukou'
    closed_at = None
    created_at = <Date 2020-06-24.13:43:42.640>
    labels = ['expert-C-API', '3.10', '3.11']
    title = 'Removing old buffer support'
    updated_at = <Date 2021-08-05.08:33:02.859>
    user = 'https://github.com/methane'

    bugs.python.org fields:

    activity = <Date 2021-08-05.08:33:02.859>
    actor = 'petr.viktorin'
    assignee = 'petr.viktorin'
    closed = False
    closed_date = None
    closer = None
    components = ['C API']
    creation = <Date 2020-06-24.13:43:42.640>
    creator = 'methane'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 41103
    keywords = ['patch']
    message_count = 12.0
    messages = ['372251', '372391', '381854', '381865', '381873', '398459', '398460', '398467', '398476', '398477', '398560', '398974']
    nosy_count = 7.0
    nosy_names = ['vstinner', 'petr.viktorin', 'methane', 'lukasz.langa', 'hroncok', 'miss-islington', 'tarun.johar']
    pr_nums = ['21117', '27437', '27441']
    priority = 'normal'
    resolution = 'wont fix'
    stage = 'resolved'
    status = 'open'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue41103'
    versions = ['Python 3.10', 'Python 3.11']

    @methane
    Copy link
    Member Author

    @methane methane commented Jun 24, 2020

    https://docs.python.org/3/c-api/objbuffer.html
    Old buffer protocol has been deprecated since Python 3.0.
    It was useful to make transition from Python 2 easy.
    But it's time to remove.

    @methane
    Copy link
    Member Author

    @methane methane commented Jun 25, 2020

    New changeset 6f8a6ee by Inada Naoki in branch 'master':
    bpo-41103: Remove old buffer protocol support (bpo-21117)
    6f8a6ee

    @hroncok
    Copy link
    Mannequin

    @hroncok hroncok mannequin commented Nov 25, 2020

    I've just seen a fix of PyQt4 that basically copy pastes (some of) the removed code to PyQt4:

    https://src.fedoraproject.org/rpms/PyQt4/pull-request/2#request_diff

    What is the benefit of removing this? Is copy pasting the compatibility layer to (possibly many) different projects worth the "cleanup"?

    In Fedora, at least 7 packages are broken so far:

    PyQt4: https://bugzilla.redhat.com/show_bug.cgi?id=1895298
    libsolv: https://bugzilla.redhat.com/show_bug.cgi?id=1896411
    m2crypto: https://bugzilla.redhat.com/show_bug.cgi?id=1897148
    apsw: https://bugzilla.redhat.com/show_bug.cgi?id=1897500
    djvulibre: https://bugzilla.redhat.com/show_bug.cgi?id=1897558
    wsaccel: https://bugzilla.redhat.com/show_bug.cgi?id=1899550
    webassets: https://bugzilla.redhat.com/show_bug.cgi?id=1899555

    @vstinner
    Copy link
    Member

    @vstinner vstinner commented Nov 25, 2020

    PyObject_AsCharBuffer() is dangerous: it returns a dangling pointer by design. PyObject_GetBuffer() design is safer: the API ensures that the buffer remains valid until PyBuffer_Release() is called.

    PyQt5 was updated to use the safe PyObject_GetBuffer()/PyBuffer_Release(). PyQt4 is no longer updated, that's why I proposed a downstream fix which copy/paste the old code. Changing PyQt4 to use the safe API was not worth it, since it's complex to redesign the impacted function qpycore_encode().

    @methane
    Copy link
    Member Author

    @methane methane commented Nov 26, 2020

    Thank you for reporting. I removed these APIs to get such feedback as early as possible.

    We are free to revive these APIs if it breaks too much and some projects can not be fixed before Python 3.10 release.

    Some project maintainers ignore deprecations and wait compile errors to fix soemthing. (e.g. rogerbinns/apsw#288 (comment))
    That's why we need to break some projects during alpha phase.

    What is the benefit of removing this? Is copy pasting the compatibility layer to (possibly many) different projects worth the "cleanup"?

    Oh, no need to copy-paste compatibility layer for all projects. They can migrate to new APIs.

    @tarunjohar
    Copy link
    Mannequin

    @tarunjohar tarunjohar mannequin commented Jul 29, 2021

    Also filed under https://bugs.python.org/issue44609

    PEP-384 and PEP-652 define a stable ABI to be used with Python 3.2 and later. On Windows, symbols for the stable ABI are exported from the python3.dll shared library.

    The following functions are present in Python 3.9 but have been removed from Python 3.10b3:

    PyObject_AsCharBuffer()
    PyObject_AsReadBuffer()
    PyObject_AsWriteBuffer()
    PyObject_CheckReadBuffer()

    Without these functions, an extension cannot utilize the stable ABI to access the buffer memory of data structures. The buffer protocol is suggested as an alternative, but the buffer functions PyObject_GetBuffer() and PyBuffer_Release() are not present in the stable ABI.

    While these two functions may be added to the stable ABI, removal of the four functions above makes Python 3.10 incompatible with previous versions. It is requested that the four functions be reinstated and maintained as described in PEP-652.

    @methane
    Copy link
    Member Author

    @methane methane commented Jul 29, 2021

    Oh, I didn't notice that APIs deprecated in Python 3.0 are included in stable ABIs defined at Python 3.2!

    @methane
    Copy link
    Member Author

    @methane methane commented Jul 29, 2021

    Should I resurrect only function implementation and symbol?
    Or may I resurrect definitions in header files too?

    @methane methane reopened this Jul 29, 2021
    @methane methane reopened this Jul 29, 2021
    @ambv
    Copy link
    Contributor

    @ambv ambv commented Jul 29, 2021

    New changeset ce5e1a6 by Inada Naoki in branch 'main':
    bpo-41103: Resurrect the old buffer protocol. (GH-27437)
    ce5e1a6

    @ambv
    Copy link
    Contributor

    @ambv ambv commented Jul 29, 2021

    New changeset 6b922da by Miss Islington (bot) in branch '3.10':
    bpo-41103: Resurrect the old buffer protocol. (GH-27437) (GH-27441)
    6b922da

    @ambv
    Copy link
    Contributor

    @ambv ambv commented Jul 30, 2021

    Resolved as "wontfix" due to presence in the stable ABI.

    @ambv ambv closed this as completed Jul 30, 2021
    @ambv ambv closed this as completed Jul 30, 2021
    @encukou
    Copy link
    Member

    @encukou encukou commented Aug 5, 2021

    These should be removed from the *limited API*, but stay for the stable ABI.
    (PEP-652 mentions this can happen but doesn't give details, which will be a bit tricky. These are good candidates for figuring out the process.)

    I assigned this to myself; I'll try to get to it for 3.11.

    What is the benefit of removing this? Is copy pasting the compatibility layer to (possibly many) different projects worth the "cleanup"?

    Yes. PyObject_AsReadBuffer is dangerous *in general*, but in certain specific cases where you're relying on CPython implementations details it can work. For example, borrowing a buffer from a *string* or *array* will work on CPython, and I don't see that changing any time soon. So, using PyObject_AsReadBuffer is still tech debt, but for some projects that's OK.

    All this should be documented in the release notes, though, so the projects know what they're doing. (And the release notes should be written *with the change*, not later, so projects testing early can read them.)

    @encukou encukou reopened this Aug 5, 2021
    @encukou encukou reopened this Aug 5, 2021
    @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

    4 participants