On Thu, Jun 17, 2021 at 3:42 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > On Thu, Jun 17, 2021 at 10:39 AM Amit Langote <amitlangot...@gmail.com> wrote: > > > > On Thu, Jun 17, 2021 at 12:56 PM Amit Kapila <amit.kapil...@gmail.com> > > wrote: > > > On Wed, Jun 16, 2021 at 8:18 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > > > > > > > > Amit Kapila <amit.kapil...@gmail.com> writes: > > > > > Pushed! > > > > > > > > skink reports that this has valgrind issues: > > > > > > > > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=skink&dt=2021-06-15%2020%3A49%3A26 > > > > > > > > > > The problem happens at line: > > > rel_sync_cache_relation_cb() > > > { > > > .. > > > if (entry->map) > > > .. > > > > > > I think the reason is that before we initialize 'entry->map' in > > > get_rel_sync_entry(), the invalidation is processed as part of which > > > when we try to clean up the entry, it tries to access uninitialized > > > value. Note, this won't happen in HEAD as we initialize 'entry->map' > > > before we get to process any invalidation. We have fixed a similar > > > issue in HEAD sometime back as part of the commit 69bd60672a, so we > > > need to make a similar change in PG-13 as well. > > > > > > This problem is introduced by commit d250568121 (Fix memory leak due > > > to RelationSyncEntry.map.) not by the patch in this thread, so keeping > > > Amit L and Osumi-San in the loop. > > > > Thanks. > > > > Maybe not sufficient as a fix, but I wonder if > > rel_sync_cache_relation_cb() should really also check that > > replicate_valid is true in the following condition: > > I don't think that is required because we initialize the entry in "if > (!found)" case in the HEAD.
Yeah, I see that. If we can be sure that the callback can't get
called between hash_search() allocating the entry and the above code
block making the entry look valid, which appears to be the case, then
I guess we don't need to worry.
> > /*
> > * Reset schema sent status as the relation definition may have changed.
> > * Also free any objects that depended on the earlier definition.
> > */
> > if (entry != NULL)
> > {
> >
> > If the problem is with HEAD,
> >
>
> The problem occurs only in PG-13. So, we need to make PG-13 code
> similar to HEAD as far as initialization of entry is concerned.
Oh I missed that the problem report is for the PG13 branch.
How about the attached patch then?
--
Amit Langote
EDB: http://www.enterprisedb.com
pg13-init-RelationSyncEntry-properly.patch
Description: Binary data
