bpo-41861: Convert _sqlite3 CursorType and ConnectionType to heap types by erlend-aasland · Pull Request #22478 · python/cpython
Erlend E. Aasland added 2 commits
October 1, 2020 15:27Merged, thanks. There are two remaining static types?
BTW, should I add traverse/clear/free methods to the module def before closing the issue?
BTW, should I add traverse/clear/free methods to the module def before closing the issue?
Currently, it's possible to have more than once instance of the _sqlite3 module. If a second instance is created and then cleared, it would clear the shared heap types of the first instance: not good.
It would be better to add a module state and retrieve these types from the module state.
The problem is to retrieve the module state. I suggest you to convert all methods which use these 4 types to Argument Clinic, so later you will be able to use "defining_class" parameter which is a reliable way to get the module state from a type. That's why I asked you to use PyType_FromModuleAndSpec().
Example: _sqlite3.Connection.backup() Python method is implemented as the pysqlite_connection_backup() C function. If you modify the function to use Argument Clinic, you can then get the defining class: then PyType_GetModuleState(cls) gives you the module state.
pysqlite_connection_backup() -> defining_class (cls) -> PyType_GetModuleState(cls) -> module state -> your heap types
Converting _sqlite3 to use a module state will make it safe to be used in Python subinterpreters!
See:
- My notes: https://pythondev.readthedocs.io/subinterpreters.html
- Convert static types to heap types: use PyType_FromSpec(): https://bugs.python.org/issue40077
- bpo-1635741: Py_Finalize() doesn’t clear all Python objects at exit: https://bugs.python.org/issue1635741
BTW, should I add traverse/clear/free methods to the module def before closing the issue?
Currently, it's possible to have more than once instance of the _sqlite3 module. If a second instance is created and then cleared, it would clear the shared heap types of the first instance: not good.
It would be better to add a module state and retrieve these types from the module state.
I already started experimenting with this. Should I put up what I've got as a draft PR? I guess this goes as a separate issue anyways.
The problem is to retrieve the module state.
I'm painfully aware of this :)
I suggest you to convert all methods which use these 4 types to Argument Clinic, so later you will be able to use "defining_class" parameter which is a reliable way to get the module state from a type. That's why I asked you to use PyType_FromModuleAndSpec().
I think I already put up a PR that converts _sqlite3 to Argument Clinic. I'll rebase onto master and ping you.
- My notes: https://pythondev.readthedocs.io/subinterpreters.html
- Convert static types to heap types: use PyType_FromSpec(): https://bugs.python.org/issue40077
- bpo-1635741: Py_Finalize() doesn’t clear all Python objects at exit: https://bugs.python.org/issue1635741
Thanks!
a PR that converts _sqlite3 to Argument Clinic
That sounds like a good start! I prefer a PR to convert to Argument Clinic, and then a second one to add a module state.
a PR that converts _sqlite3 to Argument Clinic
That sounds like a good start! I prefer a PR to convert to Argument Clinic, and then a second one to add a module state.
While my other PR's are on hold, I've started with the post-agrument-clinic-step: heap module state (PEP 573). I'm currently basing this branch on top off my work with bpo-40956. Using defining_class makes this very easy, however I need help with resolving two issues:
-
What to do with the Argument Clinic class definition? We need the module pointer to get to the instances and the types. Quoting from the Argument Clinic docs:
"When you declare a class, you must also specify two aspects of its type in C: the type declaration you’d use for a pointer to an instance of this class, and a pointer to the PyTypeObject for this class." -
What to do with
__init__and__new__? Providing a pointer to the module in the class/type state, would solve it, but that feels backwards and hacky.
UPDATE: Solved like this (excerpt from Argument Clinic for _sqlite3.Connection.__init__):
factory: object(c_default='(PyObject*)((struct pysqlite_state *)PyType_GetModuleState(Py_TYPE(self)))->ConnectionType') = ConnectionType
Is this an accepted solution?
Also, defining_class is not documented. I'll open an issue for that. https://docs.python.org/3.10/howto/clinic.html
I wrote #22712 to "explain my idea". It's a minimum change just to pass a "state" to functions which access the UCD_Type.
I wrote #22712 to "explain my idea". It's a minimum change just to pass a "state" to functions which access the UCD_Type.
Thanks, I'll have a look.
xzy3 pushed a commit to xzy3/cpython that referenced this pull request
Oct 18, 2020
kylotan
mannequin
mentioned this pull request
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters