Re: PL/Python initialization cleanup - Mailing list pgsql-hackers

From Matheus Alcantara
Subject Re: PL/Python initialization cleanup
Date
Msg-id 9fa042ab-bbcb-4818-95b3-cfac9737d775@gmail.com
Whole thread Raw
In response to Re: PL/Python initialization cleanup  (Peter Eisentraut <peter@eisentraut.org>)
Responses RE: PL/Python initialization cleanup
List 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



pgsql-hackers by date:

Previous
From: Artem Gavrilov
Date:
Subject: Re: Timeline switching with partial WAL records can break replica recovery
Next
From: Mihail Nikalayeu
Date:
Subject: Re: Adding REPACK [concurrently]