On 05.01.26 16:08, Matheus Alcantara wrote:
> On Wed Dec 31, 2025 at 5:47 AM -03, Peter Eisentraut wrote:
>> As I was working through steps to make PL/Python more thread-safe, I
>> noticed that the initialization code of PL/Python is pretty messy. I
>> think some of this has grown while both Python 2 and 3 were supported,
>> because they required different initialization steps, and we had some
>> defenses against accidentally running both at the same time. But that
>> is over, and right now a lot of this doesn't make sense anymore. For
>> example, the function PLy_init_interp() said "Initialize the Python
>> interpreter ..." but it didn't actually do this, and PLy_init_plpy()
>> said "initialize plpy module" but it didn't do that either (or at least
>> they used the term "initialize" in non-standard ways).
>>
>> Here are some patches to clean this up. After this change, all the
>> global initialization is called directly from _PG_init(), and the plpy
>> module initialization is all called from its registered initialization
>> function PyInit_plpy(). (For the thread-safety job, the plpy module
>> initialization will need to be rewritten using a different API. That's
>> why I'm keen to have it clearly separated.) I also tried to add more
>> comments and make existing comments more precise. There was also some
>> apparently obsolete or redundant code that could be deleted.
>>
>> Surely, all of this will need some more rounds of careful scrutiny, but
>> I think the overall code arrangement is correct and an improvement.
>
> 0001, 0003 and 0004 looks good to me, just a small comment on 0002:
>
> - /*
> - * PyModule_AddObject does not add a refcount to the object, for some odd
> - * reason; we must do that.
> - */
> - Py_INCREF(exc);
> - PyModule_AddObject(mod, modname, exc);
> -
> /*
> * The caller will also store a pointer to the exception object in some
> - * permanent variable, so add another ref to account for that. This is
> - * probably excessively paranoid, but let's be sure.
> + * permanent variable, so add another ref to account for that.
> */
> Py_INCREF(exc);
>
> The later code comment say "so add another ref to account for that", but
> you've removed the previous Py_INCREF() call. The returned object
> PyErr_NewException() already have a refcount increased for usage? If
> that's not the case I think that the "add another ref..." don't seems
> correct because IIUC we are increasing the ref count for the first time,
> so there is no "another" refcount being increased.
The reference created by PyErr_NewException() is "stolen" by
PyModule_AddObject(), so we need to create another one for returning the
object from the function and storing it in the permanent variable. I
have updated the comment in this new patch version. But I think the
actual code is correct.