From 249d9c691e668642e292c67f793802ed58d5910d Mon Sep 17 00:00:00 2001 From: nicky2pc <147145767+nicky2pc@users.noreply.github.com> Date: Fri, 24 Jul 2026 10:38:51 +0500 Subject: [PATCH] gh-153800: Fix crash in sqlite3 window function empty-frame callbacks SQLite may invoke a user-defined window function's ``value()`` or ``inverse()`` callback before ``step()`` has run for a given aggregate instance -- for example when the window frame is empty for an output row (``ROWS BETWEEN 1 PRECEDING AND 1 PRECEDING`` on the first row). In that case the aggregate context slot is still zero-initialised, so the callbacks dereferenced a NULL pointer: SIGSEGV on release builds, and an ``assert(*cls != NULL)`` failure on debug builds. Create the aggregate instance lazily in ``value_callback`` and ``inverse_callback``, mirroring the existing behaviour of ``step_callback``. The shared get-or-create logic is factored out into a ``get_or_create_aggregate_instance`` helper. Co-Authored-By: Claude Opus 4.8 --- Lib/test/test_sqlite3/test_userfunctions.py | 22 ++++++ ...-07-24-11-05-00.gh-issue-153800.KZauju.rst | 5 ++ Modules/_sqlite/connection.c | 70 +++++++++++++------ 3 files changed, 74 insertions(+), 23 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2026-07-24-11-05-00.gh-issue-153800.KZauju.rst diff --git a/Lib/test/test_sqlite3/test_userfunctions.py b/Lib/test/test_sqlite3/test_userfunctions.py index 11cf877a011c785..fc30fb13940602b 100644 --- a/Lib/test/test_sqlite3/test_userfunctions.py +++ b/Lib/test/test_sqlite3/test_userfunctions.py @@ -492,6 +492,28 @@ def test_win_error_on_create(self): with self.assertRaisesRegex(sqlite.ProgrammingError, "not -100"): self.con.create_window_function("shouldfail", -100, WindowSumInt) + def test_win_value_on_empty_frame(self): + # gh-153800: when a window frame is empty for a given output row, + # SQLite may invoke the value() (or inverse()) callback before step() + # has ever run for that aggregate instance. This must not crash. + class WindowConst: + def step(self, value): pass + def inverse(self, value): pass + def value(self): return 42 + def finalize(self): return 42 + + self.con.create_window_function("winconst", 1, WindowConst) + # The frame "1 preceding and 1 preceding" is empty for the first row, + # so value() is called before any step() call for that row. + self.cur.execute(""" + select x, winconst(y) over ( + order by x rows between 1 preceding and 1 preceding + ) from test order by x + """) + self.assertEqual(self.cur.fetchall(), + [("a", 42), ("b", 42), ("c", 42), + ("d", 42), ("e", 42)]) + @with_tracebacks(BadWindow) def test_win_exception_in_method(self): for meth in "__init__", "step", "value", "inverse": diff --git a/Misc/NEWS.d/next/Library/2026-07-24-11-05-00.gh-issue-153800.KZauju.rst b/Misc/NEWS.d/next/Library/2026-07-24-11-05-00.gh-issue-153800.KZauju.rst new file mode 100644 index 000000000000000..af8367b2d5a548d --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-07-24-11-05-00.gh-issue-153800.KZauju.rst @@ -0,0 +1,5 @@ +Fix a crash in :mod:`sqlite3` window functions. The ``value()`` and +``inverse()`` aggregate callbacks no longer dereference a ``NULL`` pointer +when SQLite invokes them for an empty window frame before ``step()`` has run +for that aggregate instance; the aggregate instance is now created lazily, as +it already was for ``step()``. diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index 892740b05e55c98..2b7439df331b6a2 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -978,6 +978,37 @@ func_callback(sqlite3_context *context, int argc, sqlite3_value **argv) PyGILState_Release(threadstate); } +/* Fetch the aggregate instance for a window/aggregate callback, lazily + * creating it on first use. SQLite may invoke the value() or inverse() + * callback before step() has run for a given aggregate instance -- e.g. when + * the window frame is empty for an output row -- in which case the aggregate + * context is still zero-initialised (gh-153800). Returns a borrowed reference + * owned by the aggregate context, or NULL with a SQLite error set on failure. + */ +static PyObject * +get_or_create_aggregate_instance(sqlite3_context *context, + callback_context *ctx) +{ + PyObject **aggregate_instance; + + aggregate_instance = (PyObject **)sqlite3_aggregate_context(context, + sizeof(PyObject *)); + if (aggregate_instance == NULL) { + (void)PyErr_NoMemory(); + set_sqlite_error(context, "unable to allocate SQLite aggregate context"); + return NULL; + } + if (*aggregate_instance == NULL) { + *aggregate_instance = PyObject_CallNoArgs(ctx->callable); + if (*aggregate_instance == NULL) { + set_sqlite_error(context, + "user-defined aggregate's '__init__' method raised error"); + return NULL; + } + } + return *aggregate_instance; +} + static void step_callback(sqlite3_context *context, int argc, sqlite3_value **params) { @@ -985,28 +1016,18 @@ step_callback(sqlite3_context *context, int argc, sqlite3_value **params) PyObject* args; PyObject* function_result = NULL; - PyObject** aggregate_instance; + PyObject* aggregate_instance; PyObject* stepmethod = NULL; callback_context *ctx = (callback_context *)sqlite3_user_data(context); incref_callback_context(ctx); - aggregate_instance = (PyObject**)sqlite3_aggregate_context(context, sizeof(PyObject*)); + aggregate_instance = get_or_create_aggregate_instance(context, ctx); if (aggregate_instance == NULL) { - (void)PyErr_NoMemory(); - set_sqlite_error(context, "unable to allocate SQLite aggregate context"); goto error; } - if (*aggregate_instance == NULL) { - *aggregate_instance = PyObject_CallNoArgs(ctx->callable); - if (!*aggregate_instance) { - set_sqlite_error(context, - "user-defined aggregate's '__init__' method raised error"); - goto error; - } - } - stepmethod = PyObject_GetAttr(*aggregate_instance, ctx->state->str_step); + stepmethod = PyObject_GetAttr(aggregate_instance, ctx->state->str_step); if (!stepmethod) { set_sqlite_error(context, "user-defined aggregate's 'step' method not defined"); @@ -1249,12 +1270,13 @@ inverse_callback(sqlite3_context *context, int argc, sqlite3_value **params) callback_context *ctx = (callback_context *)sqlite3_user_data(context); incref_callback_context(ctx); - int size = sizeof(PyObject *); - PyObject **cls = (PyObject **)sqlite3_aggregate_context(context, size); - assert(cls != NULL); - assert(*cls != NULL); + PyObject *method = NULL; + PyObject *aggregate_instance = get_or_create_aggregate_instance(context, ctx); + if (aggregate_instance == NULL) { + goto exit; + } - PyObject *method = PyObject_GetAttr(*cls, ctx->state->str_inverse); + method = PyObject_GetAttr(aggregate_instance, ctx->state->str_inverse); if (method == NULL) { set_sqlite_error(context, "user-defined aggregate's 'inverse' method not defined"); @@ -1298,12 +1320,14 @@ value_callback(sqlite3_context *context) callback_context *ctx = (callback_context *)sqlite3_user_data(context); incref_callback_context(ctx); - int size = sizeof(PyObject *); - PyObject **cls = (PyObject **)sqlite3_aggregate_context(context, size); - assert(cls != NULL); - assert(*cls != NULL); + PyObject *aggregate_instance = get_or_create_aggregate_instance(context, ctx); + if (aggregate_instance == NULL) { + decref_callback_context(ctx); + PyGILState_Release(gilstate); + return; + } - PyObject *res = PyObject_CallMethodNoArgs(*cls, ctx->state->str_value); + PyObject *res = PyObject_CallMethodNoArgs(aggregate_instance, ctx->state->str_value); decref_callback_context(ctx); if (res == NULL) {