gh-145040: Fix crash in sqlite3 when connection is closed from within a callback

[raminfp]

Summary

Fix a segmentation fault (NULL pointer dereference) in the _sqlite module that occurs when Connection.close() is called inside an aggregate step() callback.

Fix

1. Primary check – immediately after stmt_step(), check if self->connection->db == NULL. If so, raise ProgrammingError("Cannot operate on a closed database.") and jump to the error path.
2. Defensive check – guard the sqlite3_last_insert_rowid() call with self->connection->db != NULL as an extra safety net.

Test

Added test_aggr_close_conn_in_step in Lib/test/test_sqlite3/test_userfunctions.py that reproduces the exact crash scenario and verifies that ProgrammingError is raised instead of a segfault.

ASAN output (before fix)

AddressSanitizer: SEGV on unknown address 0x000000000038
  #0 in sqlite3_last_insert_rowid
  #1 in _pysqlite_query_execute Modules/_sqlite/cursor.c:974

After fix

sqlite3.ProgrammingError: Cannot operate on a closed database.

• Issue: _sqlite: NULL dereference when connection is closed from within a callback #145040

[erlend-aasland]

Please update the PR title to more accurately reflect the problem it addresses. Ditto for the NEWS item.

[raminfp]

Sorry for force push!!!

[erlend-aasland]

This PR adds a sanity check after the damage has been done. The SQLite C API clearly tells us what's legal and illegal operations on the sqlite3 * pointer during these callbacks. Consider an alternative approach: how about preventing the API misuse in the first place? That would probably require adding a flag to the connection object, and some machinery for setting/reading it, but it could possibly make for a better experience for the developer. I'm not sure the complexity would be worth it, though. WDYT?

[raminfp]

I agree, preventing the misuse upfront is the better approach. I've pushed a new commit that does exactly that:

• Added an in_callback counter to the connection object, incremented/decremented around stmt_step() calls.
• Connection.close() now checks this counter and raises ProgrammingError before touching the database, keeping the sqlite3 * pointer valid.
• Used a counter (not a boolean) to correctly handle nested callbacks (e.g a UDF that triggers another query).

The complexity turned out to be quite manageable, just a few lines in close(), and ++/-- around the two stmt_step() call sites.

[erlend-aasland]

We should be able to exploit the already present cursor->locked member of pysqlite_Cursor, @raminfp. This flag is set during the complete query orchestration and during the iter implementation. Unfortunately, the list of created cursors were recently purged in #144378, so we have no way of checking if a cursor is locked from within the connection close() method.

[erlend-aasland]

We should be able to exploit the already present cursor->locked member of pysqlite_Cursor […]

I did a quick PoC, and this indeed works. It's a small change, and it works for both the iternext and the query execute scenarios. However, it requires we roll back parts of 74c1f41. If we choose this path, we should do the rolling back as a separate PR first.

[raminfp]

@erlend-aasland I implemented your suggested approach. Here's what I did:

Removed in_callback counter, now using cursor->locked instead:

1. Rolled back parts of 74c1f41 - restored self->cursors weakref list and register_cursor() (without the old created_cursors counter / _pysqlite_drop_unused_cursor_references cleanup).
2. close() now iterates the cursor list and checks cursor->locked instead of checking in_callback > 0.
3. Extended locked region in cursor_iternext to cover stmt_step() - previously locked was only set during _pysqlite_fetch_one_row(), not during stmt_step() where callbacks actually fire.
4. Kept close_attempted_in_callback flag - this is still needed because when close() raises ProgrammingError inside a callback, SQLite's callback wrapper clears the Python exception and converts it to OperationalError. The flag lets us re-raise the correct ProgrammingError after stmt_step() returns.
5. Added any_other_cursor_locked() helper - for nested callback scenarios (e.g. test_close_conn_in_nested_callback_caught), we only clear close_attempted_in_callback at the outermost cursor level.
6. Added stmt_reset() before Py_CLEAR(self->statement) in iternext - when stmt_step is interrupted by a progress handler, the statement stays busy. Without resetting first, sqlite3_finalize during shutdown would crash.

All 509 test_sqlite3 tests pass.