| From: | Matheus Alcantara <matheusssilv97(at)gmail(dot)com> |
|---|---|
| To: | Peter Eisentraut <peter(at)eisentraut(dot)org>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: PL/Python initialization cleanup |
| Date: | 2026-01-12 18:07:21 |
| Message-ID: | 9fa042ab-bbcb-4818-95b3-cfac9737d775@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On 12/01/26 09:24, Peter Eisentraut wrote:
>> 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.
Thanks, it looks more clear IMHO now. Agree that the code is correct.
--
Matheus Alcantara
EDB: https://www.enterprisedb.com
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Mihail Nikalayeu | 2026-01-12 18:20:35 | Re: Adding REPACK [concurrently] |
| Previous Message | Artem Gavrilov | 2026-01-12 18:04:19 | Re: Timeline switching with partial WAL records can break replica recovery |