]> granicus.if.org Git - postgresql/commitdiff
Fix potential memory leakage from HandleParallelMessages().
authorTom Lane <tgl@sss.pgh.pa.us>
Fri, 26 Aug 2016 19:04:05 +0000 (15:04 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Fri, 26 Aug 2016 19:04:05 +0000 (15:04 -0400)
HandleParallelMessages leaked memory into the caller's context.  Since it's
called from ProcessInterrupts, there is basically zero certainty as to what
CurrentMemoryContext is, which means we could be leaking into long-lived
contexts.  Over the processing of many worker messages that would grow to
be a problem.  Things could be even worse than just a leak, if we happened
to service the interrupt while ErrorContext is current: elog.c thinks it
can reset that on its own whim, possibly yanking storage out from under
HandleParallelMessages.

Give HandleParallelMessages its own dedicated context instead, which we can
reset during each call to ensure there's no accumulation of wasted memory.

Discussion: <16610.1472222135@sss.pgh.pa.us>

src/backend/access/transam/parallel.c

index ec6e1c5e6dc20844e5f8d9a463133b1a2ccd75b9..949bfb8b3e6e3f5fb4c702d0fcf5792bc540b5e3 100644 (file)
@@ -702,6 +702,9 @@ void
 HandleParallelMessages(void)
 {
        dlist_iter      iter;
+       MemoryContext oldcontext;
+
+       static MemoryContext hpm_context = NULL;
 
        /*
         * This is invoked from ProcessInterrupts(), and since some of the
@@ -712,6 +715,23 @@ HandleParallelMessages(void)
         */
        HOLD_INTERRUPTS();
 
+       /*
+        * Moreover, CurrentMemoryContext might be pointing almost anywhere.  We
+        * don't want to risk leaking data into long-lived contexts, so let's do
+        * our work here in a private context that we can reset on each use.
+        */
+       if (hpm_context == NULL)        /* first time through? */
+               hpm_context = AllocSetContextCreate(TopMemoryContext,
+                                                                                       "HandleParallelMessages context",
+                                                                                       ALLOCSET_DEFAULT_MINSIZE,
+                                                                                       ALLOCSET_DEFAULT_INITSIZE,
+                                                                                       ALLOCSET_DEFAULT_MAXSIZE);
+       else
+               MemoryContextReset(hpm_context);
+
+       oldcontext = MemoryContextSwitchTo(hpm_context);
+
+       /* OK to process messages.  Reset the flag saying there are more to do. */
        ParallelMessagePending = false;
 
        dlist_foreach(iter, &pcxt_list)
@@ -758,6 +778,11 @@ HandleParallelMessages(void)
                }
        }
 
+       MemoryContextSwitchTo(oldcontext);
+
+       /* Might as well clear the context on our way out */
+       MemoryContextReset(hpm_context);
+
        RESUME_INTERRUPTS();
 }