Skip to content

Commit

Permalink
Backport of NotificationBucket (#4550)
Browse files Browse the repository at this point in the history
* Implemented NotificationBuckets to own notification tokens

* Adding a note to the changelog

* Apply suggestions from code review

Co-authored-by: FFranck <[email protected]>
Co-authored-by: Andrew Meyer <[email protected]>

Co-authored-by: FFranck <[email protected]>
Co-authored-by: Andrew Meyer <[email protected]>
  • Loading branch information
3 people authored May 3, 2022
1 parent 403303b commit 6140d46
Show file tree
Hide file tree
Showing 12 changed files with 219 additions and 44 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
3 changes: 3 additions & 0 deletions integration-tests/tests/.mocharc.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,8 @@
"require": [
"ts-node/register/transpile-only",
"src/node/inject-globals.ts"
],
"node-option": [
"expose_gc"
]
}
7 changes: 7 additions & 0 deletions integration-tests/tests/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down
4 changes: 4 additions & 0 deletions integration-tests/tests/src/typings.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 2 additions & 5 deletions react-native/ios/RealmReact/RealmReact.mm
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

#import <React/RCTBridge+Private.h>
#import <React/RCTJavaScriptExecutor.h>
#import <React/RCTInvalidating.h>
#include <ReactCommon/CallInvoker.h>

#import <objc/runtime.h>
Expand Down Expand Up @@ -83,7 +84,7 @@ - (void *)runtime;
return [rctJSContext context].JSGlobalContextRef;
}

@interface RealmReact () <RCTBridgeModule>
@interface RealmReact () <RCTBridgeModule, RCTInvalidating>
@end

@implementation RealmReact {
Expand Down Expand Up @@ -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];
Expand Down
17 changes: 7 additions & 10 deletions src/js_dictionary.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "js_results.hpp"
#include "js_types.hpp"
#include "js_util.hpp"
#include "js_notifications.hpp"

#include <realm/object-store/shared_realm.hpp>
#include <realm/object-store/dictionary.hpp>
Expand Down Expand Up @@ -122,7 +123,7 @@ class Dictionary : public realm::object_store::Dictionary {
Dictionary& operator=(Dictionary&&) = default;
Dictionary& operator=(Dictionary const&) = default;

std::vector<std::pair<Protected<typename T::Function>, NotificationToken>> m_listeners;
notifications::NotificationHandle<T> m_notification_handle;
};

template <typename T>
Expand All @@ -137,6 +138,7 @@ struct DictionaryClass : ClassDefinition<T, realm::js::Dictionary<T>, Collection
using String = js::String<T>;
using ReturnValue = js::ReturnValue<T>;
using Arguments = js::Arguments<T>;
using NotificationBucket = notifications::NotificationBucket<T>;

static ObjectType create_instance(ContextType, realm::object_store::Dictionary);

Expand Down Expand Up @@ -348,7 +350,7 @@ void DictionaryClass<T>::add_listener(ContextType ctx, ObjectType this_object, A

Function<T>::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 <typename T>
Expand All @@ -359,14 +361,10 @@ void DictionaryClass<T>::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<FunctionType>(ctx, callback); // Protecting for comparison, not to extend lifetime.

auto& listeners = dictionary->m_listeners;
auto compare = [&](auto&& func_and_tok) {
return typename Protected<FunctionType>::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));
}


Expand All @@ -375,9 +373,8 @@ void DictionaryClass<T>::remove_all_listeners(ContextType ctx, ObjectType this_o
ReturnValue& return_value)
{
auto dictionary = get_internal<T, DictionaryClass<T>>(ctx, this_object);

args.validate_maximum(0);
dictionary->m_listeners.clear();
NotificationBucket::erase(dictionary->m_notification_handle);
}

} // namespace js
Expand Down
6 changes: 4 additions & 2 deletions src/js_list.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "js_results.hpp"
#include "js_types.hpp"
#include "js_util.hpp"
#include "js_notifications.hpp"

#include <realm/object-store/shared_realm.hpp>
#include <realm/object-store/list.hpp>
Expand All @@ -42,7 +43,7 @@ class List : public realm::List {
{
}

std::vector<std::pair<Protected<typename T::Function>, NotificationToken>> m_notification_tokens;
notifications::NotificationHandle<T> m_notification_handle;
};

template <typename T>
Expand All @@ -56,6 +57,7 @@ struct ListClass : ClassDefinition<T, realm::js::List<T>, CollectionClass<T>> {
using Value = js::Value<T>;
using ReturnValue = js::ReturnValue<T>;
using Arguments = js::Arguments<T>;
using NotificationBucket = notifications::NotificationBucket<T>;

static ObjectType create_instance(ContextType, realm::List);

Expand Down Expand Up @@ -329,7 +331,7 @@ void ListClass<T>::remove_all_listeners(ContextType ctx, ObjectType this_object,
{
args.validate_maximum(0);
auto list = get_internal<T, ListClass<T>>(ctx, this_object);
list->m_notification_tokens.clear();
NotificationBucket::erase(list->m_notification_handle);
}

template <typename T>
Expand Down
163 changes: 163 additions & 0 deletions src/js_notifications.hpp
Original file line number Diff line number Diff line change
@@ -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 <map>
#include <vector>

#include <realm/object-store/collection_notifications.hpp>

#include "js_types.hpp"

namespace realm::js::notifications {

using IdType = unsigned long long;

template <typename T>
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 <typename T>
class NotificationBucket {
using ProtectedFunction = Protected<typename T::Function>;
using TokensMap = std::map<IdType, std::vector<std::pair<ProtectedFunction, NotificationToken>>>;

public:
static void emplace(NotificationHandle<T>& 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<T>& handle)
{
if (handle) {
auto& s_tokens = get_tokens();
s_tokens.erase(handle);
}
}

static void erase(NotificationHandle<T>& 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 <typename T>
class NotificationHandle {
static inline IdType s_next_id = 0;
std::optional<IdType> 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<IdType>::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<T>::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
Loading

0 comments on commit 6140d46

Please sign in to comment.