From e4fe706a23b24a0f8541c13efcc72c83ed3ed2bd Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Thu, 29 Jul 2021 20:07:17 +0200 Subject: [PATCH 01/15] Adapt trace callback --- Modules/_sqlite/connection.c | 59 ++++++++++++++++++++++++++++-------- Modules/_sqlite/connection.h | 2 +- 2 files changed, 47 insertions(+), 14 deletions(-) diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index 8ad5f5f061da5d..1fa41075d2c61b 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -55,6 +55,7 @@ static const char * const begin_statements[] = { static int pysqlite_connection_set_isolation_level(pysqlite_Connection* self, PyObject* isolation_level, void *Py_UNUSED(ignored)); static void _pysqlite_drop_unused_cursor_references(pysqlite_Connection* self); +static void free_callback_context(callback_context *ctx); static PyObject * new_statement_cache(pysqlite_Connection *self, int maxsize) @@ -168,7 +169,7 @@ pysqlite_connection_init_impl(pysqlite_Connection *self, self->thread_ident = PyThread_get_thread_ident(); self->check_same_thread = check_same_thread; - self->function_pinboard_trace_callback = NULL; + self->trace_ctx = NULL; self->function_pinboard_progress_handler = NULL; self->function_pinboard_authorizer_cb = NULL; @@ -214,6 +215,13 @@ pysqlite_do_all_statements(pysqlite_Connection *self) } } +#define VISIT_CALLBACK_CONTEXT(ctx) \ +do { \ + if (ctx) { \ + Py_VISIT(ctx->callable); \ + } \ +} while (0) + static int connection_traverse(pysqlite_Connection *self, visitproc visit, void *arg) { @@ -223,12 +231,20 @@ connection_traverse(pysqlite_Connection *self, visitproc visit, void *arg) Py_VISIT(self->cursors); Py_VISIT(self->row_factory); Py_VISIT(self->text_factory); - Py_VISIT(self->function_pinboard_trace_callback); + VISIT_CALLBACK_CONTEXT(self->trace_ctx); Py_VISIT(self->function_pinboard_progress_handler); Py_VISIT(self->function_pinboard_authorizer_cb); +#undef VISIT_CALLBACK_CONTEXT return 0; } +#define CLEAR_CALLBACK_CONTEXT(ctx) \ +do { \ + if (ctx) { \ + Py_CLEAR(ctx->callable); \ + } \ +} while (0) + static int connection_clear(pysqlite_Connection *self) { @@ -237,9 +253,10 @@ connection_clear(pysqlite_Connection *self) Py_CLEAR(self->cursors); Py_CLEAR(self->row_factory); Py_CLEAR(self->text_factory); - Py_CLEAR(self->function_pinboard_trace_callback); + CLEAR_CALLBACK_CONTEXT(self->trace_ctx); Py_CLEAR(self->function_pinboard_progress_handler); Py_CLEAR(self->function_pinboard_authorizer_cb); +#undef CLEAR_CALLBACK_CONTEXT return 0; } @@ -253,12 +270,19 @@ connection_close(pysqlite_Connection *self) } } +static void +free_callback_contexts(pysqlite_Connection *self) +{ + free_callback_context(self->trace_ctx); +} + static void connection_dealloc(pysqlite_Connection *self) { PyTypeObject *tp = Py_TYPE(self); PyObject_GC_UnTrack(self); tp->tp_clear((PyObject *)self); + free_callback_contexts(self); /* Clean up if user has not called .close() explicitly. */ connection_close(self); @@ -804,7 +828,7 @@ free_callback_context(callback_context *ctx) // 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); + Py_XDECREF(ctx->callable); PyMem_Free(ctx); PyGILState_Release(gstate); } @@ -1014,16 +1038,18 @@ static void _trace_callback(void* user_arg, const char* statement_string) gilstate = PyGILState_Ensure(); py_statement = PyUnicode_DecodeUTF8(statement_string, strlen(statement_string), "replace"); + callback_context *ctx = (callback_context *)user_arg; + assert(ctx != NULL); if (py_statement) { - ret = PyObject_CallOneArg((PyObject*)user_arg, py_statement); + ret = PyObject_CallOneArg(ctx->callable, py_statement); Py_DECREF(py_statement); } if (ret) { Py_DECREF(ret); } else { - pysqlite_state *state = pysqlite_get_state(NULL); - if (state->enable_callback_tracebacks) { + assert(ctx->state != NULL); + if (ctx->state->enable_callback_tracebacks) { PyErr_Print(); } else { @@ -1136,15 +1162,22 @@ pysqlite_connection_set_trace_callback_impl(pysqlite_Connection *self, #else sqlite3_trace(self->db, 0, (void*)0); #endif - Py_XSETREF(self->function_pinboard_trace_callback, NULL); - } else { + free_callback_context(self->trace_ctx); + self->trace_ctx = NULL; + } + else { + callback_context *ctx = create_callback_context(self->state, + trace_callback); + if (ctx == NULL) { + return NULL; + } #ifdef HAVE_TRACE_V2 - sqlite3_trace_v2(self->db, SQLITE_TRACE_STMT, _trace_callback, trace_callback); + sqlite3_trace_v2(self->db, SQLITE_TRACE_STMT, _trace_callback, ctx); #else - sqlite3_trace(self->db, _trace_callback, trace_callback); + sqlite3_trace(self->db, _trace_callback, ctx); #endif - Py_INCREF(trace_callback); - Py_XSETREF(self->function_pinboard_trace_callback, trace_callback); + free_callback_context(self->trace_ctx); + self->trace_ctx = ctx; } Py_RETURN_NONE; diff --git a/Modules/_sqlite/connection.h b/Modules/_sqlite/connection.h index 11b3a8005e1f95..d446b71ceca908 100644 --- a/Modules/_sqlite/connection.h +++ b/Modules/_sqlite/connection.h @@ -83,7 +83,7 @@ typedef struct PyObject* text_factory; /* remember references to object used in trace_callback/progress_handler/authorizer_cb */ - PyObject* function_pinboard_trace_callback; + callback_context *trace_ctx; PyObject* function_pinboard_progress_handler; PyObject* function_pinboard_authorizer_cb; From 8a9f9ab6b65f9828d5d864ba5162b15faa7ab3ba Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Thu, 29 Jul 2021 20:21:28 +0200 Subject: [PATCH 02/15] Adapt progress handler --- Modules/_sqlite/connection.c | 30 ++++++++++++++++++++---------- Modules/_sqlite/connection.h | 2 +- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index 1fa41075d2c61b..8ff8bf7b83edbe 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -170,7 +170,7 @@ pysqlite_connection_init_impl(pysqlite_Connection *self, self->check_same_thread = check_same_thread; self->trace_ctx = NULL; - self->function_pinboard_progress_handler = NULL; + self->progress_ctx = NULL; self->function_pinboard_authorizer_cb = NULL; self->Warning = state->Warning; @@ -232,7 +232,7 @@ connection_traverse(pysqlite_Connection *self, visitproc visit, void *arg) Py_VISIT(self->row_factory); Py_VISIT(self->text_factory); VISIT_CALLBACK_CONTEXT(self->trace_ctx); - Py_VISIT(self->function_pinboard_progress_handler); + VISIT_CALLBACK_CONTEXT(self->progress_ctx); Py_VISIT(self->function_pinboard_authorizer_cb); #undef VISIT_CALLBACK_CONTEXT return 0; @@ -254,7 +254,7 @@ connection_clear(pysqlite_Connection *self) Py_CLEAR(self->row_factory); Py_CLEAR(self->text_factory); CLEAR_CALLBACK_CONTEXT(self->trace_ctx); - Py_CLEAR(self->function_pinboard_progress_handler); + CLEAR_CALLBACK_CONTEXT(self->progress_ctx); Py_CLEAR(self->function_pinboard_authorizer_cb); #undef CLEAR_CALLBACK_CONTEXT return 0; @@ -274,6 +274,7 @@ static void free_callback_contexts(pysqlite_Connection *self) { free_callback_context(self->trace_ctx); + free_callback_context(self->progress_ctx); } static void @@ -988,7 +989,10 @@ static int _progress_handler(void* user_arg) PyGILState_STATE gilstate; gilstate = PyGILState_Ensure(); - ret = _PyObject_CallNoArg((PyObject*)user_arg); + + callback_context *ctx = (callback_context *)user_arg; + assert(ctx != NULL); + ret = _PyObject_CallNoArg(ctx->callable); if (!ret) { /* abort query if error occurred */ @@ -999,8 +1003,8 @@ static int _progress_handler(void* user_arg) Py_DECREF(ret); } if (rc < 0) { - pysqlite_state *state = pysqlite_get_state(NULL); - if (state->enable_callback_tracebacks) { + assert(ctx->state != NULL); + if (ctx->state->enable_callback_tracebacks) { PyErr_Print(); } else { @@ -1121,11 +1125,17 @@ pysqlite_connection_set_progress_handler_impl(pysqlite_Connection *self, if (progress_handler == Py_None) { /* None clears the progress handler previously set */ sqlite3_progress_handler(self->db, 0, 0, (void*)0); - Py_XSETREF(self->function_pinboard_progress_handler, NULL); + free_callback_context(self->progress_ctx); + self->progress_ctx = NULL; } else { - sqlite3_progress_handler(self->db, n, _progress_handler, progress_handler); - Py_INCREF(progress_handler); - Py_XSETREF(self->function_pinboard_progress_handler, progress_handler); + callback_context *ctx = create_callback_context(self->state, + progress_handler); + if (ctx == NULL) { + return NULL; + } + sqlite3_progress_handler(self->db, n, _progress_handler, ctx); + free_callback_context(self->progress_ctx); + self->progress_ctx = ctx; } Py_RETURN_NONE; } diff --git a/Modules/_sqlite/connection.h b/Modules/_sqlite/connection.h index d446b71ceca908..7ea0cf755f70a8 100644 --- a/Modules/_sqlite/connection.h +++ b/Modules/_sqlite/connection.h @@ -84,7 +84,7 @@ typedef struct /* remember references to object used in trace_callback/progress_handler/authorizer_cb */ callback_context *trace_ctx; - PyObject* function_pinboard_progress_handler; + callback_context *progress_ctx; PyObject* function_pinboard_authorizer_cb; /* Exception objects: borrowed refs. */ From 44c85b7fd497db8665aeb625c54afcc7324fa373 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Thu, 29 Jul 2021 20:29:16 +0200 Subject: [PATCH 03/15] Adapt authoriser callback --- Modules/_sqlite/connection.c | 36 +++++++++++++++++++++++------------- Modules/_sqlite/connection.h | 5 +++-- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index 8ff8bf7b83edbe..3eb37ce026d385 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -171,7 +171,7 @@ pysqlite_connection_init_impl(pysqlite_Connection *self, self->trace_ctx = NULL; self->progress_ctx = NULL; - self->function_pinboard_authorizer_cb = NULL; + self->authorizer_ctx = NULL; self->Warning = state->Warning; self->Error = state->Error; @@ -233,7 +233,7 @@ connection_traverse(pysqlite_Connection *self, visitproc visit, void *arg) Py_VISIT(self->text_factory); VISIT_CALLBACK_CONTEXT(self->trace_ctx); VISIT_CALLBACK_CONTEXT(self->progress_ctx); - Py_VISIT(self->function_pinboard_authorizer_cb); + VISIT_CALLBACK_CONTEXT(self->authorizer_ctx); #undef VISIT_CALLBACK_CONTEXT return 0; } @@ -255,7 +255,7 @@ connection_clear(pysqlite_Connection *self) Py_CLEAR(self->text_factory); CLEAR_CALLBACK_CONTEXT(self->trace_ctx); CLEAR_CALLBACK_CONTEXT(self->progress_ctx); - Py_CLEAR(self->function_pinboard_authorizer_cb); + CLEAR_CALLBACK_CONTEXT(self->authorizer_ctx); #undef CLEAR_CALLBACK_CONTEXT return 0; } @@ -275,6 +275,7 @@ free_callback_contexts(pysqlite_Connection *self) { free_callback_context(self->trace_ctx); free_callback_context(self->progress_ctx); + free_callback_context(self->authorizer_ctx); } static void @@ -945,11 +946,14 @@ static int _authorizer_callback(void* user_arg, int action, const char* arg1, co gilstate = PyGILState_Ensure(); - ret = PyObject_CallFunction((PyObject*)user_arg, "issss", action, arg1, arg2, dbname, access_attempt_source); + callback_context *ctx = (callback_context *)user_arg; + assert(ctx != NULL); + ret = PyObject_CallFunction(ctx->callable, "issss", action, arg1, arg2, + dbname, access_attempt_source); if (ret == NULL) { - pysqlite_state *state = pysqlite_get_state(NULL); - if (state->enable_callback_tracebacks) { + assert(ctx->state != NULL); + if (ctx->state->enable_callback_tracebacks) { PyErr_Print(); } else { @@ -962,8 +966,7 @@ static int _authorizer_callback(void* user_arg, int action, const char* arg1, co if (PyLong_Check(ret)) { rc = _PyLong_AsInt(ret); if (rc == -1 && PyErr_Occurred()) { - pysqlite_state *state = pysqlite_get_state(NULL); - if (state->enable_callback_tracebacks) { + if (ctx->state->enable_callback_tracebacks) { PyErr_Print(); } else { @@ -1087,17 +1090,24 @@ pysqlite_connection_set_authorizer_impl(pysqlite_Connection *self, int rc; if (authorizer_cb == Py_None) { rc = sqlite3_set_authorizer(self->db, NULL, NULL); - Py_XSETREF(self->function_pinboard_authorizer_cb, NULL); + free_callback_context(self->authorizer_ctx); + self->authorizer_ctx = NULL; } else { - Py_INCREF(authorizer_cb); - Py_XSETREF(self->function_pinboard_authorizer_cb, authorizer_cb); - rc = sqlite3_set_authorizer(self->db, _authorizer_callback, authorizer_cb); + callback_context *ctx = create_callback_context(self->state, + authorizer_cb); + if (ctx == NULL) { + return NULL; + } + rc = sqlite3_set_authorizer(self->db, _authorizer_callback, ctx); + free_callback_context(self->authorizer_ctx); + self->authorizer_ctx = ctx; } if (rc != SQLITE_OK) { PyErr_SetString(self->OperationalError, "Error setting authorizer callback"); - Py_XSETREF(self->function_pinboard_authorizer_cb, NULL); + free_callback_context(self->authorizer_ctx); + self->authorizer_ctx = NULL; return NULL; } Py_RETURN_NONE; diff --git a/Modules/_sqlite/connection.h b/Modules/_sqlite/connection.h index 7ea0cf755f70a8..9876b91f1d7a9a 100644 --- a/Modules/_sqlite/connection.h +++ b/Modules/_sqlite/connection.h @@ -82,10 +82,11 @@ typedef struct */ PyObject* text_factory; - /* remember references to object used in trace_callback/progress_handler/authorizer_cb */ + /* Remember callback contexts used by the trace_callback, progress_handler, + * and authorizer_cb handlers */ callback_context *trace_ctx; callback_context *progress_ctx; - PyObject* function_pinboard_authorizer_cb; + callback_context *authorizer_ctx; /* Exception objects: borrowed refs. */ PyObject* Warning; From 7f3c64b35863063f575f8c84c9fb15452e1e9f2d Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Thu, 29 Jul 2021 20:45:23 +0200 Subject: [PATCH 04/15] Add set context helper --- Modules/_sqlite/connection.c | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index 3eb37ce026d385..87d50c3625da5b 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -1070,6 +1070,14 @@ static void _trace_callback(void* user_arg, const char* statement_string) #endif } + +#define SET_CALLBACK_CONTEXT(ctx_ptr, ctx) \ +do { \ + callback_context *tmp = ctx_ptr; \ + ctx_ptr = ctx; \ + free_callback_context(tmp); \ +} while (0) + /*[clinic input] _sqlite3.Connection.set_authorizer as pysqlite_connection_set_authorizer @@ -1090,8 +1098,7 @@ pysqlite_connection_set_authorizer_impl(pysqlite_Connection *self, int rc; if (authorizer_cb == Py_None) { rc = sqlite3_set_authorizer(self->db, NULL, NULL); - free_callback_context(self->authorizer_ctx); - self->authorizer_ctx = NULL; + SET_CALLBACK_CONTEXT(self->authorizer_ctx, NULL); } else { callback_context *ctx = create_callback_context(self->state, @@ -1100,14 +1107,12 @@ pysqlite_connection_set_authorizer_impl(pysqlite_Connection *self, return NULL; } rc = sqlite3_set_authorizer(self->db, _authorizer_callback, ctx); - free_callback_context(self->authorizer_ctx); - self->authorizer_ctx = ctx; + SET_CALLBACK_CONTEXT(self->authorizer_ctx, ctx); } if (rc != SQLITE_OK) { PyErr_SetString(self->OperationalError, "Error setting authorizer callback"); - free_callback_context(self->authorizer_ctx); - self->authorizer_ctx = NULL; + SET_CALLBACK_CONTEXT(self->authorizer_ctx, NULL); return NULL; } Py_RETURN_NONE; @@ -1135,8 +1140,7 @@ pysqlite_connection_set_progress_handler_impl(pysqlite_Connection *self, if (progress_handler == Py_None) { /* None clears the progress handler previously set */ sqlite3_progress_handler(self->db, 0, 0, (void*)0); - free_callback_context(self->progress_ctx); - self->progress_ctx = NULL; + SET_CALLBACK_CONTEXT(self->progress_ctx, NULL); } else { callback_context *ctx = create_callback_context(self->state, progress_handler); @@ -1144,8 +1148,7 @@ pysqlite_connection_set_progress_handler_impl(pysqlite_Connection *self, return NULL; } sqlite3_progress_handler(self->db, n, _progress_handler, ctx); - free_callback_context(self->progress_ctx); - self->progress_ctx = ctx; + SET_CALLBACK_CONTEXT(self->progress_ctx, ctx); } Py_RETURN_NONE; } @@ -1182,8 +1185,7 @@ pysqlite_connection_set_trace_callback_impl(pysqlite_Connection *self, #else sqlite3_trace(self->db, 0, (void*)0); #endif - free_callback_context(self->trace_ctx); - self->trace_ctx = NULL; + SET_CALLBACK_CONTEXT(self->trace_ctx, NULL); } else { callback_context *ctx = create_callback_context(self->state, @@ -1196,13 +1198,14 @@ pysqlite_connection_set_trace_callback_impl(pysqlite_Connection *self, #else sqlite3_trace(self->db, _trace_callback, ctx); #endif - free_callback_context(self->trace_ctx); - self->trace_ctx = ctx; + SET_CALLBACK_CONTEXT(self->trace_ctx, ctx); } Py_RETURN_NONE; } +#undef SET_CALLBACK_CONTEXT + #ifndef SQLITE_OMIT_LOAD_EXTENSION /*[clinic input] _sqlite3.Connection.enable_load_extension as pysqlite_connection_enable_load_extension From 4880b73bd7e0f2f6764a44c4734cc00ef5d98367 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 25 Aug 2021 08:54:28 +0200 Subject: [PATCH 05/15] Free callback contexts after closing connection --- Modules/_sqlite/connection.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index 87d50c3625da5b..2a7f4233179cef 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -284,10 +284,10 @@ connection_dealloc(pysqlite_Connection *self) PyTypeObject *tp = Py_TYPE(self); PyObject_GC_UnTrack(self); tp->tp_clear((PyObject *)self); - free_callback_contexts(self); /* Clean up if user has not called .close() explicitly. */ connection_close(self); + free_callback_contexts(self); tp->tp_free(self); Py_DECREF(tp); From 3f5c0f4a7464e26ed0d6ac3d4af37cdc6caaaa5e Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 25 Aug 2021 09:07:47 +0200 Subject: [PATCH 06/15] Add print-or-clear-traceback helper --- Modules/_sqlite/connection.c | 54 ++++++++++++------------------------ 1 file changed, 18 insertions(+), 36 deletions(-) diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index 2a7f4233179cef..875ef095800694 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -624,6 +624,19 @@ _pysqlite_build_py_params(sqlite3_context *context, int argc, return NULL; } +static void +print_or_clear_traceback(callback_context *ctx) +{ + assert(ctx != NULL); + assert(ctx->state != NULL); + if (ctx->state->enable_callback_tracebacks) { + PyErr_Print(); + } + else { + PyErr_Clear(); + } +} + // Checks the Python exception and sets the appropriate SQLite error code. static void set_sqlite_error(sqlite3_context *context, const char *msg) @@ -639,14 +652,7 @@ set_sqlite_error(sqlite3_context *context, const char *msg) sqlite3_result_error(context, msg, -1); } callback_context *ctx = (callback_context *)sqlite3_user_data(context); - assert(ctx != NULL); - assert(ctx->state != NULL); - if (ctx->state->enable_callback_tracebacks) { - PyErr_Print(); - } - else { - PyErr_Clear(); - } + print_or_clear_traceback(ctx); } static void @@ -952,26 +958,14 @@ static int _authorizer_callback(void* user_arg, int action, const char* arg1, co dbname, access_attempt_source); if (ret == NULL) { - assert(ctx->state != NULL); - if (ctx->state->enable_callback_tracebacks) { - PyErr_Print(); - } - else { - PyErr_Clear(); - } - + print_or_clear_traceback(ctx); rc = SQLITE_DENY; } else { if (PyLong_Check(ret)) { rc = _PyLong_AsInt(ret); if (rc == -1 && PyErr_Occurred()) { - if (ctx->state->enable_callback_tracebacks) { - PyErr_Print(); - } - else { - PyErr_Clear(); - } + print_or_clear_traceback(ctx); rc = SQLITE_DENY; } } @@ -1006,13 +1000,7 @@ static int _progress_handler(void* user_arg) Py_DECREF(ret); } if (rc < 0) { - assert(ctx->state != NULL); - if (ctx->state->enable_callback_tracebacks) { - PyErr_Print(); - } - else { - PyErr_Clear(); - } + print_or_clear_traceback(ctx); } PyGILState_Release(gilstate); @@ -1055,13 +1043,7 @@ static void _trace_callback(void* user_arg, const char* statement_string) if (ret) { Py_DECREF(ret); } else { - assert(ctx->state != NULL); - if (ctx->state->enable_callback_tracebacks) { - PyErr_Print(); - } - else { - PyErr_Clear(); - } + print_or_clear_traceback(ctx); } PyGILState_Release(gilstate); From 71f66cac1e172ca74a3915eaddd0117cbd60bd48 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 25 Aug 2021 09:26:07 +0200 Subject: [PATCH 07/15] Ajdust comment --- Modules/_sqlite/connection.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Modules/_sqlite/connection.h b/Modules/_sqlite/connection.h index 9876b91f1d7a9a..c4cec857ddbfeb 100644 --- a/Modules/_sqlite/connection.h +++ b/Modules/_sqlite/connection.h @@ -82,8 +82,7 @@ typedef struct */ PyObject* text_factory; - /* Remember callback contexts used by the trace_callback, progress_handler, - * and authorizer_cb handlers */ + // Remember contexts used by the trace, progress, and authoriser callbacks callback_context *trace_ctx; callback_context *progress_ctx; callback_context *authorizer_ctx; From b8be98125616df0cbeb4fd28941aeda7ee4aa90b Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 25 Aug 2021 09:29:28 +0200 Subject: [PATCH 08/15] Use SET_CALLBACK_CONTEXT in __init__ --- Modules/_sqlite/connection.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index 875ef095800694..6b7cf41e39f4b0 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -77,6 +77,14 @@ new_statement_cache(pysqlite_Connection *self, int maxsize) return res; } + +#define SET_CALLBACK_CONTEXT(ctx_ptr, ctx) \ +do { \ + callback_context *tmp = ctx_ptr; \ + ctx_ptr = ctx; \ + free_callback_context(tmp); \ +} while (0) + /*[clinic input] _sqlite3.Connection.__init__ as pysqlite_connection_init @@ -169,9 +177,9 @@ pysqlite_connection_init_impl(pysqlite_Connection *self, self->thread_ident = PyThread_get_thread_ident(); self->check_same_thread = check_same_thread; - self->trace_ctx = NULL; - self->progress_ctx = NULL; - self->authorizer_ctx = NULL; + SET_CALLBACK_CONTEXT(self->trace_ctx, NULL); + SET_CALLBACK_CONTEXT(self->progress_ctx, NULL); + SET_CALLBACK_CONTEXT(self->authorizer_ctx, NULL); self->Warning = state->Warning; self->Error = state->Error; @@ -1052,14 +1060,6 @@ static void _trace_callback(void* user_arg, const char* statement_string) #endif } - -#define SET_CALLBACK_CONTEXT(ctx_ptr, ctx) \ -do { \ - callback_context *tmp = ctx_ptr; \ - ctx_ptr = ctx; \ - free_callback_context(tmp); \ -} while (0) - /*[clinic input] _sqlite3.Connection.set_authorizer as pysqlite_connection_set_authorizer From 6f6bf1d9e30bd32a0b7a675596a0b8a374466b9f Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Tue, 31 Aug 2021 14:40:55 +0200 Subject: [PATCH 09/15] Include minor PEP 7 adjustment --- Modules/_sqlite/connection.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index ea7d4a5e2866c2..0b07b7276ef260 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -1041,7 +1041,8 @@ static void _trace_callback(void* user_arg, const char* statement_string) if (ret) { Py_DECREF(ret); - } else { + } + else { print_or_clear_traceback(ctx); } From 3b971959980efcc121db98065b274360aba3e7e4 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Tue, 31 Aug 2021 18:25:40 +0200 Subject: [PATCH 10/15] Address review: explicitly initialise authoriser callback return code --- Modules/_sqlite/connection.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index 0b07b7276ef260..63fd916662d864 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -953,7 +953,7 @@ static int _authorizer_callback(void* user_arg, int action, const char* arg1, co PyGILState_STATE gilstate = PyGILState_Ensure(); PyObject *ret; - int rc; + int rc = SQLITE_DENY; callback_context *ctx = (callback_context *)user_arg; assert(ctx != NULL); From 83ab097a05e1191e35453aaad3b168a8dc4f4666 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 1 Sep 2021 09:44:04 +0200 Subject: [PATCH 11/15] Address review: implement CLEAR_CALLBACK_CONTEXT as a regular function --- Modules/_sqlite/connection.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index 63fd916662d864..1e2eac87b86dfb 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -248,12 +248,13 @@ connection_traverse(pysqlite_Connection *self, visitproc visit, void *arg) return 0; } -#define CLEAR_CALLBACK_CONTEXT(ctx) \ -do { \ - if (ctx) { \ - Py_CLEAR(ctx->callable); \ - } \ -} while (0) +static inline void +clear_callback_context(callback_context *ctx) +{ + if (ctx != NULL) { + Py_CLEAR(ctx->callable); + } +} static int connection_clear(pysqlite_Connection *self) @@ -263,10 +264,9 @@ connection_clear(pysqlite_Connection *self) Py_CLEAR(self->cursors); Py_CLEAR(self->row_factory); Py_CLEAR(self->text_factory); - CLEAR_CALLBACK_CONTEXT(self->trace_ctx); - CLEAR_CALLBACK_CONTEXT(self->progress_ctx); - CLEAR_CALLBACK_CONTEXT(self->authorizer_ctx); -#undef CLEAR_CALLBACK_CONTEXT + clear_callback_context(self->trace_ctx); + clear_callback_context(self->progress_ctx); + clear_callback_context(self->authorizer_ctx); return 0; } From 32c2073e4da68086702ae15c6f794b2130a1b02d Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Tue, 7 Sep 2021 14:12:18 +0200 Subject: [PATCH 12/15] Fix merge --- Modules/_sqlite/connection.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index eaa686520f9a36..0ebd0191d23d24 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -998,7 +998,7 @@ progress_callback(void *ctx) assert(ctx != NULL); PyObject *callable = ((callback_context *)ctx)->callable; - ret = _PyObject_CallNoArg(ctx->callable); + ret = _PyObject_CallNoArg(callable); if (!ret) { /* abort query if error occurred */ rc = -1; @@ -1042,7 +1042,6 @@ trace_callback(void *ctx, const char *statement_string) PyObject *ret = NULL; py_statement = PyUnicode_DecodeUTF8(statement_string, strlen(statement_string), "replace"); - callback_context *ctx = (callback_context *)user_arg; assert(ctx != NULL); if (py_statement) { PyObject *callable = ((callback_context *)ctx)->callable; @@ -1124,12 +1123,13 @@ pysqlite_connection_set_progress_handler_impl(pysqlite_Connection *self, /* None clears the progress handler previously set */ sqlite3_progress_handler(self->db, 0, 0, (void*)0); SET_CALLBACK_CONTEXT(self->progress_ctx, NULL); - } else { + } + else { callback_context *ctx = create_callback_context(self->state, callable); if (ctx == NULL) { return NULL; } - sqlite3_progress_handler(self->db, n, progress_handler, ctx); + sqlite3_progress_handler(self->db, n, progress_callback, ctx); SET_CALLBACK_CONTEXT(self->progress_ctx, ctx); } Py_RETURN_NONE; @@ -1179,7 +1179,7 @@ pysqlite_connection_set_trace_callback_impl(pysqlite_Connection *self, #else sqlite3_trace(self->db, trace_callback, ctx); #endif - SET_CALLBACK_CONTEXT(self->trace_ctx, callable); + SET_CALLBACK_CONTEXT(self->trace_ctx, ctx); } Py_RETURN_NONE; From cb4063a31180d88ed36fa8d809741525dac35199 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Tue, 7 Sep 2021 14:30:29 +0200 Subject: [PATCH 13/15] Make SET_CALLBACK_CONTEXT a regular function --- Modules/_sqlite/connection.c | 46 ++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index 0ebd0191d23d24..bdea01118deb28 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -56,6 +56,8 @@ static const char * const begin_statements[] = { static int pysqlite_connection_set_isolation_level(pysqlite_Connection* self, PyObject* isolation_level, void *Py_UNUSED(ignored)); static void _pysqlite_drop_unused_cursor_references(pysqlite_Connection* self); static void free_callback_context(callback_context *ctx); +static void set_callback_context(callback_context **ctx_pp, + callback_context *ctx); static PyObject * new_statement_cache(pysqlite_Connection *self, int maxsize) @@ -80,14 +82,16 @@ new_statement_cache(pysqlite_Connection *self, int maxsize) } -#define SET_CALLBACK_CONTEXT(ctx_ptr, ctx) \ -do { \ - callback_context *tmp = ctx_ptr; \ - ctx_ptr = ctx; \ - if (tmp) { \ - free_callback_context(tmp); \ - } \ -} while (0) +static void +set_callback_context(callback_context **ctx_pp, callback_context *ctx) +{ + assert(ctx_pp != NULL); + callback_context *tmp = *ctx_pp; + *ctx_pp = ctx; + if (tmp != NULL) { + free_callback_context(tmp); + } +} /*[clinic input] _sqlite3.Connection.__init__ as pysqlite_connection_init @@ -181,9 +185,9 @@ pysqlite_connection_init_impl(pysqlite_Connection *self, self->thread_ident = PyThread_get_thread_ident(); self->check_same_thread = check_same_thread; - SET_CALLBACK_CONTEXT(self->trace_ctx, NULL); - SET_CALLBACK_CONTEXT(self->progress_ctx, NULL); - SET_CALLBACK_CONTEXT(self->authorizer_ctx, NULL); + set_callback_context(&self->trace_ctx, NULL); + set_callback_context(&self->progress_ctx, NULL); + set_callback_context(&self->authorizer_ctx, NULL); self->Warning = state->Warning; self->Error = state->Error; @@ -285,9 +289,9 @@ connection_close(pysqlite_Connection *self) static void free_callback_contexts(pysqlite_Connection *self) { - SET_CALLBACK_CONTEXT(self->trace_ctx, NULL); - SET_CALLBACK_CONTEXT(self->progress_ctx, NULL); - SET_CALLBACK_CONTEXT(self->authorizer_ctx, NULL); + set_callback_context(&self->trace_ctx, NULL); + set_callback_context(&self->progress_ctx, NULL); + set_callback_context(&self->authorizer_ctx, NULL); } static void @@ -1082,7 +1086,7 @@ pysqlite_connection_set_authorizer_impl(pysqlite_Connection *self, int rc; if (callable == Py_None) { rc = sqlite3_set_authorizer(self->db, NULL, NULL); - SET_CALLBACK_CONTEXT(self->authorizer_ctx, NULL); + set_callback_context(&self->authorizer_ctx, NULL); } else { callback_context *ctx = create_callback_context(self->state, callable); @@ -1090,12 +1094,12 @@ pysqlite_connection_set_authorizer_impl(pysqlite_Connection *self, return NULL; } rc = sqlite3_set_authorizer(self->db, authorizer_callback, ctx); - SET_CALLBACK_CONTEXT(self->authorizer_ctx, ctx); + set_callback_context(&self->authorizer_ctx, ctx); } if (rc != SQLITE_OK) { PyErr_SetString(self->OperationalError, "Error setting authorizer callback"); - SET_CALLBACK_CONTEXT(self->authorizer_ctx, NULL); + set_callback_context(&self->authorizer_ctx, NULL); return NULL; } Py_RETURN_NONE; @@ -1122,7 +1126,7 @@ pysqlite_connection_set_progress_handler_impl(pysqlite_Connection *self, if (callable == Py_None) { /* None clears the progress handler previously set */ sqlite3_progress_handler(self->db, 0, 0, (void*)0); - SET_CALLBACK_CONTEXT(self->progress_ctx, NULL); + set_callback_context(&self->progress_ctx, NULL); } else { callback_context *ctx = create_callback_context(self->state, callable); @@ -1130,7 +1134,7 @@ pysqlite_connection_set_progress_handler_impl(pysqlite_Connection *self, return NULL; } sqlite3_progress_handler(self->db, n, progress_callback, ctx); - SET_CALLBACK_CONTEXT(self->progress_ctx, ctx); + set_callback_context(&self->progress_ctx, ctx); } Py_RETURN_NONE; } @@ -1167,7 +1171,7 @@ pysqlite_connection_set_trace_callback_impl(pysqlite_Connection *self, #else sqlite3_trace(self->db, 0, (void*)0); #endif - SET_CALLBACK_CONTEXT(self->trace_ctx, NULL); + set_callback_context(&self->trace_ctx, NULL); } else { callback_context *ctx = create_callback_context(self->state, callable); @@ -1179,7 +1183,7 @@ pysqlite_connection_set_trace_callback_impl(pysqlite_Connection *self, #else sqlite3_trace(self->db, trace_callback, ctx); #endif - SET_CALLBACK_CONTEXT(self->trace_ctx, ctx); + set_callback_context(&self->trace_ctx, ctx); } Py_RETURN_NONE; From e0df824aa9c86466a02fa2f6917075d4935fc02b Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Tue, 7 Sep 2021 14:33:28 +0200 Subject: [PATCH 14/15] Let set_callback_context live with the other context functions --- Modules/_sqlite/connection.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index bdea01118deb28..3e3b5d335cccc3 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -82,17 +82,6 @@ new_statement_cache(pysqlite_Connection *self, int maxsize) } -static void -set_callback_context(callback_context **ctx_pp, callback_context *ctx) -{ - assert(ctx_pp != NULL); - callback_context *tmp = *ctx_pp; - *ctx_pp = ctx; - if (tmp != NULL) { - free_callback_context(tmp); - } -} - /*[clinic input] _sqlite3.Connection.__init__ as pysqlite_connection_init @@ -846,6 +835,17 @@ free_callback_context(callback_context *ctx) PyMem_Free(ctx); } +static void +set_callback_context(callback_context **ctx_pp, callback_context *ctx) +{ + assert(ctx_pp != NULL); + callback_context *tmp = *ctx_pp; + *ctx_pp = ctx; + if (tmp != NULL) { + free_callback_context(tmp); + } +} + static void destructor_callback(void *ctx) { From 5ff195162c59ee09efab0701e54df528aefe786a Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Tue, 7 Sep 2021 14:37:13 +0200 Subject: [PATCH 15/15] Remove spurious line break --- Modules/_sqlite/connection.c | 1 - 1 file changed, 1 deletion(-) diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index 3e3b5d335cccc3..bf803370c05719 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -81,7 +81,6 @@ new_statement_cache(pysqlite_Connection *self, int maxsize) return res; } - /*[clinic input] _sqlite3.Connection.__init__ as pysqlite_connection_init