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
This commit is contained in:
Andres Freund 2018-07-25 16:31:49 -07:00
parent bcafa263ec
commit 3acc4acd9b
2 changed files with 13 additions and 10 deletions

View File

@ -47,7 +47,6 @@
#include "commands/trigger.h" #include "commands/trigger.h"
#include "executor/execdebug.h" #include "executor/execdebug.h"
#include "foreign/fdwapi.h" #include "foreign/fdwapi.h"
#include "jit/jit.h"
#include "mb/pg_wchar.h" #include "mb/pg_wchar.h"
#include "miscadmin.h" #include "miscadmin.h"
#include "optimizer/clauses.h" #include "optimizer/clauses.h"
@ -498,10 +497,6 @@ standard_ExecutorEnd(QueryDesc *queryDesc)
UnregisterSnapshot(estate->es_snapshot); UnregisterSnapshot(estate->es_snapshot);
UnregisterSnapshot(estate->es_crosscheck_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 * Must switch out of context before destroying it
*/ */

View File

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