diff --git a/CHANGELOG.md b/CHANGELOG.md index d67d2f53ee..d1f68f55ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ x.x.x Release notes (yyyy-MM-dd) ### Internal * Improved documentation for `Realm.copyBundledRealmFiles`. +* Refactored notifications to use a `NotificationBucket` API, enabling JS objects to be garbage collected on teardown of the JS engine. ([#4550](https://github.com/realm/realm-js/pull/4550)) * Updated a test to be ready for node 18. * Upgraded Realm Core from v11.14.0 to v11.15.0. diff --git a/integration-tests/tests/.mocharc.json b/integration-tests/tests/.mocharc.json index 10032931e1..70bbb91a95 100644 --- a/integration-tests/tests/.mocharc.json +++ b/integration-tests/tests/.mocharc.json @@ -6,5 +6,8 @@ "require": [ "ts-node/register/transpile-only", "src/node/inject-globals.ts" + ], + "node-option": [ + "expose_gc" ] } \ No newline at end of file diff --git a/integration-tests/tests/src/index.ts b/integration-tests/tests/src/index.ts index 7d75c1192e..4f0249f90b 100644 --- a/integration-tests/tests/src/index.ts +++ b/integration-tests/tests/src/index.ts @@ -35,6 +35,13 @@ import { testSkipIf, suiteSkipIf } from "./utils/skip-if"; global.describe.skipIf = suiteSkipIf; global.it.skipIf = testSkipIf; +afterEach(() => { + // Trigger garbage collection after every test, if exposed by the environment. + if (typeof global.gc === "function") { + global.gc(); + } +}); + // Using `require` instead of `import` here to ensure the Mocha globals (including `skipIf`) are set describe("Test Harness", () => { diff --git a/integration-tests/tests/src/typings.d.ts b/integration-tests/tests/src/typings.d.ts index 38168a19cd..97688e4420 100644 --- a/integration-tests/tests/src/typings.d.ts +++ b/integration-tests/tests/src/typings.d.ts @@ -37,6 +37,10 @@ interface Global extends NodeJS.Global { path: path; environment: Environment; require: Require; + /** + * The environment might expose a method to suggest a garbage collection. + */ + gc?: () => void; } declare const global: Global; diff --git a/react-native/ios/RealmReact/RealmReact.mm b/react-native/ios/RealmReact/RealmReact.mm index 682972f425..790bd9c638 100644 --- a/react-native/ios/RealmReact/RealmReact.mm +++ b/react-native/ios/RealmReact/RealmReact.mm @@ -22,6 +22,7 @@ #import #import +#import #include #import @@ -83,7 +84,7 @@ - (void *)runtime; return [rctJSContext context].JSGlobalContextRef; } -@interface RealmReact () +@interface RealmReact () @end @implementation RealmReact { @@ -303,10 +304,6 @@ void _initializeOnJSThread(JSContextRefExtractor jsContextExtractor, std::functi - (void)setBridge:(RCTBridge *)bridge { _bridge = bridge; - static __weak RealmReact *s_currentModule = nil; - [s_currentModule invalidate]; - s_currentModule = self; - if (objc_lookUpClass("RCTWebSocketExecutor") && [bridge executorClass] == objc_lookUpClass("RCTWebSocketExecutor")) { #if DEBUG [self startRPC]; diff --git a/src/js_dictionary.hpp b/src/js_dictionary.hpp index 3ff845c2d5..2331c10268 100644 --- a/src/js_dictionary.hpp +++ b/src/js_dictionary.hpp @@ -25,6 +25,7 @@ #include "js_results.hpp" #include "js_types.hpp" #include "js_util.hpp" +#include "js_notifications.hpp" #include #include @@ -122,7 +123,7 @@ class Dictionary : public realm::object_store::Dictionary { Dictionary& operator=(Dictionary&&) = default; Dictionary& operator=(Dictionary const&) = default; - std::vector, NotificationToken>> m_listeners; + notifications::NotificationHandle m_notification_handle; }; template @@ -137,6 +138,7 @@ struct DictionaryClass : ClassDefinition, Collection using String = js::String; using ReturnValue = js::ReturnValue; using Arguments = js::Arguments; + using NotificationBucket = notifications::NotificationBucket; static ObjectType create_instance(ContextType, realm::object_store::Dictionary); @@ -348,7 +350,7 @@ void DictionaryClass::add_listener(ContextType ctx, ObjectType this_object, A Function::callback(protected_ctx, protected_callback, protected_this, 2, arguments); }); - dictionary->m_listeners.emplace_back(protected_callback, std::move(token)); + NotificationBucket::emplace(dictionary->m_notification_handle, std::move(protected_callback), std::move(token)); } template @@ -359,14 +361,10 @@ void DictionaryClass::remove_listener(ContextType ctx, ObjectType this_object args.validate_maximum(1); auto callback = Value::validated_to_function(ctx, args[0]); - auto protected_function = + auto protected_callback = Protected(ctx, callback); // Protecting for comparison, not to extend lifetime. - auto& listeners = dictionary->m_listeners; - auto compare = [&](auto&& func_and_tok) { - return typename Protected::Comparator()(func_and_tok.first, protected_function); - }; - listeners.erase(std::remove_if(listeners.begin(), listeners.end(), compare), listeners.end()); + NotificationBucket::erase(dictionary->m_notification_handle, std::move(protected_callback)); } @@ -375,9 +373,8 @@ void DictionaryClass::remove_all_listeners(ContextType ctx, ObjectType this_o ReturnValue& return_value) { auto dictionary = get_internal>(ctx, this_object); - args.validate_maximum(0); - dictionary->m_listeners.clear(); + NotificationBucket::erase(dictionary->m_notification_handle); } } // namespace js diff --git a/src/js_list.hpp b/src/js_list.hpp index 3f7baa9a34..5ee573dcce 100644 --- a/src/js_list.hpp +++ b/src/js_list.hpp @@ -24,6 +24,7 @@ #include "js_results.hpp" #include "js_types.hpp" #include "js_util.hpp" +#include "js_notifications.hpp" #include #include @@ -42,7 +43,7 @@ class List : public realm::List { { } - std::vector, NotificationToken>> m_notification_tokens; + notifications::NotificationHandle m_notification_handle; }; template @@ -56,6 +57,7 @@ struct ListClass : ClassDefinition, CollectionClass> { using Value = js::Value; using ReturnValue = js::ReturnValue; using Arguments = js::Arguments; + using NotificationBucket = notifications::NotificationBucket; static ObjectType create_instance(ContextType, realm::List); @@ -329,7 +331,7 @@ void ListClass::remove_all_listeners(ContextType ctx, ObjectType this_object, { args.validate_maximum(0); auto list = get_internal>(ctx, this_object); - list->m_notification_tokens.clear(); + NotificationBucket::erase(list->m_notification_handle); } template diff --git a/src/js_notifications.hpp b/src/js_notifications.hpp new file mode 100644 index 0000000000..7cf7a47da9 --- /dev/null +++ b/src/js_notifications.hpp @@ -0,0 +1,163 @@ +//////////////////////////////////////////////////////////////////////////// +// +// Copyright 2021 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 "js_types.hpp" + +namespace realm::js::notifications { + +using IdType = unsigned long long; + +template +class NotificationHandle; + +/** + * @brief A class with static members used to manage ownership of `NotificationToken`s returned by the object store + * notification APIs. + * This abstraction is needed to enable graceful JS runtime destruction by preventing circular references from objects + * owning a `NotificationToken` owning a lambda that captures `Protected` values of the object itself. + * + * @note This exposes a simple `clear` method, which should be called just before the JS runtime is torn down. + * + * @tparam T The JS runtime types. + */ +template +class NotificationBucket { + using ProtectedFunction = Protected; + using TokensMap = std::map>>; + +public: + static void emplace(NotificationHandle& handle, ProtectedFunction&& callback, NotificationToken&& token) + { + if (handle) { + auto& s_tokens = get_tokens(); + s_tokens[handle].emplace_back(std::move(callback), std::move(token)); + } + else { + throw std::runtime_error("Cannot emplace notifications using an unset handle"); + } + } + + static void clear() + { + auto& s_tokens = get_tokens(); + s_tokens.clear(); + } + + static void erase(NotificationHandle& handle) + { + if (handle) { + auto& s_tokens = get_tokens(); + s_tokens.erase(handle); + } + } + + static void erase(NotificationHandle& handle, ProtectedFunction&& callback) + { + if (handle) { + auto& s_tokens = get_tokens(); + auto& tokens = s_tokens[handle]; + auto compare = [&callback](auto&& token) { + return typename ProtectedFunction::Comparator()(token.first, callback); + }; + tokens.erase(std::remove_if(tokens.begin(), tokens.end(), compare), tokens.end()); + } + else { + throw std::runtime_error("Cannot erase notifications using an unset handle"); + } + } + + /** + * @brief Get the static tokens object. + * + * @note We cannot use a simple static object on the namespace, because we are referencing this across multiple + * translation units and this could lead to code using this before it's initialized. + * + * @return auto& A reference to the static map of tokens. + */ + inline static TokensMap& get_tokens() + { + static TokensMap s_tokens; + return s_tokens; + } +}; + +/** + * @brief An object owned by objects which will delegate ownership of `NotificationToken`s to the + * `NotificationBucket`. + * + * @tparam T The JS runtime types. + */ +template +class NotificationHandle { + static inline IdType s_next_id = 0; + std::optional m_id; + +public: + /** + * @brief Construct a new Notification Handle object to be owned by an object and passed to the + * `NotificationBucket` when managing `NotificationToken`s returned by the object store notification APIs. + */ + NotificationHandle() + { + m_id = s_next_id; + if (s_next_id == std::numeric_limits::max()) { + throw std::overflow_error("No more NotificationHandle ids"); + } + s_next_id += 1; + }; + + /** + * @brief Destroys the Notification Handle object and erases any outstanding listeners from the + * `NotificationBucket`. + */ + ~NotificationHandle() + { + NotificationBucket::erase(*this); + } + + operator IdType() const + { + return *m_id; + }; + + operator bool() const + { + return m_id.has_value(); + }; + + /** + * @brief Move constructs a new Notification Handle by resetting the `id` on the object being moved from. + * This ensures that the abandoned object won't erase tokens from the `NotificationBucket` when destructed. + * + * @param other The object being moved from. + */ + NotificationHandle(NotificationHandle&& other) + { + m_id = std::move(other.m_id); + other.m_id.reset(); + } +}; + +} // namespace realm::js::notifications diff --git a/src/js_realm_object.hpp b/src/js_realm_object.hpp index 199e21bf2e..8d58c35b06 100644 --- a/src/js_realm_object.hpp +++ b/src/js_realm_object.hpp @@ -33,6 +33,7 @@ struct RealmObjectClass; #include "js_util.hpp" #include "js_realm.hpp" #include "js_schema.hpp" +#include "js_notifications.hpp" namespace realm { namespace js { @@ -47,11 +48,11 @@ class RealmObject : public realm::Object { : realm::Object(obj){}; RealmObject(realm::Object const& obj) : realm::Object(obj){}; - RealmObject(RealmObject&&) = default; + RealmObject(RealmObject&& other) = default; RealmObject& operator=(RealmObject&&) = default; RealmObject& operator=(RealmObject const&) = default; - std::vector, NotificationToken>> m_notification_tokens; + notifications::NotificationHandle m_notification_handle; }; template @@ -66,6 +67,7 @@ struct RealmObjectClass : ClassDefinition> { using Function = js::Function; using ReturnValue = js::ReturnValue; using Arguments = js::Arguments; + using NotificationBucket = notifications::NotificationBucket; static ObjectType create_instance(ContextType, realm::js::RealmObject); @@ -150,11 +152,10 @@ typename T::Object RealmObjectClass::create_instance(ContextType ctx, realm:: if (!delegate || !delegate->m_constructors.count(name)) { return create_instance_by_schema>(ctx, schema, internal); } - - FunctionType constructor = delegate->m_constructors.at(name); - auto object = create_instance_by_schema>(ctx, constructor, schema, internal); - - return object; + else { + FunctionType constructor = delegate->m_constructors.at(name); + return create_instance_by_schema>(ctx, constructor, schema, internal); + } } catch (const std::exception& e) { delete internal; @@ -403,7 +404,7 @@ void RealmObjectClass::add_listener(ContextType ctx, ObjectType this_object, ValueType arguments[]{static_cast(protected_this), object}; Function::callback(protected_ctx, protected_callback, protected_this, 2, arguments); }); - realm_object->m_notification_tokens.emplace_back(protected_callback, std::move(token)); + NotificationBucket::emplace(realm_object->m_notification_handle, std::move(protected_callback), std::move(token)); } template @@ -413,18 +414,14 @@ void RealmObjectClass::remove_listener(ContextType ctx, ObjectType this_objec args.validate_maximum(1); auto callback = Value::validated_to_function(ctx, args[0]); - auto protected_function = Protected(ctx, callback); + auto protected_callback = Protected(ctx, callback); auto realm_object = get_internal>(ctx, this_object); if (!realm_object) { throw std::runtime_error("Invalid 'this' object"); } - auto& tokens = realm_object->m_notification_tokens; - auto compare = [&](auto&& token) { - return typename Protected::Comparator()(token.first, protected_function); - }; - tokens.erase(std::remove_if(tokens.begin(), tokens.end(), compare), tokens.end()); + NotificationBucket::erase(realm_object->m_notification_handle, std::move(protected_callback)); } template @@ -438,7 +435,7 @@ void RealmObjectClass::remove_all_listeners(ContextType ctx, ObjectType this_ throw std::runtime_error("Invalid 'this' object"); } - realm_object->m_notification_tokens.clear(); + NotificationBucket::erase(realm_object->m_notification_handle); } template diff --git a/src/js_results.hpp b/src/js_results.hpp index 9be0b135b8..1e879dfd3e 100644 --- a/src/js_results.hpp +++ b/src/js_results.hpp @@ -21,6 +21,7 @@ #include "js_collection.hpp" #include "js_realm_object.hpp" #include "js_util.hpp" +#include "js_notifications.hpp" #include #include @@ -59,7 +60,7 @@ class Results : public realm::Results { using realm::Results::Results; - std::vector, NotificationToken>> m_notification_tokens; + notifications::NotificationHandle m_notification_handle; }; template @@ -73,6 +74,7 @@ struct ResultsClass : ClassDefinition, CollectionClass< using Value = js::Value; using ReturnValue = js::ReturnValue; using Arguments = js::Arguments; + using NotificationBucket = notifications::NotificationBucket; static ObjectType create_instance(ContextType, realm::Results); static ObjectType create_instance(ContextType, SharedRealm, const std::string& object_type); @@ -382,7 +384,7 @@ void ResultsClass::add_listener(ContextType ctx, U& collection, ObjectType th CollectionClass::create_collection_change_set(protected_ctx, change_set)}; Function::callback(protected_ctx, protected_callback, protected_this, 2, arguments); }); - collection.m_notification_tokens.emplace_back(protected_callback, std::move(token)); + NotificationBucket::emplace(collection.m_notification_handle, std::move(protected_callback), std::move(token)); } template @@ -400,13 +402,8 @@ void ResultsClass::remove_listener(ContextType ctx, U& collection, ObjectType args.validate_maximum(1); auto callback = Value::validated_to_function(ctx, args[0]); - auto protected_function = Protected(ctx, callback); - - auto& tokens = collection.m_notification_tokens; - auto compare = [&](auto&& token) { - return typename Protected::Comparator()(token.first, protected_function); - }; - tokens.erase(std::remove_if(tokens.begin(), tokens.end(), compare), tokens.end()); + auto protected_callback = Protected(ctx, callback); + NotificationBucket::erase(collection.m_notification_handle, std::move(protected_callback)); } template @@ -424,7 +421,7 @@ void ResultsClass::remove_all_listeners(ContextType ctx, ObjectType this_obje args.validate_maximum(0); auto results = get_internal>(ctx, this_object); - results->m_notification_tokens.clear(); + NotificationBucket::erase(results->m_notification_handle); } } // namespace js diff --git a/src/js_set.hpp b/src/js_set.hpp index 3fc42aae76..d6b55eda98 100644 --- a/src/js_set.hpp +++ b/src/js_set.hpp @@ -24,6 +24,7 @@ #include "js_results.hpp" #include "js_types.hpp" #include "js_util.hpp" +#include "js_notifications.hpp" #include #include @@ -118,7 +119,7 @@ class Set : public realm::object_store::Set { } void derive_property_type(StringData const& object_name, Property& prop) const; - std::vector, NotificationToken>> m_notification_tokens; + notifications::NotificationHandle m_notification_handle; }; @@ -139,6 +140,7 @@ struct SetClass : ClassDefinition, CollectionClass> { using Value = js::Value; using ReturnValue = js::ReturnValue; using Arguments = js::Arguments; + using NotificationBucket = notifications::NotificationBucket; static ObjectType create_instance(ContextType, realm::object_store::Set); @@ -540,7 +542,7 @@ void SetClass::remove_all_listeners(ContextType ctx, ObjectType this_object, { args.validate_maximum(0); auto set = get_internal>(ctx, this_object); - set->m_notification_tokens.clear(); + NotificationBucket::erase(set->m_notification_handle); } } // namespace js diff --git a/src/jsc/jsc_init.cpp b/src/jsc/jsc_init.cpp index b8576b549c..1b58c2f79c 100644 --- a/src/jsc/jsc_init.cpp +++ b/src/jsc/jsc_init.cpp @@ -24,6 +24,9 @@ #include "jsc_init.hpp" #include "platform.hpp" + +#include "js_notifications.hpp" + namespace realm { namespace jsc { js::Protected ObjectDefineProperty; @@ -67,6 +70,8 @@ void RJSInvalidateCaches() realm::_impl::RealmCoordinator::clear_all_caches(); // Clear the Object Store App cache, to prevent instances from using a context that was released realm::app::App::clear_cached_apps(); + // Clear Realm Object notifications + realm::js::notifications::NotificationBucket::clear(); } // Note: This must be called before RJSInvalidateCaches, otherwise the app cache