From: | Peter Smith <smithpb2250(at)gmail(dot)com> |
---|---|
To: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: replication cleanup code incorrect way to use of HTAB HASH_REMOVE ? |
Date: | 2021-03-21 21:49:50 |
Message-ID: | CAHut+PvhkQDGEXm9iEOjJGXiHNT7fD3FgPiMZPtDYC43XDnsqQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sun, Mar 21, 2021 at 8:54 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Sat, Mar 20, 2021 at 12:54 PM Peter Smith <smithpb2250(at)gmail(dot)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!)"
e.g.
- SharedFileSetDeleteAll(ent->stream_fileset);
- pfree(ent->stream_fileset);
- ent->stream_fileset = NULL;
- if (ent->subxact_fileset)
- SharedFileSetDeleteAll(ent->subxact_fileset);
- pfree(ent->subxact_fileset);
- ent->subxact_fileset = NULL;
------
Kind Regards,
Peter Smith
Fujitsu Australia.
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2021-03-21 21:53:50 | Re: shared memory stats: high level design decisions: consistency, dropping |
Previous Message | Andres Freund | 2021-03-21 21:40:11 | Re: Replication slot stats misgivings |