fixed segfault from making calls while worker thread is terminating

binding.gyp

@@ -18,7 +18,7 @@
           'dependencies': ['deps/sqlite3.gyp:sqlite3'],
           'sources': ['src/better_sqlite3.cpp'],
           'include_dirs': ["<!@(node -p \"require('node-addon-api').include\")"],
-          'defines': ['NAPI_VERSION=10', 'NAPI_DISABLE_CPP_EXCEPTIONS'],
+          'defines': ['NAPI_VERSION=10', 'NAPI_DISABLE_CPP_EXCEPTIONS', 'NODE_API_SWALLOW_UNTHROWABLE_EXCEPTIONS'],
           'cflags_cc': ['-std=c++20', '-fvisibility=hidden', '-fvisibility-inlines-hidden', '-flto'],
           'ldflags': ['-flto'],
           'xcode_settings': {

src/objects/backup.cpp

@@ -8,8 +8,7 @@ Backup::Backup(const Napi::CallbackInfo& info) :
 	id(0),
 	alive(false),
 	unlink(false) {
-	napi_status status = napi_type_tag_object(info.Env(), info.This(), &TYPE_TAG);
-	assert(status == napi_ok); ((void)status);
+	TYPE_TAG_CONSTRUCTOR(info);
 	JS_new(info);
 }
 
@@ -42,7 +41,7 @@ NODE_METHOD(Backup::JS_new) {
 	if (!addon->privileged_info) return ThrowTypeError(info.Env(), "Disabled constructor");
 	assert(info.IsConstructCall());
 	const Napi::CallbackInfo& pinfo = *addon->privileged_info;
-	Database* db = ::Unwrap<Database>(pinfo.This());
+	UNWRAP_OR_RETURN(Database, db, pinfo.This());
 	REQUIRE_DATABASE_OPEN(db->GetState());
 	REQUIRE_DATABASE_NOT_BUSY(db->GetState());
 
@@ -88,7 +87,7 @@ NODE_METHOD(Backup::JS_new) {
 }
 
 NODE_METHOD(Backup::JS_transfer) {
-	Backup* backup = ::Unwrap<Backup>(info.This());
+	UNWRAP_OR_RETURN(Backup, backup, info.This());
 	REQUIRE_ARGUMENT_INT32(first, int pages);
 	REQUIRE_DATABASE_OPEN(backup->db->GetState());
 	assert(backup->db->GetState()->busy == false);
@@ -114,7 +113,7 @@ NODE_METHOD(Backup::JS_transfer) {
 }
 
 NODE_METHOD(Backup::JS_close) {
-	Backup* backup = ::Unwrap<Backup>(info.This());
+	UNWRAP_OR_RETURN(Backup, backup, info.This());
 	assert(backup->db->GetState()->busy == false);
 	if (backup->alive) backup->db->RemoveBackup(backup);
 	backup->CloseHandles();

src/objects/database.cpp

@@ -23,8 +23,7 @@ Database::Database(const Napi::CallbackInfo& info) :
 	logger(),
 	stmts(),
 	backups() {
-	napi_status status = napi_type_tag_object(info.Env(), info.This(), &TYPE_TAG);
-	assert(status == napi_ok); ((void)status);
+	TYPE_TAG_CONSTRUCTOR(info);
 	JS_new(info);
 }
 
@@ -215,7 +214,7 @@ NODE_METHOD(Database::JS_prepare) {
 }
 
 NODE_METHOD(Database::JS_exec) {
-	Database* db = ::Unwrap<Database>(info.This());
+	UNWRAP_OR_RETURN(Database, db, info.This());
 	REQUIRE_ARGUMENT_STRING(first, Napi::String source);
 	REQUIRE_DATABASE_OPEN(db);
 	REQUIRE_DATABASE_NOT_BUSY(db);
@@ -275,7 +274,7 @@ NODE_METHOD(Database::JS_backup) {
 }
 
 NODE_METHOD(Database::JS_serialize) {
-	Database* db = ::Unwrap<Database>(info.This());
+	UNWRAP_OR_RETURN(Database, db, info.This());
 	REQUIRE_ARGUMENT_STRING(first, Napi::String attachedName);
 	REQUIRE_DATABASE_OPEN(db);
 	REQUIRE_DATABASE_NOT_BUSY(db);
@@ -294,7 +293,7 @@ NODE_METHOD(Database::JS_serialize) {
 }
 
 NODE_METHOD(Database::JS_function) {
-	Database* db = ::Unwrap<Database>(info.This());
+	UNWRAP_OR_RETURN(Database, db, info.This());
 	REQUIRE_ARGUMENT_FUNCTION(first, Napi::Function fn);
 	REQUIRE_ARGUMENT_STRING(second, Napi::String nameString);
 	REQUIRE_ARGUMENT_INT32(third, int argc);
@@ -319,7 +318,7 @@ NODE_METHOD(Database::JS_function) {
 }
 
 NODE_METHOD(Database::JS_aggregate) {
-	Database* db = ::Unwrap<Database>(info.This());
+	UNWRAP_OR_RETURN(Database, db, info.This());
 	REQUIRE_ARGUMENT_ANY(first, Napi::Value start);
 	REQUIRE_ARGUMENT_FUNCTION(second, Napi::Function step);
 	REQUIRE_ARGUMENT_ANY(third, Napi::Value inverse);
@@ -349,7 +348,7 @@ NODE_METHOD(Database::JS_aggregate) {
 }
 
 NODE_METHOD(Database::JS_table) {
-	Database* db = ::Unwrap<Database>(info.This());
+	UNWRAP_OR_RETURN(Database, db, info.This());
 	REQUIRE_ARGUMENT_FUNCTION(first, Napi::Function factory);
 	REQUIRE_ARGUMENT_STRING(second, Napi::String nameString);
 	REQUIRE_ARGUMENT_BOOLEAN(third, bool eponymous);
@@ -370,7 +369,7 @@ NODE_METHOD(Database::JS_table) {
 }
 
 NODE_METHOD(Database::JS_loadExtension) {
-	Database* db = ::Unwrap<Database>(info.This());
+	UNWRAP_OR_RETURN(Database, db, info.This());
 	Napi::String entryPoint;
 	REQUIRE_ARGUMENT_STRING(first, Napi::String filename);
 	if (info.Length() > 1) { REQUIRE_ARGUMENT_STRING(second, entryPoint); }
@@ -397,7 +396,7 @@ NODE_METHOD(Database::JS_loadExtension) {
 }
 
 NODE_METHOD(Database::JS_close) {
-	Database* db = ::Unwrap<Database>(info.This());
+	UNWRAP_OR_RETURN(Database, db, info.This());
 	if (db->open) {
 		REQUIRE_DATABASE_NOT_BUSY(db);
 		REQUIRE_DATABASE_NO_ITERATORS(db);
@@ -408,14 +407,14 @@ NODE_METHOD(Database::JS_close) {
 }
 
 NODE_METHOD(Database::JS_defaultSafeIntegers) {
-	Database* db = ::Unwrap<Database>(info.This());
+	UNWRAP_OR_RETURN(Database, db, info.This());
 	if (info.Length() == 0) db->safe_ints = true;
 	else { REQUIRE_ARGUMENT_BOOLEAN(first, db->safe_ints); }
 	return info.Env().Undefined();
 }
 
 NODE_METHOD(Database::JS_unsafeMode) {
-	Database* db = ::Unwrap<Database>(info.This());
+	UNWRAP_OR_RETURN(Database, db, info.This());
 	if (info.Length() == 0) db->unsafe_mode = true;
 	else { REQUIRE_ARGUMENT_BOOLEAN(first, db->unsafe_mode); }
 	sqlite3_db_config(db->db_handle, SQLITE_DBCONFIG_DEFENSIVE, static_cast<int>(!db->unsafe_mode), NULL);
@@ -423,11 +422,11 @@ NODE_METHOD(Database::JS_unsafeMode) {
 }
 
 NODE_GETTER(Database::JS_open) {
-	Database* db = ::Unwrap<Database>(info.This());
+	UNWRAP_OR_RETURN(Database, db, info.This());
 	return Napi::Boolean::New(info.Env(), db->open);
 }
 
 NODE_GETTER(Database::JS_inTransaction) {
-	Database* db = ::Unwrap<Database>(info.This());
+	UNWRAP_OR_RETURN(Database, db, info.This());
 	return Napi::Boolean::New(info.Env(), db->open && !static_cast<bool>(sqlite3_get_autocommit(db->db_handle)));
 }

src/objects/statement-iterator.cpp

@@ -10,8 +10,7 @@ StatementIterator::StatementIterator(const Napi::CallbackInfo& info) :
 	mode(Data::FLAT),
 	alive(false),
 	logged(false) {
-	napi_status status = napi_type_tag_object(info.Env(), info.This(), &TYPE_TAG);
-	assert(status == napi_ok); ((void)status);
+	TYPE_TAG_CONSTRUCTOR(info);
 	JS_new(info);
 }
 
@@ -102,14 +101,14 @@ NODE_METHOD(StatementIterator::JS_new) {
 }
 
 NODE_METHOD(StatementIterator::JS_next) {
-	StatementIterator* iter = ::Unwrap<StatementIterator>(info.This());
+	UNWRAP_OR_RETURN(StatementIterator, iter, info.This());
 	REQUIRE_DATABASE_NOT_BUSY(iter->db_state);
 	if (iter->alive) return iter->Next(info.Env());
 	return DoneRecord(info.Env(), iter->db_state->addon);
 }
 
 NODE_METHOD(StatementIterator::JS_return) {
-	StatementIterator* iter = ::Unwrap<StatementIterator>(info.This());
+	UNWRAP_OR_RETURN(StatementIterator, iter, info.This());
 	REQUIRE_DATABASE_NOT_BUSY(iter->db_state);
 	if (iter->alive) return iter->Return(info.Env());
 	return DoneRecord(info.Env(), iter->db_state->addon);

src/objects/statement-iterator.hpp

@@ -45,9 +45,9 @@ private:
 
 		napi_value record;
 		napi_status status = napi_create_object(env, &record);
-		assert(status == napi_ok);
+		assert(status == napi_ok || status == napi_cannot_run_js);
 		status = napi_define_properties(env, record, 2, properties);
-		assert(status == napi_ok); ((void)status);
+		assert(status == napi_ok || status == napi_cannot_run_js); ((void)status);
 		return Napi::Object(env, record);
 	}
 

src/objects/statement.cpp

@@ -12,8 +12,7 @@ Statement::Statement(const Napi::CallbackInfo& info) :
 	safe_ints(false),
 	mode(Data::FLAT),
 	returns_data(false) {
-	napi_status status = napi_type_tag_object(info.Env(), info.This(), &TYPE_TAG);
-	assert(status == napi_ok); ((void)status);
+	TYPE_TAG_CONSTRUCTOR(info);
 	JS_new(info);
 }
 
@@ -82,7 +81,7 @@ NODE_METHOD(Statement::JS_new) {
 	}
 	assert(info.IsConstructCall());
 	const Napi::CallbackInfo& pinfo = *addon->privileged_info;
-	Database* db = ::Unwrap<Database>(pinfo.This());
+	UNWRAP_OR_RETURN(Database, db, pinfo.This());
 	REQUIRE_DATABASE_OPEN(db->GetState());
 	REQUIRE_DATABASE_NOT_BUSY(db->GetState());
 
@@ -187,9 +186,9 @@ NODE_METHOD(Statement::JS_run) {
 
 		napi_value result;
 		napi_status status = napi_create_object(env, &result);
-		assert(status == napi_ok);
+		assert(status == napi_ok || status == napi_cannot_run_js);
 		status = napi_define_properties(env, result, 2, properties);
-		assert(status == napi_ok); ((void)status);
+		assert(status == napi_ok || status == napi_cannot_run_js); ((void)status);
 		STATEMENT_RETURN(Napi::Object(env, result));
 	}
 	STATEMENT_THROW();
@@ -265,7 +264,7 @@ NODE_METHOD(Statement::JS_iterate) {
 }
 
 NODE_METHOD(Statement::JS_bind) {
-	Statement* stmt = ::Unwrap<Statement>(info.This());
+	UNWRAP_OR_RETURN(Statement, stmt, info.This());
 	if (stmt->bound) return ThrowTypeError(info.Env(), "The bind() method can only be invoked once per statement object");
 	REQUIRE_DATABASE_OPEN(stmt->db->GetState());
 	REQUIRE_DATABASE_NOT_BUSY(stmt->db->GetState());
@@ -276,7 +275,7 @@ NODE_METHOD(Statement::JS_bind) {
 }
 
 NODE_METHOD(Statement::JS_pluck) {
-	Statement* stmt = ::Unwrap<Statement>(info.This());
+	UNWRAP_OR_RETURN(Statement, stmt, info.This());
 	if (!stmt->returns_data) return ThrowTypeError(info.Env(), "The pluck() method is only for statements that return data");
 	REQUIRE_DATABASE_NOT_BUSY(stmt->db->GetState());
 	REQUIRE_STATEMENT_NOT_LOCKED(stmt);
@@ -287,7 +286,7 @@ NODE_METHOD(Statement::JS_pluck) {
 }
 
 NODE_METHOD(Statement::JS_expand) {
-	Statement* stmt = ::Unwrap<Statement>(info.This());
+	UNWRAP_OR_RETURN(Statement, stmt, info.This());
 	if (!stmt->returns_data) return ThrowTypeError(info.Env(), "The expand() method is only for statements that return data");
 	REQUIRE_DATABASE_NOT_BUSY(stmt->db->GetState());
 	REQUIRE_STATEMENT_NOT_LOCKED(stmt);
@@ -298,7 +297,7 @@ NODE_METHOD(Statement::JS_expand) {
 }
 
 NODE_METHOD(Statement::JS_raw) {
-	Statement* stmt = ::Unwrap<Statement>(info.This());
+	UNWRAP_OR_RETURN(Statement, stmt, info.This());
 	if (!stmt->returns_data) return ThrowTypeError(info.Env(), "The raw() method is only for statements that return data");
 	REQUIRE_DATABASE_NOT_BUSY(stmt->db->GetState());
 	REQUIRE_STATEMENT_NOT_LOCKED(stmt);
@@ -309,7 +308,7 @@ NODE_METHOD(Statement::JS_raw) {
 }
 
 NODE_METHOD(Statement::JS_safeIntegers) {
-	Statement* stmt = ::Unwrap<Statement>(info.This());
+	UNWRAP_OR_RETURN(Statement, stmt, info.This());
 	REQUIRE_DATABASE_NOT_BUSY(stmt->db->GetState());
 	REQUIRE_STATEMENT_NOT_LOCKED(stmt);
 	if (info.Length() == 0) stmt->safe_ints = true;
@@ -318,7 +317,7 @@ NODE_METHOD(Statement::JS_safeIntegers) {
 }
 
 NODE_METHOD(Statement::JS_columns) {
-	Statement* stmt = ::Unwrap<Statement>(info.This());
+	UNWRAP_OR_RETURN(Statement, stmt, info.This());
 	if (!stmt->returns_data) return ThrowTypeError(info.Env(), "The columns() method is only for statements that return data");
 	REQUIRE_DATABASE_OPEN(stmt->db->GetState());
 	REQUIRE_DATABASE_NOT_BUSY(stmt->db->GetState());
@@ -360,7 +359,7 @@ NODE_METHOD(Statement::JS_columns) {
 }
 
 NODE_METHOD(Statement::JS_toString) {
-	Statement* stmt = ::Unwrap<Statement>(info.This());
+	UNWRAP_OR_RETURN(Statement, stmt, info.This());
 	Addon* addon = stmt->db->GetAddon();
 
 	char* expanded = stmt->alive && stmt->bound ? sqlite3_expanded_sql(stmt->handle) : NULL;
@@ -376,6 +375,6 @@ NODE_METHOD(Statement::JS_toString) {
 }
 
 NODE_GETTER(Statement::JS_busy) {
-	Statement* stmt = ::Unwrap<Statement>(info.This());
+	UNWRAP_OR_RETURN(Statement, stmt, info.This());
 	return Napi::Boolean::New(info.Env(), stmt->alive && stmt->locked);
 }

src/util/helpers.cpp

@@ -90,7 +90,9 @@ Napi::Value ThrowRangeError(Napi::Env env, const char* message) {
 // only be used on values that are already known to be backed by the given
 // class (e.g., a receiver that was validated by TypeSafeCallback below, or a
 // value produced by our own code), because napi_unwrap does not check the
-// object's type.
+// object's type. Returns NULL if the environment can no longer run JavaScript
+// (e.g., worker.terminate()), so callers should use UNWRAP_OR_RETURN, which
+// checks for that case.
 template <typename T> inline T* Unwrap(Napi::Value value) {
 	return T::Unwrap(value.As<Napi::Object>());
 }
@@ -169,7 +171,7 @@ void SetInstanceGetter(Napi::Object obj, const char* name, Addon* addon) {
 	desc.attributes = napi_enumerable;
 	desc.data = addon;
 	napi_status status = napi_define_properties(obj.Env(), obj, 1, &desc);
-	assert(status == napi_ok); ((void)status);
+	assert(status == napi_ok || status == napi_cannot_run_js); ((void)status);
 }
 
 // Determines whether to skip the given character at the start of an SQL string.

src/util/macros.cpp

@@ -8,6 +8,20 @@
 #define UseIsolate Napi::Env env = info.Env()
 #define UseAddon Addon* addon = static_cast<Addon*>(info.Data())
 
+// Type-tag an object, but bail if the thread is terminating, to avoid
+// undesirable side-effects in the subsequent JS_new call (e.g, sqlite3_open()).
+#define TYPE_TAG_CONSTRUCTOR(info)                                             \
+	napi_status status =                                                       \
+		napi_type_tag_object((info).Env(), (info).This(), &TYPE_TAG);          \
+	assert(status == napi_ok || status == napi_cannot_run_js);                 \
+	if (status != napi_ok) return
+
+// Unwrap a native-backed object (see ::Unwrap in helpers.cpp), but bail if the
+// the unwrap fails (which should only happen if the thread is terminating).
+#define UNWRAP_OR_RETURN(T, var, value)                                        \
+	T* var = ::Unwrap<T>(value);                                               \
+	if (var == NULL) return info.Env().Undefined()
+
 #define REQUIRE_ARGUMENT_ANY(at, var)                                          \
 	if (info.Length() <= (at()))                                               \
 		return ThrowTypeError(info.Env(), "Expected a "#at" argument");        \

src/util/query-macros.cpp

@@ -16,7 +16,7 @@
 	return _return_value
 
 #define STATEMENT_START_LOGIC(RETURNS_DATA_CHECK, MUTATE_CHECK)                \
-	Statement* stmt = ::Unwrap<Statement>(info.This());                        \
+	UNWRAP_OR_RETURN(Statement, stmt, info.This());                            \
 	RETURNS_DATA_CHECK();                                                      \
 	sqlite3_stmt* handle = stmt->handle;                                       \
 	Database* db = stmt->db;                                                   \