Re: [bug fix] Memory leak in dblink - Mailing list pgsql-hackers

I wrote:
> I do see growth in the per-query context as well.  I'm not sure
> where that's coming from, but we probably should try to find out.
> A couple hundred bytes per iteration is going to add up, even if it's
> not as fast as 8K per iteration.  I'm not sure it's dblink's fault,
> because I don't see anything in dblink.c that is allocating anything in
> the per-query context, except for the returned tuplestores, which
> somebody else should clean up.

I poked at this example some more, and found that the additional memory
leak is occurring during evaluation of the arguments to be passed to
dblink().  There's been a comment there for a very long time suggesting
that we might need to do something about that ...

With the attached patch on top of yours, I see no leak anymore.

            regards, tom lane

diff --git a/src/backend/executor/execQual.c b/src/backend/executor/execQual.c
index f162e92..cf8cbb1 100644
*** a/src/backend/executor/execQual.c
--- b/src/backend/executor/execQual.c
*************** ExecMakeTableFunctionResult(ExprState *f
*** 2014,2019 ****
--- 2014,2020 ----
      PgStat_FunctionCallUsage fcusage;
      ReturnSetInfo rsinfo;
      HeapTupleData tmptup;
+     MemoryContext argContext = NULL;
      MemoryContext callerContext;
      MemoryContext oldcontext;
      bool        direct_function_call;
*************** ExecMakeTableFunctionResult(ExprState *f
*** 2083,2094 ****
          /*
           * Evaluate the function's argument list.
           *
!          * Note: ideally, we'd do this in the per-tuple context, but then the
!          * argument values would disappear when we reset the context in the
!          * inner loop.  So do it in caller context.  Perhaps we should make a
!          * separate context just to hold the evaluated arguments?
           */
          argDone = ExecEvalFuncArgs(&fcinfo, fcache->args, econtext);
          /* We don't allow sets in the arguments of the table function */
          if (argDone != ExprSingleResult)
              ereport(ERROR,
--- 2084,2104 ----
          /*
           * Evaluate the function's argument list.
           *
!          * We can't do this in the per-tuple context: the argument values
!          * would disappear when we reset that context in the inner loop.  And
!          * the caller's context is typically a query-lifespan context, so we
!          * don't want to leak memory there.  So make a separate context just
!          * to hold the evaluated arguments.
           */
+         argContext = AllocSetContextCreate(callerContext,
+                                            "Table function arguments",
+                                            ALLOCSET_DEFAULT_MINSIZE,
+                                            ALLOCSET_DEFAULT_INITSIZE,
+                                            ALLOCSET_DEFAULT_MAXSIZE);
+         oldcontext = MemoryContextSwitchTo(argContext);
          argDone = ExecEvalFuncArgs(&fcinfo, fcache->args, econtext);
+         MemoryContextSwitchTo(oldcontext);
+
          /* We don't allow sets in the arguments of the table function */
          if (argDone != ExprSingleResult)
              ereport(ERROR,
*************** no_function_result:
*** 2321,2328 ****
--- 2331,2342 ----
              FreeTupleDesc(rsinfo.setDesc);
      }

+     /* Clean up */
      MemoryContextSwitchTo(callerContext);

+     if (argContext)
+         MemoryContextDelete(argContext);
+
      /* All done, pass back the tuplestore */
      return rsinfo.setResult;
  }