Adapt the CPython 3.9+ C-API usage to use newer macros #864

iemelyanov · 2023-07-21T23:02:10Z

This patch replaces the C-APIs that have been deprecated in Python 3.9 and later removed in 3.13.

What do these changes do?

Fix usage Python C API for version greater than 3.9
Fix segfault which I catch here https://github.com/aio-libs/multidict/blob/master/multidict/_multidict.c#L783

Are there changes in behavior for the user?

No

Related issue number

Fixes #862

codecov · 2023-07-22T00:08:28Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d2959d0) 93.65% compared to head (7eedf67) 93.65%.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #864   +/-   ##
=======================================
  Coverage   93.65%   93.65%           
=======================================
  Files           5        5           
  Lines         504      504           
=======================================
  Hits          472      472           
  Misses         32       32

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

hugovk · 2023-08-30T20:40:26Z

Thanks for the PR, I've confirmed there's a segmentation fault on all three OS on the CI for Python 3.12 (https://github.com/hugovk/multidict/actions/runs/6028022143), and the commits from this PR fixes it (https://github.com/hugovk/multidict/actions/runs/6029781394).

See also #877 (comment).

serhiy-storchaka · 2023-08-31T07:28:42Z

multidict/_multilib/istr.h

@@ -43,7 +43,11 @@ istr_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
    if (!ret) {
        goto fail;
    }
+#if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 9
+    s = PyObject_CallMethod(ret, "lower", NULL);


I hope it is not performance-sensitive code. Otherwise it it would be better to use a cached value.

Please look last changes, I moved multidict_str_lower to global vars and use it here and in pair_list

serhiy-storchaka · 2023-08-31T07:29:58Z

multidict/_multilib/pair_list.h

 {
+    PyObject *str_lower = PyUnicode_InternFromString("lower");


Rather of caching string object "lower", it may be better to cache method object str.lower.

str.lower can't work with CallMethod

serhiy-storchaka · 2023-08-31T07:31:23Z

multidict/_multidict.c

@@ -777,9 +785,12 @@ multidict_add(MultiDictObject *self, PyObject *const *args,
        return NULL;
    }
 #else
-    static _PyArg_Parser _parser = {NULL, _keywords, "add", 0};
+    static _PyArg_Parser _parser = {


Why this method needs keyword parameters at all?

We have a test for it https://github.com/aio-libs/multidict/blob/master/tests/test_mutable_multidict.py#L333

But why does such test exist at first place?

_PyArg_Parser is a deep internal tool, it is not purposed to be used outside of CPython. It is expected that the third-party code uses good old and slow PyArg_ParseTupleAndKeywords for parsing keyword arguments, or handle it by hand-made code.

It is out of the scope of this issue, but I would consider deprecating accepting keyword arguments in this function.

This can be done in another PR

h-vetinari · 2023-10-02T02:21:44Z

Any news on this? We're thinking about backporting this patch in conda-forge to the last released, in order to unbreak things. It's always preferable though if the respective patches are merged upstream first.

JeanChristopheMorinPerso · 2023-11-24T21:37:36Z

We also backported this PR in AnacondaRecipes/multidict-feedstock#7

stonebig · 2023-12-17T10:32:00Z

merging pulls 828, 829, 864 reduce quite a bit the compiling complains on windows

tacaswell · 2023-12-21T17:11:17Z

This is still failing to build with py313 (Python 3.13.0a2+)

      gcc -fno-strict-overflow -Wsign-compare -DNDEBUG -g -O3 -Wall -DCYTHON_FAST_THREAD_STATE=0 -fPIC -I/home/tcaswell/.virtualenvs/cp_main/include -I/home/tcaswell/.pybuild/cp_main/include/python3.13 -c multidict/_multidict.c -o build/temp.linux-x86_64-cpython-313/multidict/_multidict.o -O2 -std=c99 -Wall -Wsign-compare -Wconversion -fno-strict-aliasing -pedantic
      In file included from /home/tcaswell/.pybuild/cp_main/include/python3.13/Python.h:121,
                       from multidict/_multidict.c:1:
      /home/tcaswell/.pybuild/cp_main/include/python3.13/cpython/optimizer.h:53:3: warning: redefinition of typedef ‘_PyOptimizerObject’ [-Wpedantic]
         53 | } _PyOptimizerObject;
            |   ^~~~~~~~~~~~~~~~~~
      /home/tcaswell/.pybuild/cp_main/include/python3.13/cpython/optimizer.h:40:35: note: previous declaration of ‘_PyOptimizerObject’ with type ‘_PyOptimizerObject’
         40 | typedef struct _PyOptimizerObject _PyOptimizerObject;
            |                                   ^~~~~~~~~~~~~~~~~~
      multidict/_multidict.c: In function ‘multidict_getall’:
      multidict/_multidict.c:452:12: error: unknown type name ‘_PyArg_Parser’
        452 |     static _PyArg_Parser _parser = {"O|O:getall", _keywords, 0};
            |            ^~~~~~~~~~~~~
      multidict/_multidict.c:452:37: warning: initialization of ‘int’ from ‘char *’ makes integer from pointer without a cast [-Wint-conversion]
        452 |     static _PyArg_Parser _parser = {"O|O:getall", _keywords, 0};
            |                                     ^~~~~~~~~~~~
      multidict/_multidict.c:452:37: note: (near initialization for ‘_parser’)
      multidict/_multidict.c:452:37: error: initializer element is not computable at load time
      multidict/_multidict.c:452:37: note: (near initialization for ‘_parser’)
      multidict/_multidict.c:452:51: warning: excess elements in scalar initializer
        452 |     static _PyArg_Parser _parser = {"O|O:getall", _keywords, 0};
            |                                                   ^~~~~~~~~
      multidict/_multidict.c:452:51: note: (near initialization for ‘_parser’)
      multidict/_multidict.c:452:62: warning: excess elements in scalar initializer
        452 |     static _PyArg_Parser _parser = {"O|O:getall", _keywords, 0};
            |                                                              ^
      multidict/_multidict.c:452:62: note: (near initialization for ‘_parser’)
      multidict/_multidict.c:453:10: warning: implicit declaration of function ‘_PyArg_ParseStackAndKeywords’; did you mean ‘PyArg_ParseTupleAndKeywords’? [-Wimplicit-function-declaration]
        453 |     if (!_PyArg_ParseStackAndKeywords(args, nargs, kwnames, &_parser,
            |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
            |          PyArg_ParseTupleAndKeywords
      multidict/_multidict.c: In function ‘multidict_getone’:
      multidict/_multidict.c:497:12: error: unknown type name ‘_PyArg_Parser’
        497 |     static _PyArg_Parser _parser = {"O|O:getone", _keywords, 0};
            |            ^~~~~~~~~~~~~
      multidict/_multidict.c:497:37: warning: initialization of ‘int’ from ‘char *’ makes integer from pointer without a cast [-Wint-conversion]
        497 |     static _PyArg_Parser _parser = {"O|O:getone", _keywords, 0};
            |                                     ^~~~~~~~~~~~
      multidict/_multidict.c:497:37: note: (near initialization for ‘_parser’)
      multidict/_multidict.c:497:37: error: initializer element is not computable at load time
      multidict/_multidict.c:497:37: note: (near initialization for ‘_parser’)
      multidict/_multidict.c:497:51: warning: excess elements in scalar initializer
        497 |     static _PyArg_Parser _parser = {"O|O:getone", _keywords, 0};
            |                                                   ^~~~~~~~~
      multidict/_multidict.c:497:51: note: (near initialization for ‘_parser’)
      multidict/_multidict.c:497:62: warning: excess elements in scalar initializer
        497 |     static _PyArg_Parser _parser = {"O|O:getone", _keywords, 0};
            |                                                              ^
      multidict/_multidict.c:497:62: note: (near initialization for ‘_parser’)
      multidict/_multidict.c: In function ‘multidict_get’:
      multidict/_multidict.c:532:12: error: unknown type name ‘_PyArg_Parser’
        532 |     static _PyArg_Parser _parser = {"O|O:get", _keywords, 0};
            |            ^~~~~~~~~~~~~
      multidict/_multidict.c:532:37: warning: initialization of ‘int’ from ‘char *’ makes integer from pointer without a cast [-Wint-conversion]
        532 |     static _PyArg_Parser _parser = {"O|O:get", _keywords, 0};
            |                                     ^~~~~~~~~
      multidict/_multidict.c:532:37: note: (near initialization for ‘_parser’)
      multidict/_multidict.c:532:37: error: initializer element is not computable at load time
      multidict/_multidict.c:532:37: note: (near initialization for ‘_parser’)
      multidict/_multidict.c:532:48: warning: excess elements in scalar initializer
        532 |     static _PyArg_Parser _parser = {"O|O:get", _keywords, 0};
            |                                                ^~~~~~~~~
      multidict/_multidict.c:532:48: note: (near initialization for ‘_parser’)
      multidict/_multidict.c:532:59: warning: excess elements in scalar initializer
        532 |     static _PyArg_Parser _parser = {"O|O:get", _keywords, 0};
            |                                                           ^
      multidict/_multidict.c:532:59: note: (near initialization for ‘_parser’)
      multidict/_multidict.c: In function ‘multidict_add’:
      multidict/_multidict.c:782:12: error: unknown type name ‘_PyArg_Parser’
        782 |     static _PyArg_Parser _parser = {"OO:add", _keywords, 0};
            |            ^~~~~~~~~~~~~
      multidict/_multidict.c:782:37: warning: initialization of ‘int’ from ‘char *’ makes integer from pointer without a cast [-Wint-conversion]
        782 |     static _PyArg_Parser _parser = {"OO:add", _keywords, 0};
            |                                     ^~~~~~~~
      multidict/_multidict.c:782:37: note: (near initialization for ‘_parser’)
      multidict/_multidict.c:782:37: error: initializer element is not computable at load time
      multidict/_multidict.c:782:37: note: (near initialization for ‘_parser’)
      multidict/_multidict.c:782:47: warning: excess elements in scalar initializer
        782 |     static _PyArg_Parser _parser = {"OO:add", _keywords, 0};
            |                                               ^~~~~~~~~
      multidict/_multidict.c:782:47: note: (near initialization for ‘_parser’)
      multidict/_multidict.c:782:58: warning: excess elements in scalar initializer
        782 |     static _PyArg_Parser _parser = {"OO:add", _keywords, 0};
            |                                                          ^
      multidict/_multidict.c:782:58: note: (near initialization for ‘_parser’)
      multidict/_multidict.c: In function ‘multidict_setdefault’:
      multidict/_multidict.c:844:12: error: unknown type name ‘_PyArg_Parser’
        844 |     static _PyArg_Parser _parser = {"O|O:setdefault", _keywords, 0};
            |            ^~~~~~~~~~~~~
      multidict/_multidict.c:844:37: warning: initialization of ‘int’ from ‘char *’ makes integer from pointer without a cast [-Wint-conversion]
        844 |     static _PyArg_Parser _parser = {"O|O:setdefault", _keywords, 0};
            |                                     ^~~~~~~~~~~~~~~~
      multidict/_multidict.c:844:37: note: (near initialization for ‘_parser’)
      multidict/_multidict.c:844:37: error: initializer element is not computable at load time
      multidict/_multidict.c:844:37: note: (near initialization for ‘_parser’)
      multidict/_multidict.c:844:55: warning: excess elements in scalar initializer
        844 |     static _PyArg_Parser _parser = {"O|O:setdefault", _keywords, 0};
            |                                                       ^~~~~~~~~
      multidict/_multidict.c:844:55: note: (near initialization for ‘_parser’)
      multidict/_multidict.c:844:66: warning: excess elements in scalar initializer
        844 |     static _PyArg_Parser _parser = {"O|O:setdefault", _keywords, 0};
            |                                                                  ^
      multidict/_multidict.c:844:66: note: (near initialization for ‘_parser’)
      multidict/_multidict.c: In function ‘multidict_popone’:
      multidict/_multidict.c:880:12: error: unknown type name ‘_PyArg_Parser’
        880 |     static _PyArg_Parser _parser = {"O|O:popone", _keywords, 0};
            |            ^~~~~~~~~~~~~
      multidict/_multidict.c:880:37: warning: initialization of ‘int’ from ‘char *’ makes integer from pointer without a cast [-Wint-conversion]
        880 |     static _PyArg_Parser _parser = {"O|O:popone", _keywords, 0};
            |                                     ^~~~~~~~~~~~
      multidict/_multidict.c:880:37: note: (near initialization for ‘_parser’)
      multidict/_multidict.c:880:37: error: initializer element is not computable at load time
      multidict/_multidict.c:880:37: note: (near initialization for ‘_parser’)
      multidict/_multidict.c:880:51: warning: excess elements in scalar initializer
        880 |     static _PyArg_Parser _parser = {"O|O:popone", _keywords, 0};
            |                                                   ^~~~~~~~~
      multidict/_multidict.c:880:51: note: (near initialization for ‘_parser’)
      multidict/_multidict.c:880:62: warning: excess elements in scalar initializer
        880 |     static _PyArg_Parser _parser = {"O|O:popone", _keywords, 0};
            |                                                              ^
      multidict/_multidict.c:880:62: note: (near initialization for ‘_parser’)
      multidict/_multidict.c: In function ‘multidict_pop’:
      multidict/_multidict.c:927:12: error: unknown type name ‘_PyArg_Parser’
        927 |     static _PyArg_Parser _parser = {"O|O:pop", _keywords, 0};
            |            ^~~~~~~~~~~~~
      multidict/_multidict.c:927:37: warning: initialization of ‘int’ from ‘char *’ makes integer from pointer without a cast [-Wint-conversion]
        927 |     static _PyArg_Parser _parser = {"O|O:pop", _keywords, 0};
            |                                     ^~~~~~~~~
      multidict/_multidict.c:927:37: note: (near initialization for ‘_parser’)
      multidict/_multidict.c:927:37: error: initializer element is not computable at load time
      multidict/_multidict.c:927:37: note: (near initialization for ‘_parser’)
      multidict/_multidict.c:927:48: warning: excess elements in scalar initializer
        927 |     static _PyArg_Parser _parser = {"O|O:pop", _keywords, 0};
            |                                                ^~~~~~~~~
      multidict/_multidict.c:927:48: note: (near initialization for ‘_parser’)
      multidict/_multidict.c:927:59: warning: excess elements in scalar initializer
        927 |     static _PyArg_Parser _parser = {"O|O:pop", _keywords, 0};
            |                                                           ^
      multidict/_multidict.c:927:59: note: (near initialization for ‘_parser’)
      multidict/_multidict.c: In function ‘multidict_popall’:
      multidict/_multidict.c:975:12: error: unknown type name ‘_PyArg_Parser’
        975 |     static _PyArg_Parser _parser = {"O|O:popall", _keywords, 0};
            |            ^~~~~~~~~~~~~
      multidict/_multidict.c:975:37: warning: initialization of ‘int’ from ‘char *’ makes integer from pointer without a cast [-Wint-conversion]
        975 |     static _PyArg_Parser _parser = {"O|O:popall", _keywords, 0};
            |                                     ^~~~~~~~~~~~
      multidict/_multidict.c:975:37: note: (near initialization for ‘_parser’)
      multidict/_multidict.c:975:37: error: initializer element is not computable at load time
      multidict/_multidict.c:975:37: note: (near initialization for ‘_parser’)
      multidict/_multidict.c:975:51: warning: excess elements in scalar initializer
        975 |     static _PyArg_Parser _parser = {"O|O:popall", _keywords, 0};
            |                                                   ^~~~~~~~~
      multidict/_multidict.c:975:51: note: (near initialization for ‘_parser’)
      multidict/_multidict.c:975:62: warning: excess elements in scalar initializer
        975 |     static _PyArg_Parser _parser = {"O|O:popall", _keywords, 0};
            |                                                              ^
      multidict/_multidict.c:975:62: note: (near initialization for ‘_parser’)
      error: command '/lib/ccache/bin/gcc' failed with exit code 1
      [end of output]

multidict/_multidict.c

iemelyanov requested a review from asvetlov as a code owner July 21, 2023 23:02

hugovk mentioned this pull request Aug 30, 2023

Add support for Python 3.12 #877

Merged

4 tasks

serhiy-storchaka reviewed Aug 31, 2023

View reviewed changes

h-vetinari mentioned this pull request Sep 24, 2023

Rebuild for python312 conda-forge/multidict-feedstock#50

Merged

stonebig mentioned this pull request Dec 17, 2023

Release 2024-01 follow-up winpython/winpython#1274

Closed

4 tasks

Jamim mentioned this pull request Dec 26, 2023

Build wheels for Python 3.12 #887

Closed

webknjaz requested a review from serhiy-storchaka December 26, 2023 04:11

webknjaz reviewed Dec 26, 2023

View reviewed changes

multidict/_multidict.c Show resolved Hide resolved

webknjaz reviewed Dec 26, 2023

View reviewed changes

multidict/_multidict.c Show resolved Hide resolved

webknjaz changed the title ~~Fix usage python C API for version greater 3.9 and fix segfault~~ Adapt the CPython 3.9+ C-API usage to use newer macros Dec 26, 2023

webknjaz added a commit to iemelyanov/multidict that referenced this pull request Dec 26, 2023

Add a change note for PR aio-libs#864

0ba9381

psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Dec 26, 2023

iemelyanov and others added 5 commits December 26, 2023 06:18

Fix usage python C API for version greater 3.9 and fix segfault

fe1a0a9

Fix version macros

a37f650

Mov str_lower into global vars for use in istr and pair_list

2400b27

Fix macros usage

10af6c0

Add a change note for PR aio-libs#864

7eedf67

webknjaz force-pushed the fix-usage-py-c-api-and-segfault branch from 0ba9381 to 7eedf67 Compare December 26, 2023 05:18

webknjaz enabled auto-merge (squash) December 26, 2023 05:21

webknjaz merged commit 780a1e1 into aio-libs:master Dec 26, 2023
35 checks passed

webknjaz added a commit that referenced this pull request Jan 14, 2024

📝 Fix the byline in change note for PR #864

446843f

webknjaz added a commit that referenced this pull request Jan 14, 2024

📝 Link #862, #868, #898 to PR #864 change note

0780cca

JamesRobertsonGames mentioned this pull request Nov 5, 2024

Version 6.1.0 AnacondaRecipes/multidict-feedstock#8

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adapt the CPython 3.9+ C-API usage to use newer macros #864

Adapt the CPython 3.9+ C-API usage to use newer macros #864

iemelyanov commented Jul 21, 2023 •

edited by webknjaz

Loading

codecov bot commented Jul 22, 2023 •

edited

Loading

hugovk commented Aug 30, 2023

serhiy-storchaka Aug 31, 2023

iemelyanov Sep 2, 2023 •

edited

Loading

serhiy-storchaka Aug 31, 2023

iemelyanov Sep 2, 2023

serhiy-storchaka Aug 31, 2023

iemelyanov Sep 2, 2023

serhiy-storchaka Sep 2, 2023

iemelyanov Sep 2, 2023

h-vetinari commented Oct 2, 2023

JeanChristopheMorinPerso commented Nov 24, 2023

stonebig commented Dec 17, 2023

tacaswell commented Dec 21, 2023

Adapt the CPython 3.9+ C-API usage to use newer macros #864

Adapt the CPython 3.9+ C-API usage to use newer macros #864

Conversation

iemelyanov commented Jul 21, 2023 • edited by webknjaz Loading

What do these changes do?

Are there changes in behavior for the user?

Related issue number

codecov bot commented Jul 22, 2023 • edited Loading

Codecov Report

hugovk commented Aug 30, 2023

serhiy-storchaka Aug 31, 2023

Choose a reason for hiding this comment

iemelyanov Sep 2, 2023 • edited Loading

Choose a reason for hiding this comment

serhiy-storchaka Aug 31, 2023

Choose a reason for hiding this comment

iemelyanov Sep 2, 2023

Choose a reason for hiding this comment

serhiy-storchaka Aug 31, 2023

Choose a reason for hiding this comment

iemelyanov Sep 2, 2023

Choose a reason for hiding this comment

serhiy-storchaka Sep 2, 2023

Choose a reason for hiding this comment

iemelyanov Sep 2, 2023

Choose a reason for hiding this comment

h-vetinari commented Oct 2, 2023

JeanChristopheMorinPerso commented Nov 24, 2023

stonebig commented Dec 17, 2023

tacaswell commented Dec 21, 2023

iemelyanov commented Jul 21, 2023 •

edited by webknjaz

Loading

codecov bot commented Jul 22, 2023 •

edited

Loading

iemelyanov Sep 2, 2023 •

edited

Loading