From b65da5fae893c804b5997af15bfb522170591a1c Mon Sep 17 00:00:00 2001 From: Jelte Fennema Date: Mon, 30 Jan 2023 18:33:14 +0100 Subject: [PATCH] Fix weird valgrind transaction/connection issue Once we were able to reproduce issue #6638 consistently on our local machines we were able to use some manual guesswork at the causes and `git bisect` to find the cause of it. Which turns out were two causes, which together combined into the valgrind issue: 1. Transaction/connection cleanup refactoring in #6314 2. Adding replicationConnParam to the connection in #6080 The innocent looking `hash_uint32` call on the `replicationConnParam` field (of type `bool`) that was added in #6080 is the main culprit. And thus we address that here by replacing it with `hash_any`. It seems that hash_uint32 was causing hashing of undefined memory somehow. But I'm not entirely sure how. Since I'd expect the `bool` to be promoted to a `1` of `uint32`. It might have something to do with the fact that `hash_uint32` is defined as `inline`. The transaction/connection cleanup refactoring (while afaict correct) then caused this hashing issue to surface as a memory issue later on during cleanup of the connection. Both of these commits were included in 11.1, but my guess is that valgrind didn't catch it then because #6314 was merged after the initial release branch was cut. Fixes #6638 --- src/backend/distributed/connection/connection_management.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/backend/distributed/connection/connection_management.c b/src/backend/distributed/connection/connection_management.c index c5b300fd4..05fe185ab 100644 --- a/src/backend/distributed/connection/connection_management.c +++ b/src/backend/distributed/connection/connection_management.c @@ -1237,7 +1237,8 @@ ConnectionHashHash(const void *key, Size keysize) hash = hash_combine(hash, hash_uint32(entry->port)); hash = hash_combine(hash, string_hash(entry->user, NAMEDATALEN)); hash = hash_combine(hash, string_hash(entry->database, NAMEDATALEN)); - hash = hash_combine(hash, hash_uint32(entry->replicationConnParam)); + hash = hash_combine(hash, hash_any((void *) &entry->replicationConnParam, + sizeof(entry->replicationConnParam))); return hash; }