On 22/04/17 20:31, Andres Freund wrote: > Hi, > > I enabled skink / the valgrind animal to run the tap tests too (hugely > increasing the test time :(), and that paid of: > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=skink&dt=2017-04-22%2004%3A52%3A13 > > ==606== VALGRINDERROR-BEGIN > ==606== Conditional jump or move depends on uninitialised value(s) > ==606== at 0x46A207: logicalrep_rel_open (relation.c:361) > ==606== by 0x472D12: copy_table (tablesync.c:669) > ==606== by 0x473186: LogicalRepSyncTableStart (tablesync.c:803) > ==606== by 0x475287: ApplyWorkerMain (worker.c:1530) > ==606== by 0x440AFD: StartBackgroundWorker (bgworker.c:835) > ==606== by 0x44E48A: do_start_bgworker (postmaster.c:5577) > ==606== by 0x44E59F: maybe_start_bgworker (postmaster.c:5761) > ==606== by 0x44F11D: sigusr1_handler (postmaster.c:5015) > ==606== by 0x4E430BF: ??? (in /lib/x86_64-linux-gnu/libpthread-2.24.so) > ==606== by 0x6FB8212: __select_nocancel (syscall-template.S:84) > ==606== by 0x44F868: ServerLoop (postmaster.c:1693) > ==606== by 0x450C53: PostmasterMain (postmaster.c:1337) > ==606== Uninitialised value was created by a heap allocation > ==606== at 0x605382: MemoryContextAlloc (mcxt.c:729) > ==606== by 0x5E4E6A: DynaHashAlloc (dynahash.c:266) > ==606== by 0x5E4EEE: element_alloc (dynahash.c:1637) > ==606== by 0x5E5018: get_hash_entry (dynahash.c:1248) > ==606== by 0x5E5898: hash_search_with_hash_value (dynahash.c:1033) > ==606== by 0x5E5A0D: hash_search (dynahash.c:890) > ==606== by 0x469D38: logicalrep_relmap_update (relation.c:179) > ==606== by 0x472D05: copy_table (tablesync.c:666) > ==606== by 0x473186: LogicalRepSyncTableStart (tablesync.c:803) > ==606== by 0x475287: ApplyWorkerMain (worker.c:1530) > ==606== by 0x440AFD: StartBackgroundWorker (bgworker.c:835) > ==606== by 0x44E48A: do_start_bgworker (postmaster.c:5577) > ==606== > ==606== VALGRINDERROR-END
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. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
From 4993487f5e8750b708e76181bb78eddea933d897 Mon Sep 17 00:00:00 2001 From: Petr Jelinek <pjmo...@pjmodos.net> Date: Sat, 22 Apr 2017 20:43:16 +0200 Subject: [PATCH] Properly initialize memory for logical replication relation cache We missed setting some of the properties of the cache entry sometimes, use memset to ensure entry is always fully cleaned up. --- src/backend/replication/logical/relation.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) 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); - - entry->attrmap = NULL; - remoterel->natts = 0; - entry->localreloid = InvalidOid; - entry->localrel = NULL; } /* @@ -182,6 +173,8 @@ logicalrep_relmap_update(LogicalRepRelation *remoterel) if (found) logicalrep_relmap_free_entry(entry); + memset(entry, 0, sizeof(LogicalRepRelMapEntry)); + /* Make cached copy of the data */ oldctx = MemoryContextSwitchTo(LogicalRepRelMapContext); entry->remoterel.remoteid = remoterel->remoteid; @@ -197,8 +190,6 @@ logicalrep_relmap_update(LogicalRepRelation *remoterel) } entry->remoterel.replident = remoterel->replident; entry->remoterel.attkeys = bms_copy(remoterel->attkeys); - entry->attrmap = NULL; - entry->localreloid = InvalidOid; MemoryContextSwitchTo(oldctx); } -- 2.7.4
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers