diff --git a/.vscode/launch.json b/.vscode/launch.json index 592d9c0ba8..95be86aa2d 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -74,7 +74,7 @@ "--import=tsx", "--expose_gc", "--enable-source-maps", - "--no-warnings", + "--force-node-api-uncaught-exceptions-policy", "${workspaceRoot}/node_modules/mocha/lib/cli/cli.js", "--require", "src/node/inject-dev-environment.ts", @@ -85,6 +85,28 @@ "${input:integrationTestFilter}" ] }, + { + "type": "lldb", + "request": "launch", + "name": "LLDB: Integration tests (throwing from listeners)", + "program": "node", + "cwd": "${workspaceRoot}/integration-tests/tests", + "args": [ + "--inspect", + "--import=tsx", + "--expose_gc", + "--enable-source-maps", + "--force-node-api-uncaught-exceptions-policy", + "${workspaceRoot}/node_modules/mocha/lib/cli/cli.js", + "--require", + "src/node/inject-dev-environment.ts", + "src/node/index.ts", + "--timeout", + "10000", + "--grep", + "can throw from a listener" + ] + }, { "type": "node", "presentation": { @@ -99,7 +121,7 @@ "--import=tsx", "--expose_gc", "--enable-source-maps", - "--no-warnings" + "--force-node-api-uncaught-exceptions-policy" ], "args": [ "--require", diff --git a/integration-tests/environments/node/package.json b/integration-tests/environments/node/package.json index 41670b5498..2078944b58 100644 --- a/integration-tests/environments/node/package.json +++ b/integration-tests/environments/node/package.json @@ -6,12 +6,12 @@ "scripts": { "test": "wireit", "test:commonjs": "wireit", - "test:ci": "mocha-remote --reporter @realm/mocha-reporter -- tsx index.mjs", + "test:ci": "mocha-remote --reporter @realm/mocha-reporter -- tsx --force-node-api-uncaught-exceptions-policy index.mjs", "lint": "eslint --ext js,mjs ." }, "wireit": { "test": { - "command": "mocha-remote --reporter @realm/mocha-reporter -- tsx index.mjs", + "command": "mocha-remote --reporter @realm/mocha-reporter -- tsx --force-node-api-uncaught-exceptions-policy index.mjs", "dependencies": [ "../../../packages/realm:build:ts", "../../../packages/realm:build:node", @@ -19,7 +19,7 @@ ] }, "test:commonjs": { - "command": "mocha-remote --reporter @realm/mocha-reporter -- tsx index.cjs", + "command": "mocha-remote --reporter @realm/mocha-reporter -- tsx --force-node-api-uncaught-exceptions-policy index.cjs", "dependencies": [ "../../../packages/realm:build:ts", "../../../packages/realm:build:node", diff --git a/integration-tests/tests/.mocharc.json b/integration-tests/tests/.mocharc.json index ee3ac57d2c..69b06688a5 100644 --- a/integration-tests/tests/.mocharc.json +++ b/integration-tests/tests/.mocharc.json @@ -3,7 +3,7 @@ "import=tsx", "expose_gc", "enable-source-maps", - "no-warnings" + "force-node-api-uncaught-exceptions-policy" ], "reporter": "@realm/mocha-reporter", "require": [ diff --git a/integration-tests/tests/src/node/setup-globals.ts b/integration-tests/tests/src/node/setup-globals.ts index 3007862dd5..aaf7e9d43c 100644 --- a/integration-tests/tests/src/node/setup-globals.ts +++ b/integration-tests/tests/src/node/setup-globals.ts @@ -44,6 +44,29 @@ Object.assign(globalThis, { }, }, gc: vm.runInNewContext("gc"), + async nextUncaughtException(timeoutMs = 5000) { + // Remove any other listeners, storing them later so they can be restored + const listenersBefore = process.listeners("uncaughtException"); + process.removeAllListeners("uncaughtException"); + try { + return await new Promise((resolve, reject) => { + const timeoutTimer = setTimeout(() => { + process.off("uncaughtException", handleException); + const error = new Error(`Timed out waiting for uncaught exception (waited ${timeoutMs} ms)`); + reject(error); + }, timeoutMs); + function handleException(error: Error) { + clearTimeout(timeoutTimer); + resolve(error); + } + process.once("uncaughtException", handleException); + }); + } finally { + for (const listener of listenersBefore) { + process.addListener("uncaughtException", listener); + } + } + }, }); // Indicate that the tests are running in Node diff --git a/integration-tests/tests/src/tests/observable.ts b/integration-tests/tests/src/tests/observable.ts index 2b02dd407d..995af0a8a8 100644 --- a/integration-tests/tests/src/tests/observable.ts +++ b/integration-tests/tests/src/tests/observable.ts @@ -466,6 +466,20 @@ describe("Observable", () => { expectObservableMethods(this.object); }); + it.skipIf( + typeof nextUncaughtException !== "function", + "can throw from a listener", + async function (this: RealmObjectContext) { + assert(typeof nextUncaughtException === "function", "Expected ability to await an uncaught exception"); + const uncaughtException = nextUncaughtException(); + this.object.addListener(() => { + throw new Error("boom!"); + }); + const error = await uncaughtException; + expect(error.message).equals("boom!"); + }, + ); + it("throws on listener reuse", function (this: RealmObjectContext) { this.object.addListener(noop); expect(() => { @@ -869,6 +883,20 @@ describe("Observable", () => { expectObservableMethods(this.realm.objects("Person")); }); + it.skipIf( + typeof nextUncaughtException !== "function", + "can throw from a listener", + async function (this: RealmObjectContext) { + assert(typeof nextUncaughtException === "function", "Expected ability to await an uncaught exception"); + const uncaughtException = nextUncaughtException(); + this.realm.objects("Person").addListener(() => { + throw new Error("boom!"); + }); + const error = await uncaughtException; + expect(error.message).equals("boom!"); + }, + ); + it("throws on listener reuse", function (this: RealmObjectContext) { const collection = this.realm.objects("Person"); collection.addListener(noop); diff --git a/integration-tests/tests/src/typings.d.ts b/integration-tests/tests/src/typings.d.ts index 5ff4744775..e64c0dba95 100644 --- a/integration-tests/tests/src/typings.d.ts +++ b/integration-tests/tests/src/typings.d.ts @@ -116,6 +116,7 @@ declare const fs: fs; declare const path: path; declare const environment: Environment; declare const gc: undefined | (() => void); +declare const nextUncaughtException: undefined | (() => Promise); // Extend the mocha test function with the skipIf that we patch in from index.ts declare namespace Mocha { diff --git a/packages/realm/bindgen/js_opt_in_spec.yml b/packages/realm/bindgen/js_opt_in_spec.yml index ef80c1274e..e75f971c11 100644 --- a/packages/realm/bindgen/js_opt_in_spec.yml +++ b/packages/realm/bindgen/js_opt_in_spec.yml @@ -45,6 +45,7 @@ records: - schema_version - schema_mode - disable_format_upgrade + - scheduler - sync_config - force_sync_history - migration_function diff --git a/packages/realm/bindgen/src/templates/node-wrapper.ts b/packages/realm/bindgen/src/templates/node-wrapper.ts index 0c7cb558a2..ed98a40e0a 100644 --- a/packages/realm/bindgen/src/templates/node-wrapper.ts +++ b/packages/realm/bindgen/src/templates/node-wrapper.ts @@ -36,6 +36,8 @@ export function generate(context: TemplateContext): void { // We know that node always has real WeakRefs so just use them. export const WeakRef = global.WeakRef; + // Export a special function to get the env specific scheduler + export const getPlatformScheduler = nativeModule.getPlatformScheduler; `); generateNativeBigIntSupport(out); diff --git a/packages/realm/bindgen/src/templates/node.ts b/packages/realm/bindgen/src/templates/node.ts index a40aa6a60c..c7c202bee3 100644 --- a/packages/realm/bindgen/src/templates/node.ts +++ b/packages/realm/bindgen/src/templates/node.ts @@ -18,7 +18,7 @@ import { strict as assert } from "assert"; import { TemplateContext } from "@realm/bindgen/context"; -import { CppVar, CppFunc, CppFuncProps, CppCtor, CppMethod, CppClass, CppDecls } from "@realm/bindgen/cpp"; +import { CppVar, CppFunc, CppFuncProps, CppCtor, CppMethod, CppClass, CppDecls, CppMemInit } from "@realm/bindgen/cpp"; import { BoundSpec, Class, @@ -83,6 +83,7 @@ class NodeAddon extends CppClass { this.withCrtpBase("Napi::Addon"); this.members.push(new CppVar("std::deque", "m_string_bufs")); + this.members.push(new CppVar("std::shared_ptr", "m_scheduler")); this.addMethod( new CppMethod("wrapString", "const std::string&", [new CppVar("std::string", "str")], { attributes: "inline", @@ -104,6 +105,8 @@ class NodeAddon extends CppClass { this.classes.forEach((t) => this.members.push(new CppVar("Napi::FunctionReference", NodeAddon.memberNameForExtractor(t))), ); + + // Injectables this.addMethod( new CppMethod("injectInjectables", "void", [node_callback_info], { body: ` @@ -122,14 +125,26 @@ class NodeAddon extends CppClass { }), ); + // Env specific scheduler + this.addMethod( + new CppMethod("getPlatformScheduler", "Napi::Value", [node_callback_info], { + body: ` + const auto env = info.Env(); + return NODE_FROM_SHARED_Scheduler(env, env.GetInstanceData()->m_scheduler); + `, + }), + ); + this.addMethod( new CppCtor(this.name, [new CppVar("Napi::Env", env), new CppVar("Napi::Object", "exports")], { + mem_inits: [new CppMemInit("m_scheduler", `std::make_shared(${env})`)], body: ` DefineAddon(exports, { ${Object.entries(this.exports) .map(([name, val]) => `InstanceValue("${name}", ${val}, napi_enumerable),`) .join("\n")} InstanceMethod<&${this.name}::injectInjectables>("injectInjectables"), + InstanceMethod<&${this.name}::getPlatformScheduler>("getPlatformScheduler"), }); `, }), @@ -585,7 +600,9 @@ function convertFromNode(addon: NodeAddon, type: Type, expr: string): string { // For now assuming that all void-returning functions are "notifications" and don't need to block until done. // Non-void returning functions *must* block so they have something to return. const shouldBlock = !type.ret.isVoid(); - return shouldBlock ? `schedulerWrapBlockingFunction(${lambda})` : `util::EventLoopDispatcher(${lambda})`; + return shouldBlock + ? `schedulerWrapBlockingFunction(${lambda}, ${env}.GetInstanceData()->m_scheduler)` + : `util::EventLoopDispatcher(${lambda}, ${env}.GetInstanceData()->m_scheduler)`; case "Enum": return `${type.cppName}((${expr}).As().DoubleValue())`; @@ -936,6 +953,7 @@ export function generate({ rawSpec, spec, file: makeFile }: TemplateContext): vo #include #include #include + #include namespace realm::js::node { namespace { diff --git a/packages/realm/bindgen/src/templates/typescript.ts b/packages/realm/bindgen/src/templates/typescript.ts index 44de3541e4..f5a7536b85 100644 --- a/packages/realm/bindgen/src/templates/typescript.ts +++ b/packages/realm/bindgen/src/templates/typescript.ts @@ -187,6 +187,9 @@ export function generate({ rawSpec, spec: boundSpec, file }: TemplateContext): v out("export type AppError = Error & {code: number};"); out("export type CppErrorCode = Error & {code: number, category: string};"); + out("// Special functions"); + out("export const getPlatformScheduler: undefined | (() => binding.SharedScheduler);"); + out(` // WeakRef polyfill for Hermes. export class WeakRef { diff --git a/packages/realm/bindgen/vendor/realm-core b/packages/realm/bindgen/vendor/realm-core index 6bebc40a03..0f0e1ed638 160000 --- a/packages/realm/bindgen/vendor/realm-core +++ b/packages/realm/bindgen/vendor/realm-core @@ -1 +1 @@ -Subproject commit 6bebc40a03ca4144050bc672a6cd86c2286caa32 +Subproject commit 0f0e1ed63863d920410afb5d9655c49c497a7e52 diff --git a/packages/realm/binding/node/CMakeLists.txt b/packages/realm/binding/node/CMakeLists.txt index 389d9a1f06..a04d888c16 100644 --- a/packages/realm/binding/node/CMakeLists.txt +++ b/packages/realm/binding/node/CMakeLists.txt @@ -108,8 +108,7 @@ else() target_compile_options(realm-js PRIVATE -Wall -Wextra) endif() -target_include_directories(realm-js PRIVATE "${BINDGEN_DIR}/src") -target_include_directories(realm-js PRIVATE "${BINDING_DIR}") +target_include_directories(realm-js PRIVATE "${BINDGEN_DIR}/src" "${BINDING_DIR}" "${BINDING_DIR}/node") file(GLOB_RECURSE SDK_TS_FILES @@ -137,4 +136,4 @@ bindgen( SOURCES ${SDK_TS_FILES} ) -target_sources(realm-js PRIVATE node_init.cpp ${CMAKE_JS_SRC} ${BINDING_DIR}/node/platform.cpp) +target_sources(realm-js PRIVATE node_init.cpp ${CMAKE_JS_SRC} ${BINDING_DIR}/node/platform.cpp ${BINDING_DIR}/node/napi_scheduler.cpp) diff --git a/packages/realm/binding/node/napi_scheduler.cpp b/packages/realm/binding/node/napi_scheduler.cpp new file mode 100644 index 0000000000..6d77e907a5 --- /dev/null +++ b/packages/realm/binding/node/napi_scheduler.cpp @@ -0,0 +1,79 @@ +//////////////////////////////////////////////////////////////////////////// +// +// Copyright 2024 Realm Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +//////////////////////////////////////////////////////////////////////////// + +#include "napi_scheduler.h" + +#include +#include + +#include + +#include +#include +#include + +namespace realm::js::node { + +namespace { +/** + * Assumes called exactly once per data value: + * An absent call results in a leak and multiple calls result in use-after-free. + */ +void call_func_from_data(Napi::Env, Napi::Function, std::nullptr_t*, VoidUniqueFunctionImpl* data) +{ + (realm::util::UniqueFunction(data))(); +} + +/** + * A NAPI thread-safe function which use the data to construct and call a `UniqueFunction`: + * Simpler and faster than passing and calling a `Napi::Function` to `NonBlockingCall`. + */ +using SchedulerThreadSafeFunction = + Napi::TypedThreadSafeFunction; + +} // namespace + +NapiScheduler::NapiScheduler(Napi::Env& env) + : m_env(env) + // TODO: Consider including an id from the env in the resource name + , m_tsf(SchedulerThreadSafeFunction::New(env, "realm::NapiScheduler", 0, 1)) +{ +} + +bool NapiScheduler::is_on_thread() const noexcept +{ + return m_id == std::this_thread::get_id(); +} + +bool NapiScheduler::is_same_as(const Scheduler* other) const noexcept +{ + auto o = dynamic_cast(other); + return (o && (o->m_env == m_env)); +} + +bool NapiScheduler::can_invoke() const noexcept +{ + return true; +} + +void NapiScheduler::invoke(realm::util::UniqueFunction&& func) +{ + m_tsf.NonBlockingCall(func.release()); +} + +} // namespace realm::js::node diff --git a/packages/realm/binding/node/napi_scheduler.h b/packages/realm/binding/node/napi_scheduler.h new file mode 100644 index 0000000000..1a5c96a4db --- /dev/null +++ b/packages/realm/binding/node/napi_scheduler.h @@ -0,0 +1,46 @@ +//////////////////////////////////////////////////////////////////////////// +// +// Copyright 2024 Realm Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +//////////////////////////////////////////////////////////////////////////// + +#pragma once + +#include +#include + +#include + +#include + +namespace realm::js::node { + +using VoidUniqueFunctionImpl = std::remove_pointer_t().release())>; + +class NapiScheduler : public realm::util::Scheduler { +public: + NapiScheduler(Napi::Env& env); + bool is_on_thread() const noexcept override; + bool is_same_as(const Scheduler* other) const noexcept override; + bool can_invoke() const noexcept override; + void invoke(realm::util::UniqueFunction&& func) override; + +private: + Napi::Env m_env; + Napi::TypedThreadSafeFunction m_tsf; + std::thread::id m_id = std::this_thread::get_id(); +}; + +} // namespace realm::js::node diff --git a/packages/realm/src/Dictionary.ts b/packages/realm/src/Dictionary.ts index 9bb10be828..77eea45cfc 100644 --- a/packages/realm/src/Dictionary.ts +++ b/packages/realm/src/Dictionary.ts @@ -146,28 +146,20 @@ export class Dictionary extends Collection< super(accessor, typeHelpers, (listener, keyPaths) => { return this[INTERNAL].addKeyBasedNotificationCallback( ({ deletions, insertions, modifications }) => { - try { - listener(proxied, { - deletions: deletions.map((value) => { - assert.string(value); - return value; - }), - insertions: insertions.map((value) => { - assert.string(value); - return value; - }), - modifications: modifications.map((value) => { - assert.string(value); - return value; - }), - }); - } catch (err) { - // Scheduling a throw on the event loop, - // since throwing synchronously here would result in an abort in the calling C++ - setImmediate(() => { - throw err; - }); - } + listener(proxied, { + deletions: deletions.map((value) => { + assert.string(value); + return value; + }), + insertions: insertions.map((value) => { + assert.string(value); + return value; + }), + modifications: modifications.map((value) => { + assert.string(value); + return value; + }), + }); }, keyPaths ? realm.internal.createKeyPathArray(internal.objectSchema.name, keyPaths) : keyPaths, ); diff --git a/packages/realm/src/ObjectListeners.ts b/packages/realm/src/ObjectListeners.ts index 6813ffaead..0bf6f00c29 100644 --- a/packages/realm/src/ObjectListeners.ts +++ b/packages/realm/src/ObjectListeners.ts @@ -57,18 +57,10 @@ export class ObjectListeners { add: (callback, keyPaths) => { const token = this.notifier.addCallback( (changes) => { - try { - callback(this.object as RealmObject & T, { - deleted: changes.isDeleted, - changedProperties: changes.changedColumns.map(this.properties.getName), - }); - } catch (err) { - // Scheduling a throw on the event loop, - // since throwing synchronously here would result in an abort in the calling C++ - setImmediate(() => { - throw err; - }); - } + callback(this.object as RealmObject & T, { + deleted: changes.isDeleted, + changedProperties: changes.changedColumns.map(this.properties.getName), + }); }, keyPaths ? this.mapKeyPaths(keyPaths) : undefined, ); diff --git a/packages/realm/src/OrderedCollection.ts b/packages/realm/src/OrderedCollection.ts index 59b0407620..1bd8808a6b 100644 --- a/packages/realm/src/OrderedCollection.ts +++ b/packages/realm/src/OrderedCollection.ts @@ -183,20 +183,12 @@ export abstract class OrderedCollection< super(accessor, typeHelpers, (callback, keyPaths) => { return results.addNotificationCallback( (changes) => { - try { - callback(proxied, { - deletions: unwind(changes.deletions), - insertions: unwind(changes.insertions), - oldModifications: unwind(changes.modifications), - newModifications: unwind(changes.modificationsNew), - }); - } catch (err) { - // Scheduling a throw on the event loop, - // since throwing synchronously here would result in an abort in the calling C++ - setImmediate(() => { - throw err; - }); - } + callback(proxied, { + deletions: unwind(changes.deletions), + insertions: unwind(changes.insertions), + oldModifications: unwind(changes.modifications), + newModifications: unwind(changes.modificationsNew), + }); }, keyPaths ? this.mapKeyPaths(keyPaths) : keyPaths, ); diff --git a/packages/realm/src/Realm.ts b/packages/realm/src/Realm.ts index 4e33af4f27..352f15059f 100644 --- a/packages/realm/src/Realm.ts +++ b/packages/realm/src/Realm.ts @@ -455,6 +455,8 @@ export class Realm { syncConfig: config.sync ? toBindingSyncConfig(config.sync) : undefined, forceSyncHistory: config.openSyncedRealmLocally, automaticallyHandleBacklinksInMigrations: config.migrationOptions?.resolveEmbeddedConstraints ?? false, + // Use a platform-specific scheduler if the platform expose one + scheduler: binding.getPlatformScheduler ? binding.getPlatformScheduler() : undefined, }, }; }