Re: Fix for segfault in logical replication on master

From: Japin Li <japinli(at)hotmail(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>, osumi(dot)takamichi(at)fujitsu(dot)com, "akapila(at)postgresql(dot)org" <akapila(at)postgresql(dot)org>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Fix for segfault in logical replication on master
Date: 2021-06-21 08:00:16
Message-ID: MEYP282MB166958031E67BC202BBB06ACB60A9@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


On Sat, 19 Jun 2021 at 17:18, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> On Fri, Jun 18, 2021 at 9:18 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>>
>> > I thought it was cheap enough to check that the relation we open is an index, because if it is not, we'll segfault when accessing fields of the relation->rd_index struct. I wouldn't necessarily advocate doing any really expensive checks here, but a quick sanity check seemed worth the effort.
>> >
>>
>> I am not telling you anything about the cost of these sanity checks. I
>> suggest you raise elog rather than return NULL because if this happens
>> there is definitely some problem and continuing won't be a good idea.
>>
>
> Pushed, after making the above change. Additionally, I have moved the
> test case to the existing file 001_rep_changes instead of creating a
> new one as the test seems to fit there and I was not sure if the test
> for just this case deserves a new file.

Hi, Amit

Sorry for the late repay.

When we find that the relation has no replica identity index, I think we should
free the memory of the indexoidlist. Since we free the memory owned by
indexoidlist at end of RelationGetIdentityKeyBitmap().

if (!OidIsValid(relation->rd_replidindex))
{
list_free(indexoidlist);
return NULL;
}

Or we can free the memory owned by indexoidlist after check whether it is NIL,
because we do not use it in the later.

If we do not free the memory, there might be a memory leak when
relation->rd_replidindex is invalid. Am I right?

--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message yuzuko 2021-06-21 08:21:25 Autovacuum (analyze) on partitioned tables for ATTACH/DETACH/DROP commands
Previous Message Peter Eisentraut 2021-06-21 07:23:09 Add index OID macro argument to DECLARE_INDEX