https://github.com/python/cpython/commit/001ef4600f5ab2e1d7825ddbc0f253377c2... commit: 001ef4600f5ab2e1d7825ddbc0f253377c234d7e branch: main author: Erlend Egeberg Aasland <erlend.aasland@innova.no> committer: miss-islington <31488909+miss-islington@users.noreply.github.com> date: 2021-08-31T05:18:43-07:00 summary: bpo-44991: Make GIL handling more explicit in `sqlite3` callbacks (GH-27934) - acquire the GIL at the very start[1] - release the GIL at the very end [1] The trace callback performs a sanity check before acquiring the GIL Automerge-Triggered-By: GH:encukou files: M Modules/_sqlite/connection.c diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index 19d30d24d7f2e7..9ff5fbaae78529 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -626,14 +626,12 @@ set_sqlite_error(sqlite3_context *context, const char *msg) static void _pysqlite_func_callback(sqlite3_context *context, int argc, sqlite3_value **argv) { + PyGILState_STATE threadstate = PyGILState_Ensure(); + PyObject* args; PyObject* py_retval = NULL; int ok; - PyGILState_STATE threadstate; - - threadstate = PyGILState_Ensure(); - args = _pysqlite_build_py_params(context, argc, argv); if (args) { callback_context *ctx = (callback_context *)sqlite3_user_data(context); @@ -656,15 +654,13 @@ _pysqlite_func_callback(sqlite3_context *context, int argc, sqlite3_value **argv static void _pysqlite_step_callback(sqlite3_context *context, int argc, sqlite3_value** params) { + PyGILState_STATE threadstate = PyGILState_Ensure(); + PyObject* args; PyObject* function_result = NULL; PyObject** aggregate_instance; PyObject* stepmethod = NULL; - PyGILState_STATE threadstate; - - threadstate = PyGILState_Ensure(); - aggregate_instance = (PyObject**)sqlite3_aggregate_context(context, sizeof(PyObject*)); if (*aggregate_instance == NULL) { @@ -706,16 +702,14 @@ static void _pysqlite_step_callback(sqlite3_context *context, int argc, sqlite3_ static void _pysqlite_final_callback(sqlite3_context *context) { + PyGILState_STATE threadstate = PyGILState_Ensure(); + PyObject* function_result; PyObject** aggregate_instance; _Py_IDENTIFIER(finalize); int ok; PyObject *exception, *value, *tb; - PyGILState_STATE threadstate; - - threadstate = PyGILState_Ensure(); - aggregate_instance = (PyObject**)sqlite3_aggregate_context(context, 0); if (aggregate_instance == NULL) { /* No rows matched the query; the step handler was never called. */ @@ -787,34 +781,34 @@ static void _pysqlite_drop_unused_cursor_references(pysqlite_Connection* self) static callback_context * create_callback_context(pysqlite_state *state, PyObject *callable) { - PyGILState_STATE gstate = PyGILState_Ensure(); callback_context *ctx = PyMem_Malloc(sizeof(callback_context)); if (ctx != NULL) { ctx->callable = Py_NewRef(callable); ctx->state = state; } - PyGILState_Release(gstate); return ctx; } static void free_callback_context(callback_context *ctx) +{ + assert(ctx != NULL); + Py_DECREF(ctx->callable); + PyMem_Free(ctx); +} + +static void +_destructor(void *ctx) { if (ctx != NULL) { // This function may be called without the GIL held, so we need to // ensure that we destroy 'ctx' with the GIL held. PyGILState_STATE gstate = PyGILState_Ensure(); - Py_DECREF(ctx->callable); - PyMem_Free(ctx); + free_callback_context((callback_context *)ctx); PyGILState_Release(gstate); } } -static void _destructor(void* args) -{ - free_callback_context((callback_context *)args); -} - /*[clinic input] _sqlite3.Connection.create_function as pysqlite_connection_create_function @@ -914,11 +908,10 @@ pysqlite_connection_create_aggregate_impl(pysqlite_Connection *self, static int _authorizer_callback(void* user_arg, int action, const char* arg1, const char* arg2 , const char* dbname, const char* access_attempt_source) { + PyGILState_STATE gilstate = PyGILState_Ensure(); + PyObject *ret; int rc; - PyGILState_STATE gilstate; - - gilstate = PyGILState_Ensure(); ret = PyObject_CallFunction((PyObject*)user_arg, "issss", action, arg1, arg2, dbname, access_attempt_source); @@ -959,11 +952,10 @@ static int _authorizer_callback(void* user_arg, int action, const char* arg1, co static int _progress_handler(void* user_arg) { + PyGILState_STATE gilstate = PyGILState_Ensure(); + int rc; PyObject *ret; - PyGILState_STATE gilstate; - - gilstate = PyGILState_Ensure(); ret = _PyObject_CallNoArg((PyObject*)user_arg); if (!ret) { @@ -1000,18 +992,16 @@ static int _trace_callback(unsigned int type, void* user_arg, void* prepared_sta static void _trace_callback(void* user_arg, const char* statement_string) #endif { - PyObject *py_statement = NULL; - PyObject *ret = NULL; - - PyGILState_STATE gilstate; - #ifdef HAVE_TRACE_V2 if (type != SQLITE_TRACE_STMT) { return 0; } #endif - gilstate = PyGILState_Ensure(); + PyGILState_STATE gilstate = PyGILState_Ensure(); + + PyObject *py_statement = NULL; + PyObject *ret = NULL; py_statement = PyUnicode_DecodeUTF8(statement_string, strlen(statement_string), "replace"); if (py_statement) { @@ -1461,14 +1451,16 @@ pysqlite_collation_callback( int text1_length, const void* text1_data, int text2_length, const void* text2_data) { + PyGILState_STATE gilstate = PyGILState_Ensure(); + PyObject* string1 = 0; PyObject* string2 = 0; - PyGILState_STATE gilstate; PyObject* retval = NULL; long longval; int result = 0; - gilstate = PyGILState_Ensure(); + /* This callback may be executed multiple times per sqlite3_step(). Bail if + * the previous call failed */ if (PyErr_Occurred()) { goto finally; }