From 40855f789c4cac25f7be4dd8dc146b27d2e59ae4 Mon Sep 17 00:00:00 2001 From: Scott Kyle Date: Fri, 6 May 2016 15:40:13 -0700 Subject: [PATCH 1/4] Prevent accidental leak of RealmDelegate If a binding context already exists, we make sure it's a js::RealmDelegate for the same JS context. If not, then we throw an exception because this could lead to serious trouble. Also, we update the defaults and constructors only if new ones were provided. --- src/js_realm.hpp | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/src/js_realm.hpp b/src/js_realm.hpp index 45b6b8c3..c7b49408 100644 --- a/src/js_realm.hpp +++ b/src/js_realm.hpp @@ -37,6 +37,9 @@ namespace realm { namespace js { +template +class Realm; + template struct RealmClass; @@ -60,7 +63,7 @@ class RealmDelegate : public BindingContext { } virtual void will_change(std::vector const& observers, std::vector const& invalidated) {} - RealmDelegate(std::weak_ptr realm, GlobalContextType ctx) : m_context(ctx), m_realm(realm) {} + RealmDelegate(std::weak_ptr realm, GlobalContextType ctx) : m_context(ctx), m_realm(realm) {} ~RealmDelegate() { // All protected values need to be unprotected while the context is retained. @@ -95,7 +98,7 @@ class RealmDelegate : public BindingContext { private: Protected m_context; std::list> m_notifications; - std::weak_ptr m_realm; + std::weak_ptr m_realm; void notify(const char *notification_name) { SharedRealm realm = m_realm.lock(); @@ -112,6 +115,8 @@ class RealmDelegate : public BindingContext { Function::call(m_context, callback, realm_object, 2, arguments); } } + + friend class Realm; }; std::string default_path(); @@ -120,6 +125,7 @@ void delete_all_realms(); template class Realm { + using GlobalContextType = typename T::GlobalContext; using ContextType = typename T::Context; using FunctionType = typename T::Function; using ObjectType = typename T::Object; @@ -251,6 +257,7 @@ void Realm::constructor(ContextType ctx, ObjectType this_object, size_t argc, realm::Realm::Config config; typename Schema::ObjectDefaultsMap defaults; typename Schema::ConstructorMap constructors; + bool schema_updated = false; if (argc == 0) { config.path = default_path(); @@ -281,6 +288,7 @@ void Realm::constructor(ContextType ctx, ObjectType this_object, size_t argc, if (!Value::is_undefined(ctx, schema_value)) { ObjectType schema_object = Value::validated_to_object(ctx, schema_value, "schema"); config.schema.reset(new realm::Schema(Schema::parse_schema(ctx, schema_object, defaults, constructors))); + schema_updated = true; } static const String schema_version_string = "schemaVersion"; @@ -304,7 +312,6 @@ void Realm::constructor(ContextType ctx, ObjectType this_object, size_t argc, Function::call(ctx, migration_function, 2, arguments); }; } - static const String encryption_key_string = "encryptionKey"; ValueType encryption_key_value = Object::get_property(ctx, object, encryption_key_string); @@ -322,14 +329,23 @@ void Realm::constructor(ContextType ctx, ObjectType this_object, size_t argc, ensure_directory_exists_for_file(config.path); SharedRealm realm = realm::Realm::get_shared_realm(config); - auto delegate = new RealmDelegate(realm, Context::get_global_context(ctx)); + GlobalContextType global_context = Context::get_global_context(ctx); + BindingContext *binding_context = realm->m_binding_context.get(); + RealmDelegate *js_binding_context = dynamic_cast *>(binding_context); - if (!realm->m_binding_context) { - realm->m_binding_context.reset(delegate); + if (!binding_context) { + js_binding_context = new RealmDelegate(realm, global_context); + realm->m_binding_context.reset(js_binding_context); + } + else if (!js_binding_context || js_binding_context->m_context != global_context) { + throw std::runtime_error("Realm is already open in another context on this thread: " + config.path); } - delegate->m_defaults = std::move(defaults); - delegate->m_constructors = std::move(constructors); + // If a new schema was provided, then use its defaults and constructors. + if (schema_updated) { + js_binding_context->m_defaults = std::move(defaults); + js_binding_context->m_constructors = std::move(constructors); + } set_internal>(this_object, new SharedRealm(realm)); } From fef4be96bdc5b548a9109c30be368cc1dba1a10c Mon Sep 17 00:00:00 2001 From: Scott Kyle Date: Fri, 6 May 2016 15:41:17 -0700 Subject: [PATCH 2/4] Update tests for changing defaults and constructors --- tests/js/realm-tests.js | 81 ++++++++++++++++++++++++++++++++++------- 1 file changed, 67 insertions(+), 14 deletions(-) diff --git a/tests/js/realm-tests.js b/tests/js/realm-tests.js index 35294bd4..540c78dc 100644 --- a/tests/js/realm-tests.js +++ b/tests/js/realm-tests.js @@ -46,12 +46,10 @@ module.exports = BaseTest.extend({ var defaultDir = Realm.defaultPath.substring(0, Realm.defaultPath.lastIndexOf("/") + 1) var testPath = 'test1.realm'; var realm = new Realm({schema: [], path: testPath}); - //TestCase.assertTrue(realm instanceof Realm); TestCase.assertEqual(realm.path, defaultDir + testPath); var testPath2 = 'test2.realm'; var realm2 = new Realm({schema: [], path: testPath2}); - //TestCase.assertTrue(realm2 instanceof Realm); TestCase.assertEqual(realm2.path, defaultDir + testPath2); }, @@ -68,10 +66,9 @@ module.exports = BaseTest.extend({ var realm = new Realm({path: 'test1.realm', schema: [], schemaVersion: 1}); TestCase.assertEqual(realm.schemaVersion, 1); - // FIXME - enable once Realm exposes a schema object - //TestCase.assertEqual(realm.schema.length, 0); - + TestCase.assertEqual(realm.schema.length, 0); realm.close(); + // FIXME - enable once realm initialization supports schema comparison // TestCase.assertThrows(function() { // realm = new Realm({path: testPath, schema: [schemas.TestObject], schemaVersion: 1}); @@ -81,7 +78,9 @@ module.exports = BaseTest.extend({ realm.write(function() { realm.create('TestObject', {doubleCol: 1}); }); - TestCase.assertEqual(realm.objects('TestObject')[0].doubleCol, 1) + TestCase.assertEqual(realm.objects('TestObject')[0].doubleCol, 1); + TestCase.assertEqual(realm.schemaVersion, 2); + TestCase.assertEqual(realm.schema.length, 1); }, testRealmConstructorDynamicSchema: function() { @@ -403,7 +402,7 @@ module.exports = BaseTest.extend({ testRealmCreateWithDefaults: function() { var realm = new Realm({schema: [schemas.DefaultValues, schemas.TestObject]}); - realm.write(function() { + var writeCallback = function() { var obj = realm.create('DefaultValuesObject', {}); var properties = schemas.DefaultValues.properties; @@ -418,7 +417,36 @@ module.exports = BaseTest.extend({ TestCase.assertEqual(obj.nullObjectCol, null); TestCase.assertEqual(obj.arrayCol.length, properties.arrayCol.default.length); TestCase.assertEqual(obj.arrayCol[0].doubleCol, properties.arrayCol.default[0].doubleCol); - }); + }; + + realm.write(writeCallback); + + // Defaults should still work when creating another Realm instance. + realm = new Realm(); + realm.write(writeCallback); + }, + + testRealmCreateWithChangingDefaults: function() { + var objectSchema = { + name: 'IntObject', + properties: { + intCol: {type: 'int', default: 1}, + } + }; + + var realm = new Realm({schema: [objectSchema]}); + + var writeCallback = function() { + var object = realm.create('IntObject', {}); + TestCase.assertEqual(object.intCol, objectSchema.properties.intCol.default); + }; + + realm.write(writeCallback); + + objectSchema.properties.intCol.default++; + + realm = new Realm({schema: [objectSchema]}); + realm.write(writeCallback); }, testRealmCreateWithConstructor: function() { @@ -426,7 +454,6 @@ module.exports = BaseTest.extend({ function CustomObject() { customCreated++; - this.intCol *= 100; } CustomObject.schema = { name: 'CustomObject', @@ -447,7 +474,7 @@ module.exports = BaseTest.extend({ properties: { intCol: 'int' } - } + }; var realm = new Realm({schema: [CustomObject, InvalidObject]}); @@ -457,15 +484,11 @@ module.exports = BaseTest.extend({ TestCase.assertTrue(Object.getPrototypeOf(object) == CustomObject.prototype); TestCase.assertEqual(customCreated, 1); - // Should have been multiplied by 100 in the constructor. - TestCase.assertEqual(object.intCol, 100); - // Should be able to create object by passing in constructor. object = realm.create(CustomObject, {intCol: 2}); TestCase.assertTrue(object instanceof CustomObject); TestCase.assertTrue(Object.getPrototypeOf(object) == CustomObject.prototype); TestCase.assertEqual(customCreated, 2); - TestCase.assertEqual(object.intCol, 200); }); TestCase.assertThrows(function() { @@ -483,6 +506,36 @@ module.exports = BaseTest.extend({ realm.create(InvalidCustomObject, {intCol: 1}); }); }); + + // The constructor should still work when creating another Realm instance. + realm = new Realm(); + TestCase.assertTrue(realm.objects('CustomObject')[0] instanceof CustomObject); + TestCase.assertTrue(realm.objects(CustomObject).length > 0); + }, + + testRealmCreateWithChangingConstructor: function() { + function CustomObject() {} + CustomObject.schema = { + name: 'CustomObject', + properties: { + intCol: 'int' + } + }; + + var realm = new Realm({schema: [CustomObject]}); + realm.write(function() { + var object = realm.create('CustomObject', {intCol: 1}); + TestCase.assertTrue(object instanceof CustomObject); + }); + + function NewCustomObject() {} + NewCustomObject.schema = CustomObject.schema; + + realm = new Realm({schema: [NewCustomObject]}); + realm.write(function() { + var object = realm.create('CustomObject', {intCol: 1}); + TestCase.assertTrue(object instanceof NewCustomObject); + }); }, testRealmDelete: function() { From d4b7a6bed37edc897c64c9ff33ef37df3423482a Mon Sep 17 00:00:00 2001 From: Scott Kyle Date: Fri, 6 May 2016 15:42:17 -0700 Subject: [PATCH 3/4] Update RPC layer for changing constructors This is very important for hot module reloading. --- lib/browser/index.js | 18 ++++++++++-------- lib/browser/objects.js | 22 +++++++++++++++++----- 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/lib/browser/index.js b/lib/browser/index.js index acf7c26d..e1522c54 100644 --- a/lib/browser/index.js +++ b/lib/browser/index.js @@ -19,11 +19,11 @@ 'use strict'; import { NativeModules } from 'react-native'; -import { keys, propTypes, objectTypes } from './constants'; +import { keys, objectTypes } from './constants'; import Collection, * as collections from './collections'; import List, { createList } from './lists'; import Results, { createResults } from './results'; -import RealmObject, { createObject, registerConstructors, typeForConstructor } from './objects'; +import RealmObject, * as objects from './objects'; import * as rpc from './rpc'; import * as util from './util'; @@ -31,7 +31,7 @@ const {debugHosts, debugPort} = NativeModules.Realm; rpc.registerTypeConverter(objectTypes.LIST, createList); rpc.registerTypeConverter(objectTypes.RESULTS, createResults); -rpc.registerTypeConverter(objectTypes.OBJECT, createObject); +rpc.registerTypeConverter(objectTypes.OBJECT, objects.createObject); rpc.registerTypeConverter(objectTypes.REALM, createRealm); function createRealm(_, info) { @@ -59,7 +59,7 @@ function setupRealm(realm, realmId) { export default class Realm { constructor(config) { let schemas = typeof config == 'object' && config.schema; - let constructors = {}; + let constructors = schemas ? {} : null; for (let i = 0, len = schemas ? schemas.length : 0; i < len; i++) { let item = schemas[i]; @@ -83,14 +83,15 @@ export default class Realm { } let realmId = rpc.createRealm(Array.from(arguments)); - - registerConstructors(realmId, constructors); setupRealm(this, realmId); + + // This will create mappings between the id, path, and potential constructors. + objects.registerConstructors(realmId, this.path, constructors); } create(type, ...args) { if (typeof type == 'function') { - type = typeForConstructor(this[keys.realm], type); + type = objects.typeForConstructor(this[keys.realm], type); } let method = util.createMethod(objectTypes.REALM, 'create', true); @@ -99,7 +100,7 @@ export default class Realm { objects(type, ...args) { if (typeof type == 'function') { - type = typeForConstructor(this[keys.realm], type); + type = objects.typeForConstructor(this[keys.realm], type); } let method = util.createMethod(objectTypes.REALM, 'objects'); @@ -147,6 +148,7 @@ Object.defineProperties(Realm, { clearTestState: { value: function() { collections.clearMutationListeners(); + objects.clearRegisteredConstructors(); rpc.clearTestState(); }, }, diff --git a/lib/browser/objects.js b/lib/browser/objects.js index 30ed84c2..908c1e15 100644 --- a/lib/browser/objects.js +++ b/lib/browser/objects.js @@ -21,7 +21,8 @@ import { keys, objectTypes } from './constants'; import { getterForProperty, setterForProperty, createMethods } from './util'; -const registeredConstructors = {}; +let registeredConstructors = {}; +let registeredRealmPaths = {}; export default class RealmObject { } @@ -31,9 +32,15 @@ createMethods(RealmObject.prototype, objectTypes.OBJECT, [ 'isValid', ]); +export function clearRegisteredConstructors() { + registeredConstructors = {}; + registeredRealmPaths = {}; +} + export function createObject(realmId, info) { let schema = info.schema; - let constructor = (registeredConstructors[realmId] || {})[schema.name]; + let realmPath = registeredRealmPaths[realmId]; + let constructor = (registeredConstructors[realmPath] || {})[schema.name]; let object = Object.create(constructor ? constructor.prototype : RealmObject.prototype); object[keys.realm] = realmId; @@ -58,12 +65,17 @@ export function createObject(realmId, info) { return object; } -export function registerConstructors(realmId, constructors) { - registeredConstructors[realmId] = constructors; +export function registerConstructors(realmId, realmPath, constructors) { + registeredRealmPaths[realmId] = realmPath; + + if (constructors) { + registeredConstructors[realmPath] = constructors; + } } export function typeForConstructor(realmId, constructor) { - let constructors = registeredConstructors[realmId]; + let realmPath = registeredRealmPaths[realmId]; + let constructors = registeredConstructors[realmPath]; for (let name in constructors) { if (constructors[name] == constructor) { From 351543ca09227372ccd0667af56a3fda0d21dabd Mon Sep 17 00:00:00 2001 From: Scott Kyle Date: Tue, 10 May 2016 11:37:09 -0700 Subject: [PATCH 4/4] Rename callback based on PR feedback --- tests/js/realm-tests.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/js/realm-tests.js b/tests/js/realm-tests.js index 540c78dc..79c018d6 100644 --- a/tests/js/realm-tests.js +++ b/tests/js/realm-tests.js @@ -402,7 +402,7 @@ module.exports = BaseTest.extend({ testRealmCreateWithDefaults: function() { var realm = new Realm({schema: [schemas.DefaultValues, schemas.TestObject]}); - var writeCallback = function() { + var createAndTestObject = function() { var obj = realm.create('DefaultValuesObject', {}); var properties = schemas.DefaultValues.properties; @@ -419,11 +419,11 @@ module.exports = BaseTest.extend({ TestCase.assertEqual(obj.arrayCol[0].doubleCol, properties.arrayCol.default[0].doubleCol); }; - realm.write(writeCallback); + realm.write(createAndTestObject); // Defaults should still work when creating another Realm instance. realm = new Realm(); - realm.write(writeCallback); + realm.write(createAndTestObject); }, testRealmCreateWithChangingDefaults: function() { @@ -436,17 +436,17 @@ module.exports = BaseTest.extend({ var realm = new Realm({schema: [objectSchema]}); - var writeCallback = function() { + var createAndTestObject = function() { var object = realm.create('IntObject', {}); TestCase.assertEqual(object.intCol, objectSchema.properties.intCol.default); }; - realm.write(writeCallback); + realm.write(createAndTestObject); objectSchema.properties.intCol.default++; realm = new Realm({schema: [objectSchema]}); - realm.write(writeCallback); + realm.write(createAndTestObject); }, testRealmCreateWithConstructor: function() {