LLVMJIT: Release JIT context after running ExprContext shutdown callbacks.

Due to inlining it previously was possible that an ExprContext's
shutdown callback pointed to a JITed function. As the JIT context
previously was shut down before the shutdown callbacks were called,
that could lead to segfaults.  Fix the ordering.

Reported-By: Dmitry Dolgov
Author: Andres Freund
Discussion: https://postgr.es/m/CA+q6zcWO7CeAJtHBxgcHn_hj+PenM=tvG0RJ93X1uEJ86+76Ug@mail.gmail.com
Backpatch: 11-, where JIT compilation was added

diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 8026fe2438f89c9e9b073eb469d657fb7b22e2c8..01e1a46180435a6d66236ab6b88b1b7d82919f40 100644 (file)
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -47,7 +47,6 @@
 #include "commands/trigger.h"
 #include "executor/execdebug.h"
 #include "foreign/fdwapi.h"
-#include "jit/jit.h"
 #include "mb/pg_wchar.h"
 #include "miscadmin.h"
 #include "optimizer/clauses.h"
@@ -498,10 +497,6 @@ standard_ExecutorEnd(QueryDesc *queryDesc)
    UnregisterSnapshot(estate->es_snapshot);
    UnregisterSnapshot(estate->es_crosscheck_snapshot);
 
-   /* release JIT context, if allocated */
-   if (estate->es_jit)
-       jit_release_context(estate->es_jit);
-
    /*
     * Must switch out of context before destroying it
     */

diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index b963cae730c26758d3b51812f461cbf3d1577232..5b3eaec80bcafaeff5567c2c7376e9cfe41ecfe6 100644 (file)
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -45,6 +45,7 @@
 #include "access/relscan.h"
 #include "access/transam.h"
 #include "executor/executor.h"
+#include "jit/jit.h"
 #include "mb/pg_wchar.h"
 #include "nodes/nodeFuncs.h"
 #include "parser/parsetree.h"
@@ -174,11 +175,11 @@ CreateExecutorState(void)
  *
  *     Release an EState along with all remaining working storage.
  *
- * Note: this is not responsible for releasing non-memory resources,
- * such as open relations or buffer pins.  But it will shut down any
- * still-active ExprContexts within the EState.  That is sufficient
- * cleanup for situations where the EState has only been used for expression
- * evaluation, and not to run a complete Plan.
+ * Note: this is not responsible for releasing non-memory resources, such as
+ * open relations or buffer pins.  But it will shut down any still-active
+ * ExprContexts within the EState and deallocate associated JITed expressions.
+ * That is sufficient cleanup for situations where the EState has only been
+ * used for expression evaluation, and not to run a complete Plan.
  *
  * This can be called in any memory context ... so long as it's not one
  * of the ones to be freed.
@@ -204,6 +205,13 @@ FreeExecutorState(EState *estate)
        /* FreeExprContext removed the list link for us */
    }
 
+   /* release JIT context, if allocated */
+   if (estate->es_jit)
+   {
+       jit_release_context(estate->es_jit);
+       estate->es_jit = NULL;
+   }
+
    /*
     * Free the per-query memory context, thereby releasing all working
     * memory, including the EState node itself.