Skip to content

Commit 385708b

Browse files
committed
sqlite: skip copying changeset when no callbacks are set
Refactor: only copy the changeset before applying it when a filter or onConflict callback is present, since SQLite can only invoke JavaScript mid-apply in that case. Without callbacks, the input buffer cannot be detached or modified during sqlite3changeset_apply(), so no copy is needed and the buffer is passed through directly. Address review comment from geeksilva97. Signed-off-by: Matteo Collina <hello@matteocollina.com>
1 parent 89990be commit 385708b

1 file changed

Lines changed: 15 additions & 12 deletions

File tree

src/node_sqlite.cc

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2397,11 +2397,8 @@ void DatabaseSync::ApplyChangeset(const FunctionCallbackInfo<Value>& args) {
23972397
}
23982398
}
23992399

2400-
// Keep the database alive during sqlite3changeset_apply(), which may
2401-
// call conflict or filter callbacks that trigger JavaScript execution.
2402-
// If the JavaScript callback drops all references to the database,
2403-
// the DatabaseSync could otherwise be garbage-collected while the
2404-
// callback is still executing, causing a use-after-free.
2400+
// Keep the database alive in case a callback drops all references to it,
2401+
// which could otherwise let it be garbage-collected mid-callback.
24052402
BaseObjectPtr<DatabaseSync> guard(db);
24062403

24072404
ArrayBufferViewContents<uint8_t> buf(args[0]);
@@ -2410,8 +2407,12 @@ void DatabaseSync::ApplyChangeset(const FunctionCallbackInfo<Value>& args) {
24102407
return;
24112408
}
24122409

2410+
// A callback may detach/modify the input buffer mid-apply, so copy it.
2411+
// With no callbacks, no JS runs during sqlite3changeset_apply(), so no
2412+
// copy is needed.
24132413
std::unique_ptr<BackingStore> changeset;
2414-
if (buf.length() > 0) {
2414+
if (buf.length() > 0 &&
2415+
(context.filterCallback || context.conflictCallback)) {
24152416
changeset = ArrayBuffer::NewBackingStore(
24162417
env->isolate(),
24172418
buf.length(),
@@ -2427,12 +2428,14 @@ void DatabaseSync::ApplyChangeset(const FunctionCallbackInfo<Value>& args) {
24272428
int r;
24282429
{
24292430
CallbackDepthGuard guard(db);
2430-
r = sqlite3changeset_apply(db->connection_,
2431-
static_cast<int>(buf.length()),
2432-
changeset ? changeset->Data() : nullptr,
2433-
context.filterCallback ? xFilter : nullptr,
2434-
xConflict,
2435-
static_cast<void*>(&context));
2431+
r = sqlite3changeset_apply(
2432+
db->connection_,
2433+
static_cast<int>(buf.length()),
2434+
changeset ? changeset->Data()
2435+
: const_cast<void*>(static_cast<const void*>(buf.data())),
2436+
context.filterCallback ? xFilter : nullptr,
2437+
xConflict,
2438+
static_cast<void*>(&context));
24362439
}
24372440
if (r == SQLITE_OK) {
24382441
args.GetReturnValue().Set(true);

0 commit comments

Comments
 (0)