PySlice_GetIndicesEx change broke ABI in 3.5 and 3.6 branches

[njsmith]

BPO	29943
Nosy	@arigo, @mdickinson, @larryhastings, @benjaminp, @ned-deily, @njsmith, @skrah, @serhiy-storchaka, @zooba, @MojoVampire, @stratakis, @hroncok, @jjhelmus
PRs	[3.5] bpo-29943: Do not replace the function PySlice_GetIndicesEx() with a macro #1049 [2.7] bpo-29943: Remove the PySlice_GetIndicesEx() macro. #1050 [3.6] bpo-29943: Do not replace the function PySlice_GetIndicesEx() with a macro (GH-1049) #1813 bpo-30604: Move co_extra_freefuncs to interpreter state to avoid crashes in threads #2015

^{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/serhiy-storchaka'
closed_at = <Date 2017-05-25.12:34:20.260>
created_at = <Date 2017-03-29.23:36:28.041>
labels = ['interpreter-core', 'type-bug', 'release-blocker']
title = 'PySlice_GetIndicesEx change broke ABI in 3.5 and 3.6 branches'
updated_at = <Date 2017-09-27.16:19:36.041>
user = 'https://github.com/njsmith'

bugs.python.org fields:

activity = <Date 2017-09-27.16:19:36.041>
actor = 'josh.r'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2017-05-25.12:34:20.260>
closer = 'serhiy.storchaka'
components = ['Interpreter Core']
creation = <Date 2017-03-29.23:36:28.041>
creator = 'njs'
dependencies = []
files = []
hgrepos = []
issue_num = 29943
keywords = []
message_count = 27.0
messages = ['290796', '290797', '290806', '290815', '291118', '291331', '291406', '291408', '291743', '291746', '291979', '292276', '292543', '292544', '292715', '292716', '292718', '292726', '292727', '293434', '293452', '294367', '294390', '294474', '295481', '295487', '303101']
nosy_count = 14.0
nosy_names = ['arigo', 'mark.dickinson', 'larry', 'benjamin.peterson', 'ned.deily', 'njs', 'cgohlke', 'skrah', 'serhiy.storchaka', 'steve.dower', 'josh.r', 'cstratak', 'hroncok', 'jhelmus']
pr_nums = ['1049', '1050', '1813', '2015']
priority = 'release blocker'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue29943'
versions = ['Python 2.7', 'Python 3.5', 'Python 3.6']

[njsmith]

In the process of fixing bpo-27867, a new function PySlice_AdjustIndices was added, and PySlice_GetIndicesEx was converted into a macro that calls this new function. The patch was backported to both the 3.5 and 3.6 branches, was released in 3.6.1, and is currently slated to be released as part of 3.5.4.

Unfortunately, this breaks our normal ABI stability guarantees for micro releases: it means that if a C module that uses PySlice_GetIndicesEx is compiled against e.g. 3.6.1, then it cannot be imported on 3.6.0. This affects a number of high-profile packages (cffi, pandas, mpi4py, dnf, ...). The only workaround is that if you are distributing binary extension modules (e.g. wheels), then you need to be careful not to upgrade to 3.6.1. It's not possible for a wheel to declare that it requires 3.6.1-or-better, because CPython normally follows the rule that we don't make these kinds of changes. Oops.

CC'ing Ned and Larry, because it's possible this should trigger a 3.6.2, and I think it's a blocker for 3.5.4. CC'ing Serhiy as the author of the original patch, since you probably have the best idea how this could be unwound with minimal breakage :-).

python-dev discussion: https://mail.python.org/pipermail/python-dev/2017-March/147707.html

Fedora bug: https://bugzilla.redhat.com/show_bug.cgi?id=1435135

[larryhastings]

Let's make it a release blocker for now.

[serhiy-storchaka]

My apologies for breaking the world.

The workaround is to undefine PySlice_GetIndicesEx after #include "Python.h".

#if PY_VERSION_HEX < 0x03070000 && defined(PySlice_GetIndicesEx)
#undef PySlice_GetIndicesEx
#endif

But this restores the initial bug.

Other obvious solution -- declare 3.6.0 broken and require upgrading to 3.6.1.

For 3.5.4 we can disable defining the macro when Py_LIMITED_API is not defined. This will restore the initial bug in many extensions. And we need other way to detect if we compile the CPython core or standard extensions for fixing the bug in standard collections. Or expand all uses of PySlice_GetIndicesEx to PySlice_Unpack+PySlice_AdjustIndices (the patch can be generated automatically).

[serhiy-storchaka]

2.7 is affected too.

Nathaniel, could you add references to this issue and to original bpo-27867 on the Fedora bug tracker?

[stratakis]

Currently we haven't updated to Python 3.6.1 at Fedora 26 due to this issue.

While it is a release blocker for 3.6.2, what can be done for 3.6.1?

[serhiy-storchaka]

PR 1049 and PR 1050 restore ABI compatibility in 3.5 and 2.7. Unfortunately this restores the original bug with using PySlice_GetIndicesEx in third-party code (but CPython core and standard extensions no longer use PySlice_GetIndicesEx).

Can we consider 3.6.0 rather than 3.6.1 as broken release?

[njsmith]

Can we consider 3.6.0 rather than 3.6.1 as broken release?

In the last week, pypi downloads were about evenly split between 3.6.0 and 3.6.1 (2269969 for "3.6.1", 1927189 for "3.6.0", and those two were ~2 orders of magnitude more common than other strings like "3.6.1+", "3.6.0b2", etc. [1]). Not sure what that to conclude from that, but certainly if people start uploading 3.6.1-only wheels right now then it will break things for a lot of end users.

With my manylinux docker image maintainer hat on: we're currently shipping 3.6.0. I'm extremely confident that if we stick with this we'll never get any complaints about the obscure bug with malicious __index__ implementations that's being fixed here. OTOH if we upgrade to 3.6.1, or any version with this ABI change, then we'll definitely get many complaints so long as there's anyone at all still using 3.6.0, which is probably forever. So I'm not sure not sure what incentive we would have to ever upgrade to 3.6.1+ if this ABI change is kept?

(This isn't saying the bug is unimportant! But it sure is hard to sell its importance to folks trying to ship packages and support end-users...)

--------

[1] Somewhat crude query I used in case it's useful for future reference:

SELECT
REGEXP_EXTRACT(details.python, r"^([^\\.]+\.[^\\.]+\.[^\\.]+)") as python_version,
COUNT(*) as download_count,
FROM
TABLE_DATE_RANGE(
[the-psf:pypi.downloads],
DATE_ADD(CURRENT_TIMESTAMP(), -7, "day"),
DATE_ADD(CURRENT_TIMESTAMP(), 0, "day")
)
WHERE
REGEXP_MATCH(details.python, r"^3\.6\.")
GROUP BY
python_version,
ORDER BY
download_count DESC
LIMIT 100

[njsmith]

It looks like PyTorch got bitten by this: https://discuss.pytorch.org/t/pyslice-adjustindices-error/1475/11

[serhiy-storchaka]

New changeset 89f9eb5 by Serhiy Storchaka in branch '2.7':
bpo-29943: Remove the PySlice_GetIndicesEx() macro. (bpo-1050)
89f9eb5

[serhiy-storchaka]

New changeset 49a9059 by Serhiy Storchaka in branch '3.5':
[3.5] bpo-29943: Do not replace the function PySlice_GetIndicesEx() with a macro (bpo-1049)
49a9059

[njsmith]

FYI: pandas-dev/pandas#16066

[njsmith]

Apparently this also broke pyqt for multiple users; here's the maintainers at conda-forge struggling to figure out the best workaround: conda-forge/pyqt-feedstock#25

I really think it would be good to fix this in 3.6 sooner rather than later. Downstairs projects are accumulating workarounds and pinning 3.6.0 as we speak.