Mailing List Archive

Miro Hron?ok <miro@hroncok.cz> added the comment:

Also Numba is broken: https://bugzilla.redhat.com/show_bug.cgi?id=2005686

----------

_______________________________________
Python tracker <report@bugs.python.org>
<https://bugs.python.org/issue43760>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

STINNER Victor <vstinner@python.org> added the comment:

> A Cython issue report: https://github.com/cython/cython/issues/4153

Cython 0.29.24 released at July 13, 2021 with a fix (2 commits):

* https://github.com/cython/cython/commit/be3b178296975b976107f41a6383291701e0297f
* https://github.com/cython/cython/commit/8d177f4aa51a663e8c51de3210ccb329d7629d36

The Cython master branch was fixed as well: see https://github.com/cython/cython/issues/4153

----------

_______________________________________
Python tracker <report@bugs.python.org>
<https://bugs.python.org/issue43760>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

Change by Erlend E. Aasland <erlend.aasland@innova.no>:


----------
nosy: +erlendaasland

_______________________________________
Python tracker <report@bugs.python.org>
<https://bugs.python.org/issue43760>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

Mark Shannon <mark@hotpy.org> added the comment:

IMO those failures are bugs in the projects listed not in CPython.


Relying on the exact meaning, or even the existence of an undocumented field of a C struct is not, nor ever has been, safe.
The user of the field is assuming a meaning that is not known to the developers of the library/application, so there is no way for them to preserve that meaning.
This is not specific to CPython, but applies to any C API.

The code in the examples given above are using `tstate->use_tracing` assuming that its meaning is to determine whether tracing is turned on or not.
However, it has no such meaning. It is an internal performance optimization to avoid checking both `tstate->c_tracefunc` and `tstate->c_profilefunc`.
It is `tstate->tracing` that determines whether tracing is active or not.

I propose adding back `tstate->use_tracing` as a copy of `tstate->cframe->us_tracing`. Any writes to `tstate->use_tracing` will be ignored, but any code that also sets `tstate->tracing`, as the Cython code does, will work correctly. Any code that reads `tstate->use_tracing` will work correctly.

I'm minded to prefix all the names of all fields in all C structs that happen to be included by Python.h with "if_you_use_this_then_we_will_break_your_code_in_some_way_that_will_ruin_your_reputation_as_a_library_developer__maybe_not_tomorrow_maybe_not_next_year_but_someday"
Although that might prove tricky with a 80 character line limit :)

My attempts to avoid this happening again next year, and the year after that, and...
https://bugs.python.org/issue45247
https://github.com/cython/cython/issues/4382

----------

_______________________________________
Python tracker <report@bugs.python.org>
<https://bugs.python.org/issue43760>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

Change by Mark Shannon <mark@hotpy.org>:


----------
pull_requests: +26872
pull_request: https://github.com/python/cpython/pull/28474

_______________________________________
Python tracker <report@bugs.python.org>
<https://bugs.python.org/issue43760>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

Change by Xtrem532 <Xtrem532.tg@gmail.com>:


----------
nosy: +Xtrem532

_______________________________________
Python tracker <report@bugs.python.org>
<https://bugs.python.org/issue43760>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

Stefan Behnel <stefan_ml@behnel.de> added the comment:

> The code in the examples given above are using `tstate->use_tracing` assuming that its meaning is to determine whether tracing is turned on or not.

No, actually not. It is using the field in the same way as CPython, simply because most of this code was originally copied from CPython, and we also copied the optimisation of avoiding to check the other fields (for the obvious reason of being an optimisation).


> I propose adding back `tstate->use_tracing` as a copy of `tstate->cframe->us_tracing`.

Cython 0.29.24 has already been adapted to this change and will use the new field in CPython 3.10b1+.


> Any code that reads `tstate->use_tracing` will work correctly.

Any code that reads and /writes/ the field would probably also continue to work correctly, which is what older Cython versions did.


> if_you_use_this_then_we_will_break_your_code_in_some_way_that_will_ruin_your_reputation_as_a_library_developer…

The thing is, new APIs can only be added to new CPython releases. Supporting features in older CPython versions (currently 2.7+) means that we always *have to* use the existing fields, and can only switch to new APIs by duplicating code based on a PY_VERSION_HEX preprocessor check. Even if a new low-latency profiling API was added in CPython 3.11, we'd have to wait until there is at least an alpha release that has it before enabling this code switch.

And if the new API proves to be slower, we may end up keeping the old code around and adding a C compile-time configuration option for users to enable (or disable) its use. Cython has lots of those these days, mostly to support the different C-API capabilities of different Python implementations, e.g. to take advantage of the PyLong or PyUnicode internals if available, and use generic C-API calls if not.

----------

_______________________________________
Python tracker <report@bugs.python.org>
<https://bugs.python.org/issue43760>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

John Ehresman <jpe@wingware.com> added the comment:

Is adding the field back an option at this point? It would mean that extensions compiled against the release candidates may not be binary compatible with the final release

My take is that use_tracing is an implementation and version dependent field, and that binary compatibility will be maintained for a specific release (e.g. 3.10) but that there's no assurance that it will be there in the next release -- though these things tend not to change. I also regard generated cython code as only being valid for the releases that a specific cython version supports.

Code and API's change slowly, but eventually they do change.

----------
nosy: +jpe

_______________________________________
Python tracker <report@bugs.python.org>
<https://bugs.python.org/issue43760>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

Miro Hron?ok <miro@hroncok.cz> added the comment:

> It would mean that extensions compiled against the release candidates may not be binary compatible with the final release

If that's true, I definitively argue not to do that. We've told everybody it won't happen.

----------

_______________________________________
Python tracker <report@bugs.python.org>
<https://bugs.python.org/issue43760>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

Change by Guido van Rossum <guido@python.org>:


----------
nosy: +gvanrossum

_______________________________________
Python tracker <report@bugs.python.org>
<https://bugs.python.org/issue43760>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

Change by Pablo Galindo Salgado <pablogsal@gmail.com>:


----------
pull_requests: +26893
pull_request: https://github.com/python/cpython/pull/28498

_______________________________________
Python tracker <report@bugs.python.org>
<https://bugs.python.org/issue43760>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

Pablo Galindo Salgado <pablogsal@gmail.com> added the comment:

>From PR 28498:

@vstinner @ambv The ABI is not broken, the only thing that this PR change is the size of the struct. All the offsets to the members are the same and therefore will be valid in any compiled code.

Any compiled wheels will still work. Look at the ABI report:

[C]'function void PyEval_AcquireThread(PyThreadState*)' at ceval.c:447:1 has some indirect sub-type changes:
parameter 1 of type 'PyThreadState*' has sub-type changes:
in pointed to type 'typedef PyThreadState' at pystate.h:20:1:
underlying type 'struct _ts' at pystate.h:62:1 changed:
type size changed from 2240 to 2304 (in bits)
1 data member insertion:
'int _ts::use_tracing', at offset 2240 (in bits) at pystate.h:151:1
1 data member changes (2 filtered):
type of 'PyInterpreterState* _ts::interp' changed:
in pointed to type 'typedef PyInterpreterState' at pystate.h:22:1:
underlying type 'struct _is' at pycore_interp.h:220:1 changed:
type size hasn't changed
1 data member changes (3 filtered):
type of '_PyFrameEvalFunction _is::eval_frame' changed:
underlying type 'PyObject* (PyThreadState*, PyFrameObject*, int)*' changed:
in pointed to type 'function type PyObject* (PyThreadState*, PyFrameObject*, int)':
parameter 1 of type 'PyThreadState*' has sub-type changes:
in pointed to type 'typedef PyThreadState' at pystate.h:20:1:
underlying type 'struct _ts' at pystate.h:62:1 changed:
type size changed from 2240 to 2304 (in bits)
1 data member insertion:
'int _ts::use_tracing', at offset 2240 (in bits) at pystate.h:151:1
1 data member changes (2 filtered):
type of '_ts* _ts::next' changed:
in pointed to type 'struct _ts' at pystate.h:62:1:
type size changed from 2240 to 2304 (in bits)
1 data member insertion:
'int _ts::use_tracing', at offset 2240 (in bits) at pystate.h:151:1
no data member changes (2 filtered);




[C]'function PyThreadState* PyGILState_GetThisThreadState()' at pystate.c:1455:1 has some indirect sub-type changes:
return type changed:
in pointed to type 'typedef PyThreadState' at pystate.h:20:1:
underlying type 'struct _ts' at pystate.h:62:1 changed:
type size changed from 2240 to 2304 (in bits)
1 data member insertion:
'int _ts::use_tracing', at offset 2240 (in bits) at pystate.h:151:1
no data member changes (4 filtered);

[C]'function int _PyErr_CheckSignalsTstate(PyThreadState*)' at signalmodule.c:1767:1 has some indirect sub-type changes:
parameter 1 of type 'PyThreadState*' has sub-type changes:
in pointed to type 'typedef PyThreadState' at pystate.h:20:1:
underlying type 'struct _ts' at pystate.h:62:1 changed:
type size changed from 2240 to 2304 (in bits)
1 data member insertion:
'int _ts::use_tracing', at offset 2240 (in bits) at pystate.h:151:1
no data member changes (3 filtered);

[C]'function void _PyErr_Clear(PyThreadState*)' at errors.c:453:1 has some indirect sub-type changes:
parameter 1 of type 'PyThreadState*' has sub-type changes:
in pointed to type 'typedef PyThreadState' at pystate.h:20:1:
underlying type 'struct _ts' at pystate.h:62:1 changed:
type size changed from 2240 to 2304 (in bits)
1 data member insertion:
'int _ts::use_tracing', at offset 2240 (in bits) at pystate.h:151:1
no data member changes (3 filtered);
As you can see, the leaves of the change is only type size changed from 2240 to 2304. As the member is added

----------

_______________________________________
Python tracker <report@bugs.python.org>
<https://bugs.python.org/issue43760>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

Pablo Galindo Salgado <pablogsal@gmail.com> added the comment:

Also, just to clarify, I also opened PR 28498 to discuss the possibility of going ahead, I still don't want to move on without consensus.

----------

_______________________________________
Python tracker <report@bugs.python.org>
<https://bugs.python.org/issue43760>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

Pablo Galindo Salgado <pablogsal@gmail.com> added the comment:

Also, I personally thing there is absolutely no guarantee that Cython code generated for 3.9 should work for 3.10 and the thread state is a private structure that has undocumented fields and is not part of the stable API nor the limited API so, tstate->tracing disappearing is totally withing the guarantees between Python versions.

----------

_______________________________________
Python tracker <report@bugs.python.org>
<https://bugs.python.org/issue43760>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

Pablo Galindo Salgado <pablogsal@gmail.com> added the comment:

I discussed this particular instance with the Steering Council and the conclusion was that this field (use_tracing) is considered an implementation detail and therefore its removal it's justified so we won't be restoring it.

I'm therefore closing PR28498

Notice that this decision only affects this particular issue and should not be generalized to other fields or structures. We will try to determine and open a discusion in the future about what is considered public/private in these ambiguous cases and what can users expect regarding stability and backwards compatibility.

----------

_______________________________________
Python tracker <report@bugs.python.org>
<https://bugs.python.org/issue43760>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

Pablo Galindo Salgado <pablogsal@gmail.com> added the comment:

I'm removing the release blocker as per above, feel free to close of there is nothing else to discuss or act on here.

----------
priority: release blocker ->

_______________________________________
Python tracker <report@bugs.python.org>
<https://bugs.python.org/issue43760>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

Petr Viktorin <encukou@gmail.com> added the comment:

> The ABI is not broken, the only thing that this PR change is the size of the struct. All the offsets to the members are the same and therefore will be valid in any compiled code.

I'll just note that a change in struct size does technically break ABI, since *arrays* of PyThreadState will break.

So the size shouldn't be changed in RCs or point releases. (But since it's not part of stable ABI, it was OK to change it for 3.10.)

> We will try to determine and open a discussion in the future about what is considered public/private in these ambiguous cases and what can users expect regarding stability and backwards compatibility.

Please keep me in the loop; I'm working on summarizing my understanding of this (in a form that can be added to the docs if approved).

----------

_______________________________________
Python tracker <report@bugs.python.org>
<https://bugs.python.org/issue43760>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

Pablo Galindo Salgado <pablogsal@gmail.com> added the comment:

> I'll just note that a change in struct size does technically break ABI, since *arrays* of PyThreadState will break.

Not that matters now because we are not proceeding but just to clarify why I deemed this acceptable: arrays of PyThreadState is extremelly unlikely in extensions because we pass it by Pointer and is always manipulated by pointer. To place it in an array you either need to create one or copy one into an array, which I cannot see what would be the point because the fields are mainly pointers that would become useless as the interpreter will not update anything

----------

_______________________________________
Python tracker <report@bugs.python.org>
<https://bugs.python.org/issue43760>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

Pablo Galindo Salgado <pablogsal@gmail.com> added the comment:

Also, I checked the DWARF tree of all existing wheels for 3.10 on PyPI (there aren't many) and none had anything that uses the size of the struct.

----------

_______________________________________
Python tracker <report@bugs.python.org>
<https://bugs.python.org/issue43760>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

Change by STINNER Victor <vstinner@python.org>:


----------
pull_requests: +26918
pull_request: https://github.com/python/cpython/pull/28527

_______________________________________
Python tracker <report@bugs.python.org>
<https://bugs.python.org/issue43760>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

STINNER Victor <vstinner@python.org> added the comment:

I created PR 28527 to document PyThreadState.use_tracing removal and explain how to port existing code to Python 3.10.

----------

_______________________________________
Python tracker <report@bugs.python.org>
<https://bugs.python.org/issue43760>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

STINNER Victor <vstinner@python.org> added the comment:


New changeset f4ccb79d52ee726d58bbb038ea98b4deec52001d by Victor Stinner in branch 'main':
bpo-43760: Document PyThreadState.use_tracing removal (GH-28527)
https://github.com/python/cpython/commit/f4ccb79d52ee726d58bbb038ea98b4deec52001d


----------

_______________________________________
Python tracker <report@bugs.python.org>
<https://bugs.python.org/issue43760>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

Change by miss-islington <mariatta.wijaya+miss-islington@gmail.com>:


----------
nosy: +miss-islington
nosy_count: 11.0 -> 12.0
pull_requests: +26919
pull_request: https://github.com/python/cpython/pull/28529

_______________________________________
Python tracker <report@bugs.python.org>
<https://bugs.python.org/issue43760>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

?ukasz Langa <lukasz@langa.pl> added the comment:


New changeset 55576893b31452ba739e01189424cd62cf11ed20 by Miss Islington (bot) in branch '3.10':
bpo-43760: Document PyThreadState.use_tracing removal (GH-28527) (GH-28529)
https://github.com/python/cpython/commit/55576893b31452ba739e01189424cd62cf11ed20


----------
nosy: +lukasz.langa

_______________________________________
Python tracker <report@bugs.python.org>
<https://bugs.python.org/issue43760>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com