View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0000901 | LDMud 3.6 | Implementation | public | 2022-01-02 20:18 | 2022-01-06 12:09 |
| Reporter | paradox | Assigned To | zesstra | ||
| Priority | normal | Severity | minor | Reproducibility | always |
| Status | closed | Resolution | no change required | ||
| Platform | Linux | OS | NixOS | OS Version | 21.11 |
| Product Version | 3.6.4 | ||||
| Summary | 0000901: Python efun function type enforcement broken with Python 3.10 | ||||
| Description | When linking LD 3.6.4 against various Python versions I noticed that the `t-python` unit tests start to fail when using Python 3.10, but work with 3.7, 3.8 and 3.9. Specifically, the "Running Test passing invalid arguments 1..." and "Running Test passing invalid arguments 2..." unit tests fail while the others pass. Digging into the problem it appears that the C API function `PyFunction_GetAnnotations` has changed between Python 3.9 and 3.10. In my testing on 3.9 this function consistently returned a `PyDict_Type` of the form `{"result": <some type>, "someArgName": <some type> ...}`. This made it possible on L447 (https://github.com/ldmud/ldmud/blob/264d845769cd8b997e0a4abf3ea61a7daafe90db/src/pkg-python.c#L447) and L472 (https://github.com/ldmud/ldmud/blob/264d845769cd8b997e0a4abf3ea61a7daafe90db/src/pkg-python.c#L472) to look up types by name in the dict using `PyObject_GetItem`. On 3.10 this function is now returning a `PyTuple_Type` of the form `( "result", <some type>, "someArgName", <some type>, ...)`. In this case calling `PyObject_GetItem` to look up an item by a string key returns nothing, and so the efun is registered without type information. Subsequently the unit tests that check efun type enforcement works correctly begin to fail. I've worked up a fix that handles this case by iterating the tuple as a generic sequence, finding the type information positionaly based on a matching key's index. In my testing this fixes the unit tests with Python 3.10 and has no adverse affect on builds linking Python 3.9. I will open a Github PR shortly with my fix. There might be a better fix someone more familiar with the C API can dream up :-) | ||||
| Steps To Reproduce | 1. Build LDMud with Python support, and specifically Python 3.10+ 2. `cd test && ./run.sh t-python` 3. Observe the two invalid arguments tests fail. 4. Build LDMud with Python support, and specifically Python 3.9 or lower. 5. `cd test && ./run.sh t-python` 6. Observe the two invalid argument tests pass. | ||||
| Tags | No tags attached. | ||||
|
|
As promised: here's a pull request implementing one potential solution: https://github.com/ldmud/ldmud/pull/68 I've tried my best to match driver style and would be generally interested in feedback on where I could improve! Thanks folks. |
|
|
The biggest open question with this issue (and how to handle the fix) is whether this is a Python regression or an intended API change. Digging around in the changelog it looks possibly related to an optimization made in 3.10 described in the What's New Doc (https://github.com/python/cpython/blob/main/Doc/whatsnew/3.10.rst): > When using stringized annotations, annotations dicts for functions are no longer created when the function is created. Instead, they are stored as a tuple of strings, and the function object lazily converts this into the annotations dict on demand. This optimization cuts the CPU time needed to define an annotated function by half. (Contributed by Yurii Karabas and Inada Naoki in :issue:`42202`) Implemented in https://github.com/python/cpython/commit/7301979b23406220510dd2c7934a21b41b647119 Associated bug: https://bugs.python.org/issue42202 I'm going to try and find the right cpython mailing list to ask if this is a regression. |
|
|
I put together a smaller reproduction demo to use to report upstream: https://github.com/cpu/pyfunction_getannotations_test |
|
|
Here's the upstream bug I opened: https://bugs.python.org/issue46236 |
|
|
Thank you! :-) I am curious what the result will be from the python community. |
|
|
This was confirmed as a regression and fixed upstream by the CPython developers. I'll close the associated PR since it won't be required once a new point release of the 3.10 series is available. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2022-01-02 20:18 | paradox | New Issue | |
| 2022-01-02 20:37 | paradox | Note Added: 0002667 | |
| 2022-01-02 21:27 | paradox | Note Added: 0002668 | |
| 2022-01-03 03:06 | paradox | Note Added: 0002669 | |
| 2022-01-03 03:18 | paradox | Note Added: 0002670 | |
| 2022-01-03 09:09 | zesstra | Note Added: 0002671 | |
| 2022-01-05 22:59 | paradox | Note Added: 0002672 | |
| 2022-01-06 12:09 | zesstra | Assigned To | => zesstra |
| 2022-01-06 12:09 | zesstra | Status | new => closed |
| 2022-01-06 12:09 | zesstra | Resolution | open => no change required |
