Merge pull request cloudflare#693 from cloudflare/kenton/fix-sql-curs…

…or-lifetime

SQLite: Bugfix: Cursor from prepared statement should keep statement alive.

src/workerd/api/sql-test.js

@@ -437,6 +437,22 @@ async function test(storage) {
     });
     assert.equal(getI(), 2);
   }
+
+  await scheduler.wait(1);
+
+  // Test for bug where a cursor constructed from a prepared statement didn't have a strong ref
+  // to the statement object.
+  {
+    sql.exec("CREATE TABLE iteratorTest (i INTEGER)");
+    sql.exec("INSERT INTO iteratorTest VALUES (0), (1)");
+
+    let q = sql.prepare("SELECT * FROM iteratorTest")();
+    let i = 0;
+    for (let row of q) {
+      assert.equal(row.i, i++);
+      gc();
+    }
+  }
 }
 
 export class DurableObjectExample {

src/workerd/api/sql-test.wd-test

@@ -4,7 +4,9 @@ const config :Workerd.Config = (
   services = [
     (name = "main", worker = .mainWorker),
     (name = "TEST_TMPDIR", disk = (writable = true)),
-  ]
+  ],
+
+  v8Flags = [ "--expose-gc" ],
 );
 
 const mainWorker :Workerd.Worker = (

src/workerd/api/sql.c++

@@ -227,10 +227,12 @@ jsg::Ref<SqlStorage::Cursor> SqlStorage::Statement::run(jsg::Arguments<BindingVa
       c->state = nullptr;
     }
     c->selfRef = nullptr;
+    c->statement = nullptr;
     currentCursor = nullptr;
   }
 
   auto result = jsg::alloc<Cursor>(cachedColumnNames, statementRef, kj::mv(bindings));
+  result->statement = JSG_THIS;
 
   result->selfRef = currentCursor;
   currentCursor = *result;

src/workerd/api/sql.h

@@ -158,9 +158,16 @@ class SqlStorage::Cursor final: public jsg::Object {
   // destruction. Used by Statement to invalidate past cursors when the statement is
   // executed again.
 
+  kj::Maybe<jsg::Ref<Statement>> statement;
+  // If this cursor was created from a prepared statement, this keeps the statement object alive.
+
   kj::Maybe<CachedColumnNames> ownCachedColumnNames;
   CachedColumnNames& cachedColumnNames;
 
+  void visitForGc(jsg::GcVisitor& visitor) {
+    visitor.visit(statement);
+  }
+
   static kj::Array<const SqliteDatabase::Query::ValuePtr> mapBindings(
       kj::ArrayPtr<BindingValue> values);