sqlite: reject reentry into a running statement

src/node_sqlite.cc

@@ -2661,12 +2661,17 @@ void StatementSync::Close(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   THROW_AND_RETURN_ON_BAD_STATE(
       env, stmt->IsFinalized(), "statement has been finalized");
+  THROW_AND_RETURN_ON_BAD_STATE(
+      env, stmt->IsStepping(), "statement is currently being executed");
   stmt->Close();
 }
 
 void StatementSync::Dispose(const FunctionCallbackInfo<Value>& args) {
   StatementSync* stmt;
   ASSIGN_OR_RETURN_UNWRAP(&stmt, args.This());
+  Environment* env = Environment::GetCurrent(args);
+  THROW_AND_RETURN_ON_BAD_STATE(
+      env, stmt->IsStepping(), "statement is currently being executed");
   stmt->Close();
 }
 
@@ -3111,7 +3116,10 @@ void StatementSync::All(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   THROW_AND_RETURN_ON_BAD_STATE(
       env, stmt->IsFinalized(), "statement has been finalized");
+  THROW_AND_RETURN_ON_BAD_STATE(
+      env, stmt->IsStepping(), "statement is currently being executed");
   Isolate* isolate = env->isolate();
+  auto stepping = stmt->MarkStepping();
   int r = stmt->ResetStatement();
   CHECK_ERROR_OR_THROW(isolate, stmt->db_.get(), r, SQLITE_OK, void());
 
@@ -3138,6 +3146,8 @@ void StatementSync::Iterate(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   THROW_AND_RETURN_ON_BAD_STATE(
       env, stmt->IsFinalized(), "statement has been finalized");
+  THROW_AND_RETURN_ON_BAD_STATE(
+      env, stmt->IsStepping(), "statement is currently being executed");
   int r = stmt->ResetStatement();
   CHECK_ERROR_OR_THROW(env->isolate(), stmt->db_.get(), r, SQLITE_OK, void());
 
@@ -3161,6 +3171,9 @@ void StatementSync::Get(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   THROW_AND_RETURN_ON_BAD_STATE(
       env, stmt->IsFinalized(), "statement has been finalized");
+  THROW_AND_RETURN_ON_BAD_STATE(
+      env, stmt->IsStepping(), "statement is currently being executed");
+  auto stepping = stmt->MarkStepping();
   int r = stmt->ResetStatement();
   CHECK_ERROR_OR_THROW(env->isolate(), stmt->db_.get(), r, SQLITE_OK, void());
 
@@ -3185,6 +3198,9 @@ void StatementSync::Run(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   THROW_AND_RETURN_ON_BAD_STATE(
       env, stmt->IsFinalized(), "statement has been finalized");
+  THROW_AND_RETURN_ON_BAD_STATE(
+      env, stmt->IsStepping(), "statement is currently being executed");
+  auto stepping = stmt->MarkStepping();
   int r = stmt->ResetStatement();
   CHECK_ERROR_OR_THROW(env->isolate(), stmt->db_.get(), r, SQLITE_OK, void());
 
@@ -3474,6 +3490,9 @@ void SQLTagStore::Run(const FunctionCallbackInfo<Value>& args) {
     return;
   }
 
+  THROW_AND_RETURN_ON_BAD_STATE(
+      env, stmt->IsStepping(), "statement is currently being executed");
+  auto stepping = stmt->MarkStepping();
   if (!ResetAndBindStatement(env, stmt.get(), args)) {
     return;
   }
@@ -3500,6 +3519,8 @@ void SQLTagStore::Iterate(const FunctionCallbackInfo<Value>& args) {
     return;
   }
 
+  THROW_AND_RETURN_ON_BAD_STATE(
+      env, stmt->IsStepping(), "statement is currently being executed");
   if (!ResetAndBindStatement(env, stmt.get(), args)) {
     return;
   }
@@ -3528,6 +3549,9 @@ void SQLTagStore::Get(const FunctionCallbackInfo<Value>& args) {
     return;
   }
 
+  THROW_AND_RETURN_ON_BAD_STATE(
+      env, stmt->IsStepping(), "statement is currently being executed");
+  auto stepping = stmt->MarkStepping();
   if (!ResetAndBindStatement(env, stmt.get(), args)) {
     return;
   }
@@ -3557,6 +3581,9 @@ void SQLTagStore::All(const FunctionCallbackInfo<Value>& args) {
     return;
   }
 
+  THROW_AND_RETURN_ON_BAD_STATE(
+      env, stmt->IsStepping(), "statement is currently being executed");
+  auto stepping = stmt->MarkStepping();
   if (!ResetAndBindStatement(env, stmt.get(), args)) {
     return;
   }
@@ -3769,6 +3796,8 @@ void StatementSyncIterator::Next(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   THROW_AND_RETURN_ON_BAD_STATE(
       env, iter->stmt_->IsFinalized(), "statement has been finalized");
+  THROW_AND_RETURN_ON_BAD_STATE(
+      env, iter->stmt_->IsStepping(), "statement is currently being executed");
   Isolate* isolate = env->isolate();
 
   auto iter_template = getLazyIterTemplate(env);
@@ -3791,6 +3820,7 @@ void StatementSyncIterator::Next(const FunctionCallbackInfo<Value>& args) {
       iter->statement_reset_generation_ != iter->stmt_->reset_generation_,
       "iterator was invalidated");
 
+  auto stepping = iter->stmt_->MarkStepping();
   int r = sqlite3_step(iter->stmt_->statement_);
   if (r != SQLITE_ROW) {
     CHECK_ERROR_OR_THROW(
@@ -3846,6 +3876,8 @@ void StatementSyncIterator::Return(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   THROW_AND_RETURN_ON_BAD_STATE(
       env, iter->stmt_->IsFinalized(), "statement has been finalized");
+  THROW_AND_RETURN_ON_BAD_STATE(
+      env, iter->stmt_->IsStepping(), "statement is currently being executed");
   Isolate* isolate = env->isolate();
 
   sqlite3_reset(iter->stmt_->statement_);

src/node_sqlite.h

@@ -291,6 +291,19 @@ class StatementSync : public BaseObject {
   bool GetCachedColumnNames(v8::LocalVector<v8::Name>* keys);
   void Finalize();
   bool IsFinalized();
+  bool IsStepping() const { return stepping_; }
+
+  // SQLite forbids stepping, resetting, or finalizing a statement while that
+  // same statement's user-defined function callback is on the stack. The
+  // callback depth tracked by DatabaseSync is per-database, so it cannot
+  // distinguish reentry into the running statement from the common pattern of
+  // querying a *different* statement from a callback. This flag marks the
+  // statement that is currently being stepped so that only the former is
+  // rejected.
+  inline auto MarkStepping() {
+    stepping_ = true;
+    return OnScopeLeave([this]() { stepping_ = false; });
+  }
 
   SET_MEMORY_INFO_NAME(StatementSync)
   SET_SELF_SIZE(StatementSync)
@@ -304,6 +317,7 @@ class StatementSync : public BaseObject {
   bool use_big_ints_;
   bool allow_bare_named_params_;
   bool allow_unknown_named_params_;
+  bool stepping_ = false;
   uint64_t reset_generation_ = 0;
   std::optional<std::map<std::string, std::string>> bare_named_params_;
   inline int ResetStatement();

test/parallel/test-sqlite-udf-statement-reentry.js

@@ -0,0 +1,210 @@
+'use strict';
+
+const { skipIfSQLiteMissing, mustCall } = require('../common');
+skipIfSQLiteMissing();
+const assert = require('node:assert');
+const { suite, test } = require('node:test');
+const { DatabaseSync } = require('node:sqlite');
+
+const reentryError = {
+  code: 'ERR_INVALID_STATE',
+  message: 'statement is currently being executed',
+};
+
+function newDbWithRows() {
+  const db = new DatabaseSync(':memory:');
+  db.exec(`
+    CREATE TABLE data (value INTEGER);
+    INSERT INTO data VALUES (1), (2), (3);
+  `);
+  return db;
+}
+
+suite('reentry into the running statement is rejected', () => {
+  for (const method of ['all', 'get', 'run']) {
+    test(`statement.${method}() from its own UDF`, () => {
+      const db = newDbWithRows();
+      let statement;
+      db.function('reenter', mustCall((value) => {
+        assert.throws(() => statement[method](), reentryError);
+        return value;
+      }));
+
+      statement = db.prepare('SELECT reenter(value) AS value FROM data LIMIT 1');
+      assert.deepStrictEqual(statement.get(), { __proto__: null, value: 1 });
+      assert.strictEqual(db.isOpen, true);
+    });
+  }
+
+  test('iterator next() from its own UDF', () => {
+    const db = newDbWithRows();
+    let iterator;
+    db.function('reenter', mustCall((value) => {
+      assert.throws(() => iterator.next(), reentryError);
+      return value;
+    }, 3));
+
+    iterator = db.prepare('SELECT reenter(value) AS value FROM data').iterate();
+    assert.deepStrictEqual([...iterator].map((row) => row.value), [1, 2, 3]);
+    assert.strictEqual(db.isOpen, true);
+  });
+
+  test('iterator return() from its own UDF', () => {
+    const db = newDbWithRows();
+    let iterator;
+    db.function('reenter', mustCall((value) => {
+      assert.throws(() => iterator.return(), reentryError);
+      return value;
+    }));
+
+    iterator = db.prepare('SELECT reenter(value) AS value FROM data').iterate();
+    assert.strictEqual(iterator.next().done, false);
+    iterator.return();
+    assert.strictEqual(db.isOpen, true);
+  });
+
+  test('recursive get() reports the reentry rather than overflowing the stack',
+       () => {
+         const db = new DatabaseSync(':memory:');
+         let statement;
+         db.function('reenter', mustCall(() => {
+           assert.throws(() => statement.get(), reentryError);
+           return 1;
+         }));
+
+         statement = db.prepare('SELECT reenter() AS value');
+         assert.deepStrictEqual(statement.get(), { __proto__: null, value: 1 });
+       });
+
+  test('statement.close() from its own UDF', () => {
+    const db = newDbWithRows();
+    let statement;
+    db.function('reenter', mustCall((value) => {
+      assert.throws(() => statement.close(), reentryError);
+      return value;
+    }));
+
+    statement = db.prepare('SELECT reenter(value) AS value FROM data LIMIT 1');
+    assert.deepStrictEqual(statement.get(), { __proto__: null, value: 1 });
+    statement.close();
+  });
+
+  test('statement[Symbol.dispose]() from its own UDF', () => {
+    const db = newDbWithRows();
+    let statement;
+    db.function('reenter', mustCall((value) => {
+      assert.throws(() => statement[Symbol.dispose](), reentryError);
+      return value;
+    }));
+
+    statement = db.prepare('SELECT reenter(value) AS value FROM data LIMIT 1');
+    assert.deepStrictEqual(statement.get(), { __proto__: null, value: 1 });
+    statement[Symbol.dispose]();
+  });
+
+  test('statement is usable again after the callback returns', () => {
+    const db = newDbWithRows();
+    let statement;
+    db.function('reenter', mustCall((value) => {
+      assert.throws(() => statement.all(), reentryError);
+      return value;
+    }, 2));
+
+    statement = db.prepare('SELECT reenter(value) AS value FROM data LIMIT 1');
+    assert.deepStrictEqual(statement.all(), [{ __proto__: null, value: 1 }]);
+    assert.deepStrictEqual(statement.all(), [{ __proto__: null, value: 1 }]);
+  });
+});
+
+suite('a different statement remains usable from a callback', () => {
+  test('the lookup pattern still works', () => {
+    const db = newDbWithRows();
+    db.exec('CREATE TABLE names (value INTEGER, name TEXT);' +
+            "INSERT INTO names VALUES (1, 'one'), (2, 'two'), (3, 'three');");
+    const lookup = db.prepare('SELECT name FROM names WHERE value = ?');
+
+    db.function('name_of', mustCall((value) => lookup.get(value).name, 3));
+
+    assert.deepStrictEqual(
+      db.prepare('SELECT name_of(value) AS name FROM data').all(),
+      [
+        { __proto__: null, name: 'one' },
+        { __proto__: null, name: 'two' },
+        { __proto__: null, name: 'three' },
+      ],
+    );
+  });
+
+  test('a nested iterator over a different statement still works', () => {
+    const db = newDbWithRows();
+    const inner = db.prepare('SELECT value FROM data');
+
+    db.function('sum_all', mustCall(() => {
+      let total = 0;
+      for (const row of inner.iterate()) {
+        total += row.value;
+      }
+      return total;
+    }));
+
+    assert.deepStrictEqual(
+      db.prepare('SELECT sum_all() AS total LIMIT 1').get(),
+      { __proto__: null, total: 6 },
+    );
+  });
+});
+
+suite('SQL tag store reentry is rejected', () => {
+  for (const method of ['all', 'get', 'run']) {
+    test(`tag store ${method} re-executing the same tag`, () => {
+      const db = newDbWithRows();
+      const sql = db.createTagStore(4);
+      db.function('reenter', mustCall((value) => {
+        assert.throws(
+          () => sql[method]`SELECT reenter(value) AS value FROM data LIMIT 1`,
+          reentryError,
+        );
+        return value;
+      }));
+
+      assert.deepStrictEqual(
+        sql.get`SELECT reenter(value) AS value FROM data LIMIT 1`,
+        { __proto__: null, value: 1 },
+      );
+      assert.strictEqual(db.isOpen, true);
+    });
+  }
+});
+
+suite('aggregate functions', () => {
+  test('reentry from an aggregate step is rejected', () => {
+    const db = newDbWithRows();
+    let statement;
+    db.aggregate('reenter_agg', {
+      start: 0,
+      step: mustCall((total, value) => {
+        assert.throws(() => statement.get(), reentryError);
+        return total + value;
+      }, 3),
+    });
+
+    statement = db.prepare('SELECT reenter_agg(value) AS total FROM data');
+    assert.deepStrictEqual(statement.get(), { __proto__: null, total: 6 });
+  });
+
+  test('reentry from an aggregate result is rejected', () => {
+    const db = newDbWithRows();
+    let statement;
+    db.aggregate('reenter_result', {
+      start: 0,
+      step: (total, value) => total + value,
+      result: mustCall((total) => {
+        assert.throws(() => statement.get(), reentryError);
+        return total;
+      }),
+    });
+
+    statement = db.prepare('SELECT reenter_result(value) AS total FROM data');
+    assert.deepStrictEqual(statement.get(), { __proto__: null, total: 6 });
+  });
+});