Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit da5ff0b

Browse files
tglsfdcCommitfest Bot
authored and
Commitfest Bot
committed
Silence complaints involving dlist_node lists.
Put the dlist_node fields of catctup and catclist structs first. This ensures that the dlist pointers point to the starts of these palloc blocks, and thus that Valgrind won't consider them "possibly lost". The postmaster's PMChild structs and the autovac launcher's avl_dbase structs also have the dlist_node-is-not-first problem, but putting it first still wouldn't silence the warning because we bulk-allocate those structs in an array, so that Valgrind sees a single allocation. Commonly the first array element will be pointed to only from some later element, so that the reference would be an interior pointer even if it pointed to the array start. (This is the same issue fixed in the previous patch for dynahash elements.) Since these are pretty simple data structures, I don't feel too bad about faking out Valgrind by just keeping a static pointer to the array start. This is all quite hacky, and it's not hard to imagine usages where we'd need some other idea in order to have reasonable leak tracking of structures that are only accessible via dlist_node lists. But this patch seems to be enough to silence this class of leakage complaints for the moment. Author: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/285483.1746756246@sss.pgh.pa.us
1 parent 46706ce commit da5ff0b

File tree

3 files changed

+45
-10
lines changed

3 files changed

+45
-10
lines changed

src/backend/postmaster/autovacuum.c

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,16 @@ static AutoVacuumShmemStruct *AutoVacuumShmem;
310310
static dlist_head DatabaseList = DLIST_STATIC_INIT(DatabaseList);
311311
static MemoryContext DatabaseListCxt = NULL;
312312

313+
/*
314+
* Dummy pointer to persuade Valgrind that we've not leaked the array of
315+
* avl_dbase structs. Make it global to ensure the compiler doesn't
316+
* optimize it away.
317+
*/
318+
#ifdef USE_VALGRIND
319+
extern avl_dbase *avl_dbase_array;
320+
avl_dbase *avl_dbase_array;
321+
#endif
322+
313323
/* Pointer to my own WorkerInfo, valid on each worker */
314324
static WorkerInfo MyWorkerInfo = NULL;
315325

@@ -1020,6 +1030,10 @@ rebuild_database_list(Oid newdb)
10201030

10211031
/* put all the hash elements into an array */
10221032
dbary = palloc(nelems * sizeof(avl_dbase));
1033+
/* keep Valgrind quiet */
1034+
#ifdef USE_VALGRIND
1035+
avl_dbase_array = dbary;
1036+
#endif
10231037

10241038
i = 0;
10251039
hash_seq_init(&seq, dbhash);

src/backend/postmaster/pmchild.c

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,17 @@ NON_EXEC_STATIC int num_pmchild_slots = 0;
5959
*/
6060
dlist_head ActiveChildList;
6161

62+
/*
63+
* Dummy pointer to persuade Valgrind that we've not leaked the array of
64+
* PMChild structs. Make it global to ensure the compiler doesn't
65+
* optimize it away.
66+
*/
67+
#ifdef USE_VALGRIND
68+
extern PMChild *pmchild_array;
69+
PMChild *pmchild_array;
70+
#endif
71+
72+
6273
/*
6374
* MaxLivePostmasterChildren
6475
*
@@ -125,8 +136,13 @@ InitPostmasterChildSlots(void)
125136
for (int i = 0; i < BACKEND_NUM_TYPES; i++)
126137
num_pmchild_slots += pmchild_pools[i].size;
127138

128-
/* Initialize them */
139+
/* Allocate enough slots, and make sure Valgrind doesn't complain */
129140
slots = palloc(num_pmchild_slots * sizeof(PMChild));
141+
#ifdef USE_VALGRIND
142+
pmchild_array = slots;
143+
#endif
144+
145+
/* Initialize them */
130146
slotno = 0;
131147
for (int btype = 0; btype < BACKEND_NUM_TYPES; btype++)
132148
{

src/include/utils/catcache.h

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,14 @@ typedef struct catcache
8787

8888
typedef struct catctup
8989
{
90+
/*
91+
* Each tuple in a cache is a member of a dlist that stores the elements
92+
* of its hash bucket. We keep each dlist in LRU order to speed repeated
93+
* lookups. Keep the dlist_node field first so that Valgrind understands
94+
* the struct is reachable.
95+
*/
96+
dlist_node cache_elem; /* list member of per-bucket list */
97+
9098
int ct_magic; /* for identifying CatCTup entries */
9199
#define CT_MAGIC 0x57261502
92100

@@ -98,13 +106,6 @@ typedef struct catctup
98106
*/
99107
Datum keys[CATCACHE_MAXKEYS];
100108

101-
/*
102-
* Each tuple in a cache is a member of a dlist that stores the elements
103-
* of its hash bucket. We keep each dlist in LRU order to speed repeated
104-
* lookups.
105-
*/
106-
dlist_node cache_elem; /* list member of per-bucket list */
107-
108109
/*
109110
* A tuple marked "dead" must not be returned by subsequent searches.
110111
* However, it won't be physically deleted from the cache until its
@@ -158,13 +159,17 @@ typedef struct catctup
158159
*/
159160
typedef struct catclist
160161
{
162+
/*
163+
* Keep the dlist_node field first so that Valgrind understands the struct
164+
* is reachable.
165+
*/
166+
dlist_node cache_elem; /* list member of per-catcache list */
167+
161168
int cl_magic; /* for identifying CatCList entries */
162169
#define CL_MAGIC 0x52765103
163170

164171
uint32 hash_value; /* hash value for lookup keys */
165172

166-
dlist_node cache_elem; /* list member of per-catcache list */
167-
168173
/*
169174
* Lookup keys for the entry, with the first nkeys elements being valid.
170175
* All by-reference are separately allocated.

0 commit comments

Comments
 (0)