bpo-41861: Convert _sqlite3 CursorType and ConnectionType to heap types

[erlend-aasland]

Part 4 of bpo-41861, @vstinner. #norefleaks

After this, I'd like to merge #22419 to finalise bpo-41861.

https://bugs.python.org/issue41861

[vstinner]

Merged, thanks. There are two remaining static types?

[erlend-aasland]

Merged, thanks. There are two remaining static types?

Thanks! No more remaining types.

[erlend-aasland]

Merged, thanks. There are two remaining static types?

BTW, should I add traverse/clear/free methods to the module def before closing the issue?

[vstinner]

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

[erlend-aasland]

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!

[vstinner]

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.

[erlend-aasland]

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:

1. 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."

2. 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

[vstinner]

Also, defining_class is not documented. I'll open an issue for that. https://docs.python.org/3.10/howto/clinic.html

Good idea.

[vstinner]

I wrote #22712 to "explain my idea". It's a minimum change just to pass a "state" to functions which access the UCD_Type.

[erlend-aasland]

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.