Re: valgrind error in subscription code

Andres Freund <andres@anarazel.de>

From: Andres Freund <andres@anarazel.de>
To: Petr Jelinek <petr.jelinek@2ndquadrant.com>
Cc: pgsql-hackers@postgresql.org, Petr Jelinek <petr@2ndquadrant.com>, Peter Eisentraut <peter_e@gmx.net>
Date: 2017-04-22T19:16:51Z
Lists: pgsql-hackers
Hi,

On 2017-04-22 21:08:18 +0200, Petr Jelinek wrote:
> Thanks, here is patch to fix that - I also removed the individual
> settings of everything to NULL/0/InvalidOid etc and just replaced it all
> with memset.

Cool.

> diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c
> index 875a081..5bc54dd 100644
> --- a/src/backend/replication/logical/relation.c
> +++ b/src/backend/replication/logical/relation.c
> @@ -141,19 +141,10 @@ logicalrep_relmap_free_entry(LogicalRepRelMapEntry *entry)
>  		pfree(remoterel->attnames);
>  		pfree(remoterel->atttyps);
>  	}
> -	remoterel->attnames = NULL;
> -	remoterel->atttyps = NULL;
> -
>  	bms_free(remoterel->attkeys);
> -	remoterel->attkeys = NULL;
>  
>  	if (entry->attrmap)
>  		pfree(entry->attrmap);
> -

Btw, I think it's a good pattern to zero things like attrmap after
freeing.  Based on a minute of looking it's unclear to me if
logicalrep_relmap_update() could be called again, if e.g. one of the
pallocs after the logicalrep_relmap_free_entry() errors out.  I think
you essentially addressed that with the memset, so that's good.

- Andres


Commits

  1. Zero padding in replication origin's checkpointed on disk-state.

  2. Initialize all memory for logical replication relation cache.