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:
/*
* 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, I don't quite understand why the last
statement of the following code block doesn't suffice:
/* Not found means schema wasn't sent */
if (!found)
{
/* immediately make a new entry valid enough to satisfy callbacks */
entry->schema_sent = false;
entry->streamed_txns = NIL;
entry->replicate_valid = false;
entry->pubactions.pubinsert = entry->pubactions.pubupdate =
entry->pubactions.pubdelete = entry->pubactions.pubtruncate = false;
entry->publish_as_relid = InvalidOid;
entry->map = NULL; /* will be set by maybe_send_schema() if needed */
}
Do we need the same statement at the end of the following block?
/* Validate the entry */
if (!entry->replicate_valid)
{
--
Amit Langote
EDB: http://www.enterprisedb.com
