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

Commit eaa5808

Browse files
committed
Redefine MemoryContextReset() as deleting, not resetting, child contexts.
That is, MemoryContextReset() now means what was formerly meant by MemoryContextResetAndDeleteChildren(), and the latter is now just a macro alias for the former. If you really want the functionality that was formerly provided by MemoryContextReset(), what you have to do is MemoryContextResetChildren() plus MemoryContextResetOnly() (which is a new API to reset *only* the named context and not touch its children). The reason for this change is that near fifteen years of experience has proven that there is noplace where old-style MemoryContextReset() is actually what you want. Making that the default behavior has led to lots of context-leakage bugs, while we've not found anyplace where it's actually necessary to keep the child contexts; at least the standard regression tests do not reveal anyplace where this change breaks anything. And there are upcoming patches that will introduce additional reasons why child contexts need to be removed. We could change existing calls of MemoryContextResetAndDeleteChildren to be just MemoryContextReset, but for the moment I'll leave them alone; they're not costing anything.
1 parent fbef434 commit eaa5808

File tree

3 files changed

+34
-28
lines changed

3 files changed

+34
-28
lines changed

src/backend/utils/mmgr/README

+8-3
Original file line numberDiff line numberDiff line change
@@ -125,9 +125,14 @@ lifetimes that only partially overlap can be handled by allocating
125125
from different trees of the context forest (there are some examples
126126
in the next section).
127127

128-
For convenience we will also want operations like "reset/delete all
129-
children of a given context, but don't reset or delete that context
130-
itself".
128+
Actually, it turns out that resetting a given context should almost
129+
always imply deleting (not just resetting) any child contexts it has.
130+
So MemoryContextReset() means that, and if you really do want a tree of
131+
empty contexts you need to call MemoryContextResetOnly() plus
132+
MemoryContextResetChildren().
133+
134+
For convenience we also provide operations like "reset/delete all children
135+
of a given context, but don't reset or delete that context itself".
131136

132137

133138
Globally Known Contexts

src/backend/utils/mmgr/mcxt.c

+22-24
Original file line numberDiff line numberDiff line change
@@ -132,11 +132,8 @@ MemoryContextInit(void)
132132

133133
/*
134134
* MemoryContextReset
135-
* Release all space allocated within a context and its descendants,
136-
* but don't delete the contexts themselves.
137-
*
138-
* The type-specific reset routine handles the context itself, but we
139-
* have to do the recursion for the children.
135+
* Release all space allocated within a context and delete all its
136+
* descendant contexts (but not the named context itself).
140137
*/
141138
void
142139
MemoryContextReset(MemoryContext context)
@@ -145,7 +142,22 @@ MemoryContextReset(MemoryContext context)
145142

146143
/* save a function call in common case where there are no children */
147144
if (context->firstchild != NULL)
148-
MemoryContextResetChildren(context);
145+
MemoryContextDeleteChildren(context);
146+
147+
/* save a function call if no pallocs since startup or last reset */
148+
if (!context->isReset)
149+
MemoryContextResetOnly(context);
150+
}
151+
152+
/*
153+
* MemoryContextResetOnly
154+
* Release all space allocated within a context.
155+
* Nothing is done to the context's descendant contexts.
156+
*/
157+
void
158+
MemoryContextResetOnly(MemoryContext context)
159+
{
160+
AssertArg(MemoryContextIsValid(context));
149161

150162
/* Nothing to do if no pallocs since startup or last reset */
151163
if (!context->isReset)
@@ -172,7 +184,10 @@ MemoryContextResetChildren(MemoryContext context)
172184
AssertArg(MemoryContextIsValid(context));
173185

174186
for (child = context->firstchild; child != NULL; child = child->nextchild)
175-
MemoryContextReset(child);
187+
{
188+
MemoryContextResetChildren(child);
189+
MemoryContextResetOnly(child);
190+
}
176191
}
177192

178193
/*
@@ -234,23 +249,6 @@ MemoryContextDeleteChildren(MemoryContext context)
234249
MemoryContextDelete(context->firstchild);
235250
}
236251

237-
/*
238-
* MemoryContextResetAndDeleteChildren
239-
* Release all space allocated within a context and delete all
240-
* its descendants.
241-
*
242-
* This is a common combination case where we want to preserve the
243-
* specific context but get rid of absolutely everything under it.
244-
*/
245-
void
246-
MemoryContextResetAndDeleteChildren(MemoryContext context)
247-
{
248-
AssertArg(MemoryContextIsValid(context));
249-
250-
MemoryContextDeleteChildren(context);
251-
MemoryContextReset(context);
252-
}
253-
254252
/*
255253
* MemoryContextRegisterResetCallback
256254
* Register a function to be called before next context reset/delete.

src/include/utils/memutils.h

+4-1
Original file line numberDiff line numberDiff line change
@@ -84,16 +84,19 @@ extern PGDLLIMPORT MemoryContext CurTransactionContext;
8484
/* This is a transient link to the active portal's memory context: */
8585
extern PGDLLIMPORT MemoryContext PortalContext;
8686

87+
/* Backwards compatibility macro */
88+
#define MemoryContextResetAndDeleteChildren(ctx) MemoryContextReset(ctx)
89+
8790

8891
/*
8992
* Memory-context-type-independent functions in mcxt.c
9093
*/
9194
extern void MemoryContextInit(void);
9295
extern void MemoryContextReset(MemoryContext context);
9396
extern void MemoryContextDelete(MemoryContext context);
97+
extern void MemoryContextResetOnly(MemoryContext context);
9498
extern void MemoryContextResetChildren(MemoryContext context);
9599
extern void MemoryContextDeleteChildren(MemoryContext context);
96-
extern void MemoryContextResetAndDeleteChildren(MemoryContext context);
97100
extern void MemoryContextRegisterResetCallback(MemoryContext context,
98101
MemoryContextCallback *cb);
99102
extern void MemoryContextSetParent(MemoryContext context,

0 commit comments

Comments
 (0)