diff --git a/tests/README.md b/tests/README.md new file mode 100644 index 0000000..844ca72 --- /dev/null +++ b/tests/README.md @@ -0,0 +1,18 @@ +# usqlite regression tests + +Host-side tests, run against a MicroPython unix build that includes this +module. On the Pico Computer 3 tree that is the `pc3` variant, and the heap +size must match the board (8 MB) so memory behaviour is faithful: + + cd ports/unix + ./build-pc3/micropython -X heapsize=8m ../../lib/usqlite/tests/test_smoke.py + +All scripts print `... PASSED` on success and use `/tmp` for their databases. + +| script | covers | +|-------------------|--------| +| `test_smoke.py` | basic CRUD, executemany error-path leak, connection.close() with live/closed cursors, GC finaliser reclaim, row/dict/named-parameter binding | +| `test_simreset.py`| the soft-reset session cycle: `usqlite.__init__()` + `gc.collect()` simulate what a bare-metal soft reset does (roots are NOT auto-zeroed; the module `__init__` clears them on the next import), then the new session must reinitialize and close cleanly | +| `test_fix456.py` | 64-bit INTEGER bind/read roundtrip, `.description` with computed columns (NULL decltype) and before first fetch, VFS errors surfacing as `usqlite_Error` (not exceptions unwinding SQLite), raising trace callback swallowed | +| `test_crash.py` | power-fail recovery: snapshots of db+journal taken mid-transaction and post-commit are reopened; mid-transaction must roll back exactly, post-commit must keep data, a garbage journal must be ignored (`PRAGMA integrity_check` throughout) | +| `test_churn.py` | heavy fetchall + gc.collect churn under an open connection, then close (the historical GC-vs-SQLite corruption pattern) | diff --git a/tests/test_churn.py b/tests/test_churn.py new file mode 100644 index 0000000..d3c5e06 --- /dev/null +++ b/tests/test_churn.py @@ -0,0 +1,29 @@ +import usqlite, gc, os + +try: + os.remove("/tmp/phaseb.db") +except OSError: + pass + +db = usqlite.connect("/tmp/phaseb.db") +db.execute("CREATE TABLE t (id INTEGER PRIMARY KEY, name TEXT, v REAL)") +db.execute("BEGIN") +for i in range(100): + db.execute("INSERT INTO t (name, v) VALUES (?, ?)", ("row%d" % i, i * 1.5)) +db.execute("COMMIT") +print("count:", db.execute("SELECT COUNT(*) FROM t").fetchone()[0]) + +# exact phase-B shape: COUNT fetchone + 20x (fetchall + churn + collect) +row = db.execute("SELECT COUNT(*), SUM(v) FROM t").fetchone() +assert row == (100, 7425.0), row +for k in range(20): + x = ["pad%d" % i for i in range(2000)] + rows = db.execute("SELECT * FROM t ORDER BY v DESC").fetchall() + assert len(rows) == 100 and rows[0][2] == 148.5 + gc.collect() +print("churn done, mem:", usqlite.mem_current()) +print("closing...") +db.close() +print("closed ok") +os.remove("/tmp/phaseb.db") +print("PHASE-B REPLICA PASSED") diff --git a/tests/test_crash.py b/tests/test_crash.py new file mode 100644 index 0000000..0d3f456 --- /dev/null +++ b/tests/test_crash.py @@ -0,0 +1,92 @@ +import usqlite, os + +DB = "/tmp/crash.db" + +def rm(p): + try: + os.remove(p) + except OSError: + pass + +def cp(src, dst): + rm(dst) + try: + with open(src, "rb") as s, open(dst, "wb") as d: + while True: + b = s.read(4096) + if not b: + break + d.write(b) + except OSError: + pass # source may not exist (no journal) + +def snapshot(tag): + cp(DB, "/tmp/%s.db" % tag) + cp(DB + "-journal", "/tmp/%s.db-journal" % tag) + +def content(path): + db = usqlite.connect(path) + ic = db.execute("PRAGMA integrity_check").fetchone()[0] + row = db.execute("SELECT COUNT(*), SUM(v), SUM(LENGTH(pad)) FROM t").fetchone() + db.close() + return ic, row + +for f in os.listdir("/tmp"): + if f.startswith("crash") and (f.endswith(".db") or f.endswith("-journal")): + rm("/tmp/" + f) + +# --- baseline: committed state ------------------------------------------- +db = usqlite.connect(DB) +db.execute("CREATE TABLE t (id INTEGER PRIMARY KEY, v INTEGER, pad TEXT)") +db.execute("BEGIN") +for i in range(100): + db.execute("INSERT INTO t VALUES (?, ?, ?)", (i, i, "a" * 200)) +db.execute("COMMIT") +db.close() +baseline = content(DB) +print("baseline:", baseline) +assert baseline[0] == "ok" and baseline[1][0] == 100 + +# --- big uncommitted transaction, snapshot mid-flight --------------------- +db = usqlite.connect(DB) +db.execute("PRAGMA cache_size=10") # tiny cache -> spill writes db mid-txn +db.execute("BEGIN") +for i in range(100): + db.execute("UPDATE t SET v = v + 1000000, pad = ? WHERE id = ?", + ("z" * 2000, i)) +jsz = os.stat(DB + "-journal")[6] +print("mid-txn journal bytes:", jsz) +assert jsz > 0, "journal must exist mid-transaction" +snapshot("crashA") # power cut mid-transaction + +db.execute("COMMIT") +snapshot("crashB") # power cut just after commit +db.close() +committed = content(DB) +print("committed:", committed) +assert committed[0] == "ok" and committed[1][1] == sum(range(100)) + 100 * 1000000 + +# --- recovery A: mid-transaction cut must roll back to baseline ----------- +recA = content("/tmp/crashA.db") +print("crashA recovered:", recA) +assert recA == baseline, (recA, baseline) +try: + os.stat("/tmp/crashA.db-journal") + print("note: crashA journal still present after recovery") +except OSError: + print("crashA journal cleaned up") + +# --- recovery B: post-commit cut must keep the committed data ------------- +recB = content("/tmp/crashB.db") +print("crashB recovered:", recB) +assert recB == committed, (recB, committed) + +# --- garbage journal next to a healthy db is ignored ---------------------- +cp(DB, "/tmp/crashC.db") +with open("/tmp/crashC.db-journal", "wb") as f: + f.write(bytes(range(256)) * 16) +recC = content("/tmp/crashC.db") +print("crashC (garbage journal):", recC) +assert recC == committed, (recC, committed) + +print("CRASH RECOVERY TESTS PASSED") diff --git a/tests/test_fix456.py b/tests/test_fix456.py new file mode 100644 index 0000000..483a138 --- /dev/null +++ b/tests/test_fix456.py @@ -0,0 +1,52 @@ +import usqlite, gc, os + +try: + os.remove("/tmp/t456.db") +except OSError: + pass + +db = usqlite.connect("/tmp/t456.db") + +# --- fix 4: 64-bit INTEGER bind + read ------------------------------------ +db.execute("CREATE TABLE big (id INTEGER PRIMARY KEY, v INTEGER)") +vals = [2**40, 1753305600123, -2**45, 2**62, -1, 0, 2**31, -(2**31) - 1] +for i, v in enumerate(vals): + db.execute("INSERT INTO big VALUES (?, ?)", (i, v)) +back = [r[0] for r in db.execute("SELECT v FROM big ORDER BY id")] +assert back == vals, (back, vals) +row = db.execute("SELECT SUM(v) FROM big").fetchone() +assert row[0] == sum(vals), row +print("4: 64-bit roundtrip ok:", back[:3], "...") + +# --- fix 5: decltype NULL + description before fetch ---------------------- +cur = db.cursor() +cur.execute("SELECT COUNT(*), v + 1, v FROM big") +d = cur.description # computed columns -> decltype NULL; was a crash +assert len(d) == 3, d +assert d[0][1] is None and d[1][1] is None and d[2][1] == "INTEGER", d +cur.fetchall() +cur2 = db.execute("SELECT id, v FROM big") +d2 = cur2.description # before any fetch consumed rows -- was empty +assert len(d2) == 2 and d2[0][0] == "id", d2 +cur2.fetchall() +print("5: description/decltype ok") + +# --- fix 6: VFS errors as codes, not exceptions --------------------------- +try: + usqlite.connect("/tmp/nosuchdir/x.db") + raise SystemExit("expected connect to fail") +except usqlite.usqlite_Error as e: + print("6: bad-path connect -> usqlite_Error:", e) +# engine must still be fully usable afterwards +assert db.execute("SELECT COUNT(*) FROM big").fetchone()[0] == len(vals) + +def bad_trace(stmt): + raise ValueError("boom") +db.set_trace_callback(bad_trace) +assert db.execute("SELECT COUNT(*) FROM big").fetchone()[0] == len(vals) +db.set_trace_callback(None) +print("6: raising trace callback swallowed ok") + +db.close() +os.remove("/tmp/t456.db") +print("FIX 4/5/6 TESTS PASSED") diff --git a/tests/test_simreset.py b/tests/test_simreset.py new file mode 100644 index 0000000..9c768a6 --- /dev/null +++ b/tests/test_simreset.py @@ -0,0 +1,43 @@ +import usqlite, gc, os + +try: + os.remove("/tmp/simreset.db") +except OSError: + pass + +# --- session 1: mirror sqltest_a end state --------------------------------- +db = usqlite.connect("/tmp/simreset.db") +db.execute("CREATE TABLE t (id INTEGER PRIMARY KEY, name TEXT, v REAL)") +db.execute("BEGIN") +for i in range(100): + db.execute("INSERT INTO t (name, v) VALUES (?, ?)", ("row%d" % i, i * 1.5)) +db.execute("COMMIT") +dbopen = db # connection deliberately left open across the "reset" +print("session 1 up, mem:", usqlite.mem_current()) + +# --- simulated soft reset --------------------------------------------------- +# Faithful to rp2: root pointers are NOT zeroed by the reset itself. What a +# new session does is (a) sweep all objects (finalisers run) and (b) call the +# module __init__ on its first import, which forgets the dead session's state. +del db, dbopen +gc.collect() # ~ gc_sweep_all() at soft_reset_exit +usqlite.__init__() # ~ first `import usqlite` of the new session +gc.collect() # old pool is now unreferenced garbage +print("simulated soft reset done") + +# --- session 2: mirror sqltest_b ------------------------------------------- +db = usqlite.connect("/tmp/simreset.db") +row = db.execute("SELECT COUNT(*), SUM(v) FROM t").fetchone() +print("B1 count/sum:", row) +assert row == (100, 7425.0), row +for k in range(5): + x = ["pad%d" % i for i in range(2000)] + rows = db.execute("SELECT * FROM t ORDER BY v DESC").fetchall() + assert len(rows) == 100 and rows[0][2] == 148.5 + gc.collect() +print("B2 churn ok, mem:", usqlite.mem_current()) +print("B3 closing...") +db.close() +print("B3 closed ok") +os.remove("/tmp/simreset.db") +print("SIM-RESET TEST PASSED") diff --git a/tests/test_smoke.py b/tests/test_smoke.py new file mode 100644 index 0000000..fc70ac0 --- /dev/null +++ b/tests/test_smoke.py @@ -0,0 +1,70 @@ +import usqlite, gc, os + +try: + os.remove("/tmp/smoke.db") +except OSError: + pass + +# --- basic sanity --------------------------------------------------------- +db = usqlite.connect("/tmp/smoke.db") +db.execute("CREATE TABLE t (id INTEGER PRIMARY KEY, name TEXT, v REAL)") +db.executemany( + "INSERT INTO t (name, v) VALUES ('a', 1.5);" + "INSERT INTO t (name, v) VALUES ('b', 2.5)") +cur = db.execute("SELECT * FROM t ORDER BY id") +rows = cur.fetchall() +assert rows == [(1, 'a', 1.5), (2, 'b', 2.5)], rows +print("basic ok") + +# --- fix 9: failed executemany must not leak the error message ------------ +gc.collect() +base = usqlite.mem_current() +for i in range(50): + try: + db.executemany("INSERT INTO nosuch VALUES (1)") + raise SystemExit("expected an error") + except usqlite.usqlite_Error: + pass +gc.collect() +leak = usqlite.mem_current() - base +print("executemany 50-error leak bytes:", leak) +assert leak < 512, leak + +# --- fix 2: connection.close() with live / explicitly-closed cursors ------ +cur2 = db.execute("SELECT * FROM t") # unexhausted -> still registered +cur3 = db.execute("SELECT * FROM t") +cur3.close() # explicitly closed -> must untrack +db.close() +gc.collect() +for i in range(2000): # churn the heap: UAF would corrupt/crash + x = [i] * 8 +print("post-close rowcounts:", cur2.rowcount, cur3.rowcount) +assert cur2.fetchone() is None +assert list(cur2) == [] +print("close-with-live-cursors ok") + +# --- fix 8: dropped connection is closed by the finaliser ----------------- +db2 = usqlite.connect("/tmp/smoke.db") +db2.execute("SELECT COUNT(*) FROM t").fetchone() +mid = usqlite.mem_current() +db2 = None +gc.collect() +after = usqlite.mem_current() +print("mem with-open-conn/after-collect:", mid, after) +assert after < mid, (mid, after) +print("finaliser ok") + +# --- row_type + named params regression sweep ----------------------------- +db3 = usqlite.connect("/tmp/smoke.db") +db3.row_type = "row" +r = db3.execute("SELECT name, v FROM t WHERE id = :i", {"i": 2}).fetchone() +assert tuple(r) == ('b', 2.5), r +assert r.keys == ('name', 'v'), r.keys +db3.row_type = "dict" +r = db3.execute("SELECT name FROM t WHERE id = ?", (1,)).fetchone() +assert r == {"name": "a"}, r +db3.close() +print("row/dict/named-params ok") + +os.remove("/tmp/smoke.db") +print("ALL SMOKE TESTS PASSED") diff --git a/usqlite_config.h b/usqlite_config.h index 9452214..9426b2d 100644 --- a/usqlite_config.h +++ b/usqlite_config.h @@ -30,6 +30,20 @@ SOFTWARE. #undef USQLITE_DEBUG +// Directory for SQLite temporary files (sort/merge spill, temp b-trees). The +// engine asks the VFS for anonymous temp files by passing a NULL name; usqlite +// gives them a unique name so large sorts / GROUP BY / index builds spill to +// disk instead of failing. By default it uses the SD card (USQLITE_TEMP_SD) +// when one is mounted -- sparing flash from the write churn of a big spill -- +// and falls back to the flash root (USQLITE_TEMP_DIR) otherwise. Either is +// overridden at runtime by PRAGMA temp_store_directory. +#ifndef USQLITE_TEMP_DIR +#define USQLITE_TEMP_DIR "/" +#endif +#ifndef USQLITE_TEMP_SD +#define USQLITE_TEMP_SD "/sd" +#endif + // ------------------------------------------------------------------------------ // SQLite configuration options - https://sqlite.org/compile.html @@ -50,7 +64,10 @@ SOFTWARE. #define SQLITE_OMIT_SHARED_CACHE 1 #define SQLITE_OMIT_TCL_VARIABLE 1 #define SQLITE_OMIT_UTF16 1 -// #define SQLITE_OMIT_WAL 1 +// WAL needs the xShm* shared-memory VFS methods, which this VFS does not +// provide (they are NULL) -- PRAGMA journal_mode=WAL would call through a +// NULL pointer. Rollback journals are the crash-safety mechanism here. +#define SQLITE_OMIT_WAL 1 #define SQLITE_UNTESTABLE 1 @@ -63,18 +80,46 @@ SOFTWARE. #undef SQLITE_ENABLE_RTREE #define SQLITE_ENABLE_MEMORY_MANAGEMENT 1 -#define SQLITE_DEFAULT_MEMSTATUS 0 +// Track heap usage so usqlite.mem_current()/mem_peak() report real figures -- +// invaluable for watching the dedicated heap fill. Cheap here: THREADSAFE is 0, +// so it is just an unlocked counter per malloc/free. +#define SQLITE_DEFAULT_MEMSTATUS 1 #define SQLITE_ZERO_MALLOC 1 // ------------------------------------------------------------------------------ +// Give SQLite one dedicated heap (MEMSYS5) instead of a GC-heap allocation per +// call. The per-call allocator (gc_alloc) puts SQLite's structures on the +// MicroPython GC heap, where the conservative collector cannot follow SQLite's +// interior/tagged pointers and frees live memory on any gc.collect() under an +// open connection -- corrupting the database or hanging the board. One pooled +// block is opaque to the GC and fixes both. See usqlite_mem.c. #ifdef SQLITE_ZERO_MALLOC -// #define SQLITE_ENABLE_MEMSYS5 1 +#define SQLITE_ENABLE_MEMSYS5 1 #endif #ifdef SQLITE_ENABLE_MEMSYS5 -#define MEMSYS5_HEAP_SIZE 128 * 1024 +// Size of that pool, reserved lazily on the first connect() (see +// usqlite_mem.c), so a program that never opens a database pays nothing. Small +// by default so it fits constrained targets -- a plain RP2040 or ESP32 has only +// a few hundred KB of RAM -- and raised per board where there is room: +// -DMEMSYS5_HEAP_SIZE=0x400000 (e.g. 4 MB on a board with PSRAM) +#ifndef MEMSYS5_HEAP_SIZE +#define MEMSYS5_HEAP_SIZE (128 * 1024) +#endif +// Page cache: small and mostly independent of the pool size (negative = KiB). +// A big cache (the old half-the-pool default) let the sorter hoard memory +// before spilling, so a large ORDER BY / GROUP BY / index build climbed to the +// edge of the pool and thrashed. Capping the cache low makes the sorter's temp +// b-tree spill to disk early, so big sorts stay bounded (sub-MB) and the pool +// keeps headroom -- measured at no speed cost (spilling to flash is cheap, and +// even on SD the same query went from a 150s+ thrash to a clean 20s). ~256 KB +// where the pool allows, never more than 1/8 of a small pool. Override to tune. +#ifndef SQLITE_DEFAULT_CACHE_SIZE +#define SQLITE_DEFAULT_CACHE_SIZE \ + (MEMSYS5_HEAP_SIZE / 8 < 256 * 1024 ? -(MEMSYS5_HEAP_SIZE / 8 / 1024) : -256) +#endif #endif // ------------------------------------------------------------------------------ diff --git a/usqlite_connection.c b/usqlite_connection.c index a05bdf4..c54121d 100644 --- a/usqlite_connection.c +++ b/usqlite_connection.c @@ -33,11 +33,16 @@ static mp_obj_t usqlite_connection_close(mp_obj_t self_in); // ------------------------------------------------------------------------------ static mp_obj_t usqlite_connection_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *args) { - usqlite_connection_t *self = m_new_obj(usqlite_connection_t); + // Allocated with a finaliser so __del__ really runs when an unclosed + // connection is collected (a plain m_new_obj has none, which made the + // __del__ below dead code and left the db handle -- page cache and all -- + // in the SQLite pool for the rest of the session). + usqlite_connection_t *self = mp_obj_malloc_with_finaliser(usqlite_connection_t, &usqlite_connection_type); - self->base.type = &usqlite_connection_type; self->db = (sqlite3 *)MP_OBJ_TO_PTR(args[0]); self->row_type = MP_QSTR_tuple; + self->row_factory = mp_const_none; + self->trace_callback = mp_const_none; mp_obj_list_init(&self->cursors, 0); return MP_OBJ_FROM_PTR(self); @@ -61,20 +66,21 @@ static mp_obj_t usqlite_connection_close(mp_obj_t self_in) { return mp_const_none; } - for (size_t i = 0; i < self->cursors.len; i++) - { - mp_obj_t cursor = self->cursors.items[i]; - self->cursors.items[0] = mp_const_none; + // Finalize the statement of every cursor still tracked. The cursor + // objects themselves belong to the GC -- Python code may well still hold + // references to them -- so they are never freed here (m_free of a live + // object was a use-after-free); the collector reclaims each one once it + // goes unreferenced. Drain from the tail, unhooking before closing. + while (self->cursors.len) { + mp_obj_t cursor = self->cursors.items[--self->cursors.len]; + ((usqlite_cursor_t *)MP_OBJ_TO_PTR(cursor))->registered = false; usqlite_cursor_close(cursor); - #if MICROPY_MALLOC_USES_ALLOCATED_SIZE - m_free(MP_OBJ_TO_PTR(cursor), sizeof(usqlite_cursor_t)); - #else - m_free(MP_OBJ_TO_PTR(cursor)); - #endif } usqlite_logprintf(___FUNC___ " closing '%s'\n", sqlite3_db_filename(self->db, NULL)); - sqlite3_close(self->db); + // close_v2: if anything is somehow still unfinalized, the handle becomes a + // zombie freed when the last statement goes, instead of leaking outright. + sqlite3_close_v2(self->db); self->db = NULL; return mp_const_none; @@ -169,13 +175,19 @@ static int traceCallback(unsigned uMask, void *context, void *p, void *x) { usqlite_connection_t *self = (usqlite_connection_t *)context; sqlite3_stmt *stmt = (sqlite3_stmt *)p; char *xsql = sqlite3_expanded_sql(stmt); - if (xsql) { - mp_call_function_1(self->trace_callback, mp_obj_new_str(xsql, strlen(xsql))); - sqlite3_free(xsql); - } else { - const char *sql = sqlite3_sql(stmt); - mp_call_function_1(self->trace_callback, mp_obj_new_str(sql, strlen(sql))); + const char *sql = xsql ? xsql : sqlite3_sql(stmt); + + // The callback runs from inside sqlite3_step; a raise in user code would + // longjmp through the VDBE's C frames (and leak xsql). Swallow it, like + // CPython's trace callbacks do. + if (sql) { + nlr_buf_t nlr; + if (nlr_push(&nlr) == 0) { + mp_call_function_1(self->trace_callback, mp_obj_new_str(sql, strlen(sql))); + nlr_pop(); + } } + sqlite3_free(xsql); return 0; } @@ -208,8 +220,18 @@ void usqlite_connection_register(usqlite_connection_t *connection, mp_obj_t curs // ------------------------------------------------------------------------------ void usqlite_connection_deregister(usqlite_connection_t *connection, mp_obj_t cursor) { - mp_obj_t cursors = MP_OBJ_FROM_PTR(&connection->cursors); - mp_obj_list_remove(cursors, cursor); + // Identity swap-remove that never raises: mp_obj_list_remove raises when + // the item is absent, and this runs on close/finaliser paths where a raise + // must not happen. + mp_obj_list_t *l = &connection->cursors; + for (size_t i = 0; i < l->len; i++) { + if (l->items[i] == cursor) { + l->items[i] = l->items[l->len - 1]; + l->items[l->len - 1] = MP_OBJ_NULL; + l->len--; + return; + } + } } // ------------------------------------------------------------------------------ diff --git a/usqlite_cursor.c b/usqlite_cursor.c index f779802..7b398b5 100644 --- a/usqlite_cursor.c +++ b/usqlite_cursor.c @@ -27,6 +27,7 @@ SOFTWARE. #include "py/objstr.h" #include "py/objtuple.h" +#include #include // ------------------------------------------------------------------------------ @@ -40,19 +41,78 @@ static mp_obj_t row_type(usqlite_cursor_t *cursor); // ------------------------------------------------------------------------------ +// A cursor is tracked in the connection's cursor list only while it holds a +// live prepared statement, so the list is exactly the set of statements that +// must be finalized at connection close -- and nothing accumulates there across +// a loop of result-less execute()s. Both calls are idempotent. +static void usqlite_cursor_track(usqlite_cursor_t *self, mp_obj_t self_in) { + if (!self->registered) { + usqlite_connection_register(self->connection, self_in); + self->registered = true; + } +} + +static void usqlite_cursor_untrack(usqlite_cursor_t *self, mp_obj_t self_in) { + if (self->registered) { + usqlite_connection_deregister(self->connection, self_in); + self->registered = false; + } +} + +// ------------------------------------------------------------------------------ + +// Build once and cache the tuple of result-column names, so .keys keeps working +// after the statement is finalized on exhaustion (see cursor_finish). +static mp_obj_t cursor_colnames(usqlite_cursor_t *self) { + if (self->colnames == MP_OBJ_NULL && self->stmt) { + int n = sqlite3_column_count(self->stmt); + mp_obj_tuple_t *o = MP_OBJ_TO_PTR(mp_obj_new_tuple(n, NULL)); + for (int i = 0; i < n; i++) + { + o->items[i] = usqlite_column_name(self->stmt, i); + } + self->colnames = MP_OBJ_FROM_PTR(o); + } + return self->colnames; +} + +// Finalize an exhausted statement and drop the cursor from the connection's +// list, so a long-lived connection running many SELECTs without closing each +// cursor does not accumulate open statements in the fixed SQLite heap. rowcount +// is left untouched and the column names are cached first, so the cursor stays +// usable for rowcount/lastrowid/.keys afterwards. +static void cursor_finish(usqlite_cursor_t *self) { + if (!self->stmt) { + return; + } + cursor_colnames(self); + sqlite3_finalize(self->stmt); + self->stmt = NULL; + usqlite_cursor_untrack(self, MP_OBJ_FROM_PTR(self)); +} + +// ------------------------------------------------------------------------------ + static mp_obj_t usqlite_cursor_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *args) { usqlite_row_type_initialize(); - usqlite_cursor_t *self = m_new_obj(usqlite_cursor_t); + // With-finaliser alloc so a dropped-without-close cursor still finalizes + // its statement via __del__ (m_new_obj has no finaliser slot, so the + // __del__ below never ran). + usqlite_cursor_t *self = mp_obj_malloc_with_finaliser(usqlite_cursor_t, &usqlite_cursor_type); mp_obj_t self_obj = MP_OBJ_FROM_PTR(self); - memset(self, 0, sizeof(usqlite_cursor_t)); - - self->base.type = &usqlite_cursor_type; self->connection = (usqlite_connection_t *)MP_OBJ_TO_PTR(args[0]); + self->stmt = NULL; + self->rc = SQLITE_OK; + self->rowcount = 0; self->arraysize = 1; + self->registered = false; + self->colnames = MP_OBJ_NULL; - usqlite_connection_register(self->connection, self_obj); + // Not registered here: registration happens when execute() acquires a live + // statement (usqlite_cursor_track), so result-less statements never linger + // in the connection's cursor list. switch (self->connection->row_type) { @@ -106,15 +166,19 @@ mp_obj_t usqlite_cursor_close(mp_obj_t self_in) { // usqlite_logprintf(___FUNC___ "\n"); usqlite_cursor_t *self = (usqlite_cursor_t *)MP_OBJ_TO_PTR(self_in); - if (!self->stmt) { - return mp_const_none; + if (self->stmt) { + usqlite_logprintf(___FUNC___ " closing: '%s'\n", sqlite3_sql(self->stmt)); + sqlite3_finalize(self->stmt); + self->stmt = NULL; + self->rowcount = -1; + self->rc = SQLITE_OK; + self->colnames = MP_OBJ_NULL; } - usqlite_logprintf(___FUNC___ " closing: '%s'\n", sqlite3_sql(self->stmt)); - sqlite3_finalize(self->stmt); - self->stmt = NULL; - self->rowcount = -1; - self->rc = SQLITE_OK; + // Always drop out of the connection's tracking list, statement or not: an + // explicitly closed cursor that stayed registered would be revisited (and + // formerly freed) by connection.close() while Python still referenced it. + usqlite_cursor_untrack(self, self_in); return mp_const_none; } @@ -124,6 +188,9 @@ MP_DEFINE_CONST_FUN_OBJ_1(usqlite_cursor_close_obj, usqlite_cursor_close); // ------------------------------------------------------------------------------ static int stepExecute(usqlite_cursor_t *self) { + if (!self->stmt) { + return self->rc; + } self->rc = sqlite3_step(self->stmt); switch (self->rc) @@ -136,6 +203,10 @@ static int stepExecute(usqlite_cursor_t *self) { break; case SQLITE_DONE: + // Exhausted: free the statement now rather than pinning it in the + // connection's cursor list until close. This is what stops a + // long-lived connection's SELECTs from piling up in the heap. + cursor_finish(self); break; case SQLITE_ERROR: @@ -157,21 +228,26 @@ static int bindParameter(sqlite3_stmt *stmt, int index, mp_obj_t value) { if (value == mp_const_none) { return sqlite3_bind_null(stmt, index); } else if (mp_obj_is_integer(value)) { - return sqlite3_bind_int(stmt, index, mp_obj_get_int(value)); + // Full 64-bit bind: bind_int + mp_obj_get_int raised OverflowError + // for anything past 2^31 (epoch milliseconds, large ids). + return sqlite3_bind_int64(stmt, index, mp_obj_get_ll(value)); } else if (mp_obj_is_str(value)) { GET_STR_DATA_LEN(value, str, nstr); - return sqlite3_bind_text(stmt, index, (const char *)str, nstr, NULL); - } else if (mp_obj_is_type(value, &mp_type_float)) { + // SQLITE_TRANSIENT: SQLite copies the bytes now. SQLITE_STATIC (a NULL + // destructor) would keep this raw pointer into Python's string data, + // which the GC may free or the statement may outlive -- use-after-free. + return sqlite3_bind_text(stmt, index, (const char *)str, nstr, SQLITE_TRANSIENT); + } else if (mp_obj_is_float(value)) { return sqlite3_bind_double(stmt, index, mp_obj_get_float(value)); } else if (mp_obj_is_type(value, &mp_type_bytes)) { GET_STR_DATA_LEN(value, bytes, nbytes); - return sqlite3_bind_blob(stmt, index, bytes, nbytes, NULL); + return sqlite3_bind_blob(stmt, index, bytes, nbytes, SQLITE_TRANSIENT); } #if MICROPY_PY_BUILTINS_BYTEARRAY if (mp_obj_is_type(value, &mp_type_bytearray)) { mp_buffer_info_t buffer; if (mp_get_buffer(value, &buffer, MP_BUFFER_READ)) { - return sqlite3_bind_blob(stmt, index, buffer.buf, buffer.len, NULL); + return sqlite3_bind_blob(stmt, index, buffer.buf, buffer.len, SQLITE_TRANSIENT); } } #endif @@ -299,6 +375,13 @@ static mp_obj_t usqlite_cursor_execute(size_t n_args, const mp_obj_t *args) { return mp_const_none; } + self->colnames = MP_OBJ_NULL; // fresh statement -> rebuild names on demand + + // Track it now that a live statement exists, so it is finalized at + // connection close even if binding/stepping below raises or the caller + // drops the cursor. + usqlite_cursor_track(self, self_in); + int nParams = sqlite3_bind_parameter_count(self->stmt); if (nParams > 0) { if (n_args >= 3) { @@ -337,6 +420,13 @@ static mp_obj_t usqlite_cursor_execute(size_t n_args, const mp_obj_t *args) { break; } + // No explicit finalize needed here: a statement that returns no rows + // (INSERT/UPDATE/DELETE/DDL) or an empty SELECT has already stepped to + // SQLITE_DONE above, and stepExecute() -> cursor_finish() has finalized it + // and dropped the cursor from the connection's list. rowcount (captured in + // the switch above) and lastrowid (read from the connection) both survive. + // A SELECT that produced rows keeps its statement, to be finalized when the + // caller exhausts it, closes it, or closes the connection. return self_in; } @@ -359,11 +449,15 @@ static mp_obj_t usqlite_cursor_executemany(mp_obj_t self_in, mp_obj_t sql_in) { int rc = sqlite3_exec(self->connection->db, sql, NULL, NULL, &errmsg); if (rc) { - mp_raise_msg_varg(&usqlite_Error, MP_ERROR_TEXT("%s"), errmsg ? errmsg : ""); + // Copy the message and free it *before* raising: the raise unwinds + // immediately, and a free placed after it leaked the message in the + // SQLite pool for the rest of the session. + char msg[128]; + snprintf(msg, sizeof(msg), "%s", errmsg ? errmsg : sqlite3_errstr(rc)); + sqlite3_free(errmsg); + mp_raise_msg_varg(&usqlite_Error, MP_ERROR_TEXT("%s"), msg); } - sqlite3_free(errmsg); - return self_in; } @@ -372,15 +466,11 @@ static MP_DEFINE_CONST_FUN_OBJ_2(usqlite_cursor_executemany_obj, usqlite_cursor_ // ------------------------------------------------------------------------------ static mp_obj_t usqlite_cursor_getiter(mp_obj_t self_in, mp_obj_iter_buf_t *iter_buf) { - usqlite_cursor_t *self = MP_OBJ_TO_PTR(self_in); (void)iter_buf; - if (!self->stmt) { - mp_raise_msg(&usqlite_Error, MP_ERROR_TEXT("No iter data")); - return mp_const_none; - } - - return self; + // A finalized or result-less cursor iterates as empty (iternext stops as + // soon as rc != SQLITE_ROW), so there is no "no iter data" error here. + return self_in; } // ------------------------------------------------------------------------------ @@ -420,6 +510,13 @@ static mp_obj_t row_type(usqlite_cursor_t *cursor) { mp_obj_tuple_t *o = MP_OBJ_TO_PTR(mp_obj_new_tuple(columns + 1, NULL)); + // A Row is the column values plus one extra, hidden slot holding the cursor + // (usqlite_row_attr reads it at items[len] to build .keys()). Stamp the Row + // type so .keys resolves, and set len to the column count so the cursor slot + // stays hidden from indexing, iteration, len and printing. (The block is + // still columns+1 wide, so the GC keeps the cursor referenced.) + o->base.type = (const mp_obj_type_t *)&usqlite_row_type; + o->len = columns; o->items[columns] = MP_OBJ_FROM_PTR(cursor); for (int i = 0; i < columns; i++) @@ -471,7 +568,7 @@ static mp_obj_t usqlite_cursor_fetchone(mp_obj_t self_in) { : mp_const_none; if (self->rc == SQLITE_ROW) { - stepExecute(self_in); + stepExecute(self); } return result; @@ -496,7 +593,7 @@ static mp_obj_t usqlite_cursor_fetchmany(size_t n_args, const mp_obj_t *args) { ? mp_obj_get_int(args[1]) : self->arraysize; - stepExecute(args[0]); + stepExecute(self); if (!size) { size = 1; @@ -509,7 +606,7 @@ static mp_obj_t usqlite_cursor_fetchmany(size_t n_args, const mp_obj_t *args) { while (self->rc == SQLITE_ROW && (size < 0 || (int)listt->len < size)) { row = self->rowfactory(self); mp_obj_list_append(list, row); - stepExecute(args[0]); + stepExecute(self); } return list; @@ -534,7 +631,10 @@ static MP_DEFINE_CONST_FUN_OBJ_1(usqlite_cursor_fetchall_obj, usqlite_cursor_fet // ------------------------------------------------------------------------------ static mp_obj_t usqlite_cursor_description(sqlite3_stmt *stmt) { - int columns = sqlite3_data_count(stmt); + // column_count (stable from prepare), not data_count (0 unless a row is + // currently loaded) -- .description is valid before the first fetch and + // after the last, like CPython's. + int columns = sqlite3_column_count(stmt); mp_obj_tuple_t *o = MP_OBJ_TO_PTR(mp_obj_new_tuple(columns, NULL)); @@ -569,30 +669,23 @@ static void usqlite_cursor_attr(mp_obj_t self_in, qstr attr, mp_obj_t *dest) { return; } - const char* strConnection = "connection"; - mp_obj_t objConnection = mp_obj_new_str(strConnection, strlen(strConnection)); - qstr qstrConnection = mp_obj_str_get_qstr(objConnection); - - if (attr == qstrConnection) { - dest[0] = MP_OBJ_FROM_PTR(self->connection); - } - else { - - switch (attr) - { + switch (attr) + { case MP_QSTR_connection: dest[0] = MP_OBJ_FROM_PTR(self->connection); break; case MP_QSTR_description: - dest[0] = usqlite_cursor_description(self->stmt); + dest[0] = self->stmt + ? usqlite_cursor_description(self->stmt) + : mp_const_none; break; case MP_QSTR_lastrowid: { sqlite3_int64 rowid = sqlite3_last_insert_rowid(self->connection->db); dest[0] = rowid ? mp_obj_new_int_from_ll(rowid) : mp_const_none; + break; } - break; case MP_QSTR_rowcount: dest[0] = mp_obj_new_int(self->rowcount); @@ -601,7 +694,6 @@ static void usqlite_cursor_attr(mp_obj_t self_in, qstr attr, mp_obj_t *dest) { case MP_QSTR_arraysize: dest[0] = mp_obj_new_int(self->arraysize); break; - } } } else if (dest[1] != MP_OBJ_NULL) { switch (attr) @@ -617,12 +709,9 @@ static void usqlite_cursor_attr(mp_obj_t self_in, qstr attr, mp_obj_t *dest) { // ------------------------------------------------------------------------------ static mp_obj_t usqlite_cursor_del(mp_obj_t self_in) { - usqlite_cursor_t *self = MP_OBJ_TO_PTR(self_in); - usqlite_logprintf(___FUNC___ "\n"); - usqlite_cursor_close(self_in); - usqlite_connection_deregister(self->connection, self_in); + usqlite_cursor_close(self_in); // finalizes the statement and untracks return mp_const_none; } diff --git a/usqlite_cursor.h b/usqlite_cursor.h index 3880e98..f9e25e6 100644 --- a/usqlite_cursor.h +++ b/usqlite_cursor.h @@ -46,6 +46,8 @@ struct _usqlite_cursor_t int rowcount; usqlite_rowfactory_t rowfactory; int arraysize; + bool registered; // true while listed in connection->cursors (holds a stmt) + mp_obj_t colnames; // cached result-column names, so .keys survives finalize }; // ------------------------------------------------------------------------------ diff --git a/usqlite_file.c b/usqlite_file.c index 9912a1e..cc3fb10 100644 --- a/usqlite_file.c +++ b/usqlite_file.c @@ -24,59 +24,84 @@ SOFTWARE. #include "usqlite.h" +#include + #include "py/objstr.h" #include "py/objmodule.h" +#include "py/objlist.h" #include "py/runtime.h" +#include "py/gc.h" #include "py/stream.h" #include "py/builtin.h" +#include "py/mphal.h" extern const mp_obj_module_t mp_module_io; // ------------------------------------------------------------------------------ -bool usqlite_file_exists(const char *pathname) { - mp_obj_t os = mp_module_get_builtin(MP_QSTR_uos, 0); - mp_obj_t ilistdir = usqlite_method(os, MP_QSTR_ilistdir); - - char path[MAXPATHNAME + 1]; - strcpy(path, pathname); - const char *filename = pathname; +// Every open database file is a MicroPython stream object (io.open()), but the +// only pointer to it lives inside SQLite's sqlite3_file, which SQLite allocates +// from its own dedicated heap. The GC does not walk SQLite's heap as object +// memory, so without a strong reference here the stream can be collected out +// from under an open connection -- a use-after-free that surfaces only after a +// gc.collect(), and only on some memory layouts. Pin every open stream in a +// GC-visible list held from a root pointer; unpin on close. +MP_REGISTER_ROOT_POINTER(mp_obj_t usqlite_files); + +static void usqlite_file_pin(mp_obj_t stream) { + if (!MP_STATE_VM(usqlite_files)) { + MP_STATE_VM(usqlite_files) = mp_obj_new_list(0, NULL); + } + mp_obj_list_append(MP_STATE_VM(usqlite_files), stream); +} - char *lastSep = strrchr(path, '/'); - if (lastSep) { - *lastSep++ = 0; - filename = lastSep; - } else { - lastSep = strrchr(path, '\\'); - if (lastSep) { - *lastSep++ = 0; - filename = lastSep; - } else { - path[0] = '.'; - path[1] = 0; +static void usqlite_file_unpin(mp_obj_t stream) { + mp_obj_t lst = MP_STATE_VM(usqlite_files); + if (!lst) { + return; + } + // Swap-remove by identity; never raises (mp_obj_list_remove would, and this + // runs on the close/finalize path where a raise must not happen). + mp_obj_list_t *l = MP_OBJ_TO_PTR(lst); + for (size_t i = 0; i < l->len; i++) { + if (l->items[i] == stream) { + l->items[i] = l->items[l->len - 1]; + l->items[l->len - 1] = MP_OBJ_NULL; + l->len--; + return; } } +} - bool exists = false; - mp_obj_t listdir = mp_call_function_1(ilistdir, mp_obj_new_str(path, strlen(path))); - mp_obj_t entry = mp_iternext(listdir); +// ------------------------------------------------------------------------------ - while (entry != MP_OBJ_STOP_ITERATION) { - mp_obj_tuple_t *t = MP_OBJ_TO_PTR(entry); +// os.stat probe: one name lookup instead of the old ilistdir() directory walk +// (which was slower, raised through SQLite's frames when the directory was +// missing, and mis-resolved root-level names to the cwd). Never raises. Now +// also on the hot path: hot-journal detection probes existence via xAccess. +bool usqlite_file_exists(const char *pathname) { + return usqlite_file_accessible(pathname); +} - int type = mp_obj_get_int(t->items[1]); - if (type == 0x8000) { - const char *name = mp_obj_str_get_str(t->items[0]); - if ((exists = strcmp(filename, name) == 0)) { - break; - } - } +// ------------------------------------------------------------------------------ - entry = mp_iternext(listdir); +// True if os.stat(pathname) succeeds -- used by the VFS xAccess to answer +// SQLite's "is this a writable directory?" probe (temp_store_directory). Guarded +// by nlr because os.stat raises rather than returning a code when absent. +bool usqlite_file_accessible(const char *pathname) { + if (!pathname) { + return false; } - - - return exists; + nlr_buf_t nlr; + bool ok = false; + if (nlr_push(&nlr) == 0) { + mp_obj_t os = mp_module_get_builtin(MP_QSTR_uos, 0); + mp_obj_t stat = usqlite_method(os, MP_QSTR_stat); + mp_call_function_1(stat, mp_obj_new_str(pathname, strlen(pathname))); + nlr_pop(); + ok = true; + } + return ok; } // ------------------------------------------------------------------------------ @@ -84,14 +109,39 @@ bool usqlite_file_exists(const char *pathname) { int usqlite_file_open(MPFILE *file, const char *pathname, int flags) { LOGFUNC; - mp_obj_t filename = mp_obj_new_str(pathname, strlen(pathname)); + // SQLite requests anonymous temporary files (sorter/merge spill, temp + // b-trees) by passing a NULL name and expecting the VFS to invent one -- + // dereferencing that NULL previously crashed the board. Give temp files a + // unique name in the temp directory so large sorts and index builds spill + // to disk instead of faulting. PRAGMA temp_store_directory overrides the + // default (USQLITE_TEMP_DIR). + char tmpname[128]; + bool is_temp = (pathname == NULL) || + ((flags & (SQLITE_OPEN_TEMP_DB | SQLITE_OPEN_TEMP_JOURNAL | + SQLITE_OPEN_TRANSIENT_DB | SQLITE_OPEN_SUBJOURNAL)) != 0); + if (pathname == NULL) { + static uint32_t seq; + // Prefer the SD card when it is mounted (spares flash the spill churn), + // fall back to flash; PRAGMA temp_store_directory overrides both. + const char *dir = + sqlite3_temp_directory ? sqlite3_temp_directory : + usqlite_file_accessible(USQLITE_TEMP_SD) ? USQLITE_TEMP_SD : + USQLITE_TEMP_DIR; + size_t dlen = strlen(dir); + const char *sep = (dlen && dir[dlen - 1] == '/') ? "" : "/"; + snprintf(tmpname, sizeof(tmpname), "%s%setilqs_%08x%08x", + dir, sep, (unsigned)mp_hal_ticks_ms(), (unsigned)seq++); + pathname = tmpname; + } char mode[8]; memset(mode, 0, sizeof(mode)); char *pMode = mode; if (flags & SQLITE_OPEN_CREATE) { - if (!usqlite_file_exists(pathname)) { + // A temp file has a fresh unique name (and root-level paths confuse the + // existence probe), so skip the check and always create-new. + if (is_temp || !usqlite_file_exists(pathname)) { *pMode++ = 'w'; } @@ -107,17 +157,27 @@ int usqlite_file_open(MPFILE *file, const char *pathname, int flags) { *pMode++ = 'b'; - mp_obj_t filemode = mp_obj_new_str(mode, strlen(mode)); - usqlite_logprintf(___FUNC___ " '%s' mode:%s\n", pathname, mode); - mp_obj_t open = usqlite_method(&mp_module_io, MP_QSTR_open); - file->stream = mp_call_function_2(open, filename, filemode); + // io.open raises on failure (missing directory, absent or full card, ...). + // A raise here would unwind SQLite mid-operation, leaking engine state -- + // convert it to the error code the VFS contract expects instead. + nlr_buf_t nlr; + if (nlr_push(&nlr) == 0) { + mp_obj_t filename = mp_obj_new_str(pathname, strlen(pathname)); + mp_obj_t filemode = mp_obj_new_str(mode, strlen(mode)); + mp_obj_t open = usqlite_method(&mp_module_io, MP_QSTR_open); + file->stream = mp_call_function_2(open, filename, filemode); + usqlite_file_pin(file->stream); + nlr_pop(); + } else { + file->stream = NULL; + return SQLITE_CANTOPEN; + } + strcpy(file->pathname, pathname); file->flags = flags; - // const mp_stream_p_t* stream = mp_get_stream(file->stream); - return SQLITE_OK; } @@ -139,7 +199,14 @@ int usqlite_file_close(MPFILE *file) { if (file->stream) { usqlite_logprintf(___FUNC___ " %s\n", file->pathname); - mp_stream_close(file->stream); + // A stream-close error must not unwind through SQLite's teardown; + // whatever happens, finish the job -- unpin and forget the stream. + nlr_buf_t nlr; + if (nlr_push(&nlr) == 0) { + mp_stream_close(file->stream); + nlr_pop(); + } + usqlite_file_unpin(file->stream); file->stream = NULL; if (file->flags & SQLITE_OPEN_DELETEONCLOSE) { @@ -152,13 +219,46 @@ int usqlite_file_close(MPFILE *file) { // ------------------------------------------------------------------------------ +// Truncate the file to zero length. MicroPython streams have no truncate +// ioctl, so emulate by reopening the file with a truncating mode. SQLite +// calls this on every commit (locking_mode=EXCLUSIVE finalizes the rollback +// journal with a truncate instead of a delete); if the old journal content +// survived a "successful" truncate, the next open after a power cut would +// replay a stale journal over committed data. +int usqlite_file_truncate0(MPFILE *file) { + LOGFUNC; + + if (!file->stream) { + return SQLITE_IOERR_TRUNCATE; + } + + nlr_buf_t nlr; + if (nlr_push(&nlr) == 0) { + mp_stream_close(file->stream); + usqlite_file_unpin(file->stream); + file->stream = NULL; + + mp_obj_t filename = mp_obj_new_str(file->pathname, strlen(file->pathname)); + mp_obj_t filemode = mp_obj_new_str("w+b", 3); + mp_obj_t open = usqlite_method(&mp_module_io, MP_QSTR_open); + file->stream = mp_call_function_2(open, filename, filemode); + usqlite_file_pin(file->stream); + nlr_pop(); + return SQLITE_OK; + } else { + return SQLITE_IOERR_TRUNCATE; + } +} + +// ------------------------------------------------------------------------------ + int usqlite_file_read(MPFILE *file, void *pBuf, size_t nBuf) { LOGFUNC; int error = 0; mp_uint_t size = mp_stream_rw(file->stream, pBuf, nBuf, &error, MP_STREAM_RW_READ); if (size != nBuf) { - usqlite_errprintf("write error: %d", error); + usqlite_errprintf("read error: %d", error); } return size; @@ -234,11 +334,27 @@ int usqlite_file_delete(const char *pathname) { usqlite_logprintf("%s: %s\n", __func__, pathname); - mp_obj_t filename = mp_obj_new_str(pathname, strlen(pathname)); - mp_obj_t remove = usqlite_method(mp_module_get_builtin(MP_QSTR_uos, 0), MP_QSTR_remove); - mp_call_function_1(remove, filename); + // This path allocates Python objects to call os.remove(). During a GC + // sweep (finaliser-driven close, e.g. at soft reset) the heap is locked + // and that would raise MemoryError out through SQLite's C frames, + // aborting sqlite3_close halfway. Report failure instead; a leftover + // journal file is harmless here (hot-journal recovery is not enabled). + if (gc_is_locked()) { + return SQLITE_IOERR_DELETE; + } - return SQLITE_OK; + // os.remove raises when the file is already gone or the card was pulled; + // report failure as a code rather than unwinding SQLite's C frames. + nlr_buf_t nlr; + if (nlr_push(&nlr) == 0) { + mp_obj_t filename = mp_obj_new_str(pathname, strlen(pathname)); + mp_obj_t remove = usqlite_method(mp_module_get_builtin(MP_QSTR_uos, 0), MP_QSTR_remove); + mp_call_function_1(remove, filename); + nlr_pop(); + return SQLITE_OK; + } else { + return SQLITE_IOERR_DELETE; + } } // ------------------------------------------------------------------------------ diff --git a/usqlite_file.h b/usqlite_file.h index 0389d5b..c2844eb 100644 --- a/usqlite_file.h +++ b/usqlite_file.h @@ -45,8 +45,10 @@ MPFILE; // ------------------------------------------------------------------------------ bool usqlite_file_exists(const char *filepath); +bool usqlite_file_accessible(const char *pathname); int usqlite_file_open(MPFILE *file, const char *name, int flags); int usqlite_file_close(MPFILE *file); +int usqlite_file_truncate0(MPFILE *file); int usqlite_file_read(MPFILE *file, void *pBuf, size_t nBuf); int usqlite_file_write(MPFILE *file, const void *pBuf, size_t nBuf); int usqlite_file_flush(MPFILE *file); diff --git a/usqlite_mem.c b/usqlite_mem.c index 85dee94..481eee6 100644 --- a/usqlite_mem.c +++ b/usqlite_mem.c @@ -32,26 +32,40 @@ SOFTWARE. #if defined(SQLITE_ZERO_MALLOC) && defined(SQLITE_ENABLE_MEMSYS5) -static mp_obj_t sqlite_heap; +// The one dedicated heap for the whole SQLite engine, handed to SQLite via +// SQLITE_CONFIG_HEAP so every SQLite allocation lives inside it. To the +// MicroPython GC this is a single block: it never sees, and so never frees, +// SQLite's internal allocations -- which is what makes gc.collect() (explicit +// or automatic) safe while a connection is open. Held in a GC root so the +// collector keeps the block for the session; on soft reset the module __init__ +// clears the root (root pointers are NOT auto-zeroed) and the swept heap hands +// the RAM back. +MP_REGISTER_ROOT_POINTER(void *usqlite_heap); // ------------------------------------------------------------------------------ void usqlite_mem_init(void) { LOGFUNC; - // usqlite_logprintf("usqlite_init\n"); - usqlite_logprintf("zero malloc heap: %d\n", MEMSYS5_HEAP_SIZE); + // Reserve lazily and once. usqlite_mem_init() runs from the module's + // initialize(), which fires on the first connect() -- so a program that + // never opens a database reserves nothing. + if (MP_STATE_VM(usqlite_heap)) { + return; + } - void *heap = m_malloc(MEMSYS5_HEAP_SIZE); + void *heap = m_malloc_maybe(MEMSYS5_HEAP_SIZE); if (!heap) { - mp_raise_msg_varg(&usqlite_Error, MP_ERROR_TEXT("Failed to alloc heap: %d"), HEAP_SIZE); - return; + mp_raise_msg_varg(&usqlite_Error, + MP_ERROR_TEXT("cannot reserve %d bytes for the SQLite engine"), + MEMSYS5_HEAP_SIZE); } - LOGLINE; - sqlite_heap = MP_OBJ_FROM_PTR(heap); - sqlite3_config(SQLITE_CONFIG_HEAP, heap, HEAP_SIZE, 0); - LOGLINE; + MP_STATE_VM(usqlite_heap) = heap; + // Third arg is MEMSYS5's minimum allocation (atom) size: it must be a sane + // power of two, not 0 -- 0 becomes a 1-byte atom and cripples the buddy + // allocator with control overhead. 64 bytes is in SQLite's recommended range. + sqlite3_config(SQLITE_CONFIG_HEAP, heap, MEMSYS5_HEAP_SIZE, 64); } #endif diff --git a/usqlite_module.c b/usqlite_module.c index 99bb68f..8eaf7dd 100644 --- a/usqlite_module.c +++ b/usqlite_module.c @@ -65,13 +65,27 @@ static const mp_rom_obj_tuple_t sqlite_version_info = { // ------------------------------------------------------------------------------ -static void initialize() { - static int initialized = 0; +// Session marker for the one-time engine setup. A C static would survive a +// soft reset outright; a root pointer keeps the marker GC-visible, but note +// that root pointers are NOT auto-zeroed on soft reset either -- the module's +// __init__ (called by the runtime on the first import of each session) clears +// it, which is what makes each new session re-run the full +// shutdown / configure / initialize cycle below. +MP_REGISTER_ROOT_POINTER(void *usqlite_initialized); - if (initialized) { +static void initialize() { + if (MP_STATE_VM(usqlite_initialized)) { return; } + // A previous session may have left the engine initialized with its pool + // (and anything PRAGMA temp_store_directory allocated there) inside a heap + // that no longer exists; running on would corrupt the new Python heap. + // Clear the dangling pointer and shut the engine down cleanly so it can be + // reconfigured from scratch. Both are no-ops on a cold start. + sqlite3_temp_directory = NULL; + sqlite3_shutdown(); + usqlite_mem_init(); int rc = sqlite3_initialize(); @@ -80,7 +94,7 @@ static void initialize() { return; } - initialized = 1; + MP_STATE_VM(usqlite_initialized) = (void *)&usqlite_Error; // any non-NULL tag } // ------------------------------------------------------------------------------ @@ -88,7 +102,20 @@ static void initialize() { static mp_obj_t usqlite_init(void) { LOGFUNC; - // initialize(); + // The runtime calls a builtin module's __init__ on the FIRST import of + // each session (the loaded-modules dict is per-session state). Crucially, + // root pointers are NOT auto-zeroed across a soft reset on bare-metal + // ports -- mp_init() never memsets the VM state -- so this hook is the + // one reliable place to forget the previous session's engine state. The + // old pool died with the old heap contents; clearing these markers makes + // the next connect() re-run the full shutdown/configure/initialize cycle. + // (Only the runtime should call this; invoking usqlite.__init__() by hand + // mid-session would orphan live connections.) + MP_STATE_VM(usqlite_initialized) = NULL; + #if defined(SQLITE_ZERO_MALLOC) && defined(SQLITE_ENABLE_MEMSYS5) + MP_STATE_VM(usqlite_heap) = NULL; + #endif + MP_STATE_VM(usqlite_files) = MP_OBJ_NULL; return mp_const_none; } diff --git a/usqlite_row.c b/usqlite_row.c index 687e341..a0c9da0 100644 --- a/usqlite_row.c +++ b/usqlite_row.c @@ -61,7 +61,19 @@ void usqlite_row_type_initialize() { // ------------------------------------------------------------------------------ static mp_obj_t keys(usqlite_cursor_t *cursor) { - int columns = sqlite3_data_count(cursor->stmt); + // Prefer the names cached when the statement was finalized on exhaustion, so + // .keys still works on rows from an already-consumed cursor. + if (cursor->colnames != MP_OBJ_NULL) { + return cursor->colnames; + } + if (!cursor->stmt) { + return mp_obj_new_tuple(0, NULL); + } + + // sqlite3_column_count (result column count, stable after prepare), not + // sqlite3_data_count (current-row count, 0 once the fetch has stepped past + // the row) -- .keys is typically read after the row has been fetched. + int columns = sqlite3_column_count(cursor->stmt); mp_obj_tuple_t *o = MP_OBJ_TO_PTR(mp_obj_new_tuple(columns, NULL)); @@ -86,7 +98,9 @@ static void usqlite_row_attr(mp_obj_t self_in, qstr attr, mp_obj_t *dest) { switch (attr) { case MP_QSTR_keys: - dest[0] = keys(self->items[self->len]); + // The hidden slot holds the cursor object; convert properly + // rather than relying on object representation A. + dest[0] = keys((usqlite_cursor_t *)MP_OBJ_TO_PTR(self->items[self->len])); break; } } diff --git a/usqlite_utils.c b/usqlite_utils.c index f6e89fb..6c9fa09 100644 --- a/usqlite_utils.c +++ b/usqlite_utils.c @@ -157,7 +157,9 @@ mp_obj_t usqlite_column_value(sqlite3_stmt *stmt, int column) { return mp_const_none; case SQLITE_INTEGER: - return mp_obj_new_int(sqlite3_column_int(stmt, column)); + // Full 64-bit read: sqlite3_column_int truncates to 32 bits, which + // silently wrapped stored timestamps / large ids. + return mp_obj_new_int_from_ll(sqlite3_column_int64(stmt, column)); case SQLITE_FLOAT: return mp_obj_new_float((mp_float_t)sqlite3_column_double(stmt, column)); @@ -203,7 +205,12 @@ mp_obj_t usqlite_column_type(sqlite3_stmt *stmt, int column) { // ------------------------------------------------------------------------------ #ifndef SQLITE_OMIT_DECLTYPE mp_obj_t usqlite_column_decltype(sqlite3_stmt *stmt, int column) { + // NULL for any computed column (COUNT(*), expressions, ...) -- strlen(NULL) + // was a hard fault the first time .description met an aggregate. const char *type = sqlite3_column_decltype(stmt, column); + if (!type) { + return mp_const_none; + } return mp_obj_new_str(type, strlen(type)); } diff --git a/usqlite_vfs.c b/usqlite_vfs.c index 2041751..00c673f 100644 --- a/usqlite_vfs.c +++ b/usqlite_vfs.c @@ -26,11 +26,13 @@ SOFTWARE. #include #include +#include #include "py/objstr.h" #include "py/runtime.h" #include "py/stream.h" #include "py/builtin.h" +#include "py/mphal.h" // ------------------------------------------------------------------------------ @@ -76,10 +78,19 @@ static int mpvfsRead(sqlite3_file *pFile, void *pBuf, int nBuf, sqlite3_int64 of } int size = usqlite_file_read(file, pBuf, nBuf); + if (size == nBuf) { + return SQLITE_OK; + } - return size == nBuf - ? SQLITE_OK - : SQLITE_IOERR_SHORT_READ; + // VFS contract: on a short read the unread tail MUST be zero-filled. + // SQLite relies on this when sizing up a journal or database whose tail + // was lost -- exactly the situation crash recovery reads into. + if (size < 0) { + size = 0; + } + memset((char *)pBuf + size, 0, nBuf - size); + + return SQLITE_IOERR_SHORT_READ; } // ------------------------------------------------------------------------------ @@ -104,11 +115,18 @@ static int mpvfsWrite(sqlite3_file *pFile, const void *pBuf, int nBuf, sqlite3_i static int mpvfsTruncate(sqlite3_file *pFile, sqlite3_int64 size) { LOGFUNC; - #ifdef USQLITE_DEBUG MPFILE *file = (MPFILE *)pFile; - usqlite_logprintf(___FUNC___ "%s\n", file->pathname); - #endif + // Truncate-to-zero is how SQLite finalizes the rollback journal on every + // commit under locking_mode=EXCLUSIVE (the default here); it MUST really + // happen, or a later power cut would replay the stale journal over + // committed data. MicroPython streams cannot shrink a file to a nonzero + // length, so that case (only VACUUM's final shrink of the main database) + // stays a no-op: the file keeps stale pages past the page count in the + // header, which SQLite ignores -- the file just does not get smaller. + if (size == 0) { + return usqlite_file_truncate0(file); + } return SQLITE_OK; } @@ -252,8 +270,11 @@ static int mpvfsAccess(sqlite3_vfs *vfs, const char *zName, int flags, int *pRes // if (flags == SQLITE_ACCESS_READWRITE) eAccess = R_OK | W_OK; // if (flags == SQLITE_ACCESS_READ) eAccess = R_OK; -// rc = access(zPath, eAccess); - *pResOut = 0;// (rc == 0); + // Honest answers, including SQLITE_ACCESS_EXISTS: this is what lets the + // pager see a hot journal left by a power cut and roll the transaction + // back instead of serving a torn database. (Historically this VFS answered + // "no" to every existence probe, which silently disabled crash recovery.) + *pResOut = usqlite_file_accessible(zName) ? 1 : 0; return SQLITE_OK; } @@ -262,7 +283,12 @@ static int mpvfsAccess(sqlite3_vfs *vfs, const char *zName, int flags, int *pRes static int mpvfsFullPathname(sqlite3_vfs *vfs, const char *zName, int nOut, char *zOut) { LOGFUNC; - strcpy(zOut, zName); + if (!zName) { + zOut[0] = 0; + return SQLITE_OK; + } + // Names are already absolute here; just copy, bounded by the caller's buffer. + snprintf(zOut, nOut, "%s", zName); return SQLITE_OK; } @@ -281,7 +307,21 @@ static int mpvfsFullPathname(sqlite3_vfs *vfs, const char *zName, int nOut, char static int mpvfsRandomness(sqlite3_vfs *pVfs, int nByte, char *zByte) { LOGFUNC; - return SQLITE_OK; + // Seed material for SQLite's internal PRNG. It must actually be filled + // (this used to return leaving the buffer as stack garbage); xorshift over + // the microsecond clock is plenty for what SQLite uses it for. + static uint32_t s; + if (s == 0) { + s = (uint32_t)mp_hal_ticks_us() | 1; + } + for (int i = 0; i < nByte; i++) { + s ^= s << 13; + s ^= s >> 17; + s ^= s << 5; + zByte[i] = (char)s; + } + + return nByte; } // ------------------------------------------------------------------------------