Re: replication cleanup code incorrect way to use of HTAB HASH_REMOVE ? - Mailing list pgsql-hackers

From Amit Kapila
Subject Re: replication cleanup code incorrect way to use of HTAB HASH_REMOVE ?
Date
Msg-id CAA4eK1KYLF7x4a_DZUUj8Q4XdiUSAd2y27y9ehKKtXk7jZbcbQ@mail.gmail.com
Whole thread Raw
In response to Re: replication cleanup code incorrect way to use of HTAB HASH_REMOVE ?  (Amit Kapila <amit.kapila16@gmail.com>)
List pgsql-hackers
On Mon, Mar 22, 2021 at 7:57 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Mon, Mar 22, 2021 at 3:20 AM Peter Smith <smithpb2250@gmail.com> wrote:
> >
> > On Sun, Mar 21, 2021 at 8:54 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > > On Sat, Mar 20, 2021 at 12:54 PM Peter Smith <smithpb2250@gmail.com> wrote:
> > > >
> > > > PSA my patch to correct this by firstly doing a HASH_FIND, then only
> > > > HASH_REMOVE after we've finished using the ent.
> > > >
> > >
> > > Why can't we keep using HASH_REMOVE as it is but get the output (entry
> > > found or not) in the last parameter of hash_search API and then
> > > perform Assert based on that? See similar usage in reorderbuffer.c and
> > > rewriteheap.c.
> > >
> >
> > Changing the Assert doesn't do anything to fix the problem as
> > described, i.e. dereferencing of ent after the HASH_REMOVE.
> >
> > The real problem isn't the Assert. It's all those other usages of ent
> > disobeying the API rule: "(NB: in the case of the REMOVE action, the
> > result is a dangling pointer that shouldn't be dereferenced!)"
> >
>
> Right, that is a problem. I see that your patch will fix it. Thanks.
>

Pushed your patch.

-- 
With Regards,
Amit Kapila.



pgsql-hackers by date:

Previous
From: Neha Sharma
Date:
Subject: Re: [CLOBBER_CACHE]Server crashed with segfault 11 while executing clusterdb
Next
From: Michael Paquier
Date:
Subject: Re: Proposal: Save user's original authenticated identity for logging